Merge pull request #37 from tomba/work

Formatting changes and testmodes.py

14 files changed, 161 insertions, 106 deletions

diff --git a/.gitmodules b/.gitmodules
index c6d1083..6ac367e 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -1,3 +1,6 @@
 [submodule "ext/pybind11"]
 	path = ext/pybind11
 	url = https://github.com/pybind/pybind11.git
+[submodule "ext/fmt"]
+	path = ext/fmt
+	url = https://github.com/fmtlib/fmt.git
diff --git a/CMakeLists.txt b/CMakeLists.txt
index f2f24fe..216f537 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -79,6 +79,8 @@ add_subdirectory(kms++)
 add_subdirectory(kms++util)
 add_subdirectory(utils)
 
+add_subdirectory(ext/fmt)
+
 if(KMSXX_ENABLE_KMSCUBE)
 	add_subdirectory(kmscube)
 endif()
diff --git a/ext/fmt b/ext/fmt
new file mode 160000
+Subproject 7512a55aa3ae309587ca89668ef9ec4074a51a1
diff --git a/kms++/CMakeLists.txt b/kms++/CMakeLists.txt
index 10352a2..1ca8815 100644
--- a/kms++/CMakeLists.txt
+++ b/kms++/CMakeLists.txt
@@ -22,7 +22,7 @@ target_include_directories(kms++ PUBLIC
     $<INSTALL_INTERFACE:include>
     PRIVATE src)
 
-target_link_libraries(kms++ ${LIBDRM_LIBRARIES} ${LIBDRM_OMAP_LIBRARIES})
+target_link_libraries(kms++ ${LIBDRM_LIBRARIES} ${LIBDRM_OMAP_LIBRARIES} fmt::fmt)
 
 set_target_properties(kms++ PROPERTIES
     PUBLIC_HEADER "${PUB_HDRS}")
diff --git a/kms++/inc/kms++/videomode.h b/kms++/inc/kms++/videomode.h
index d7f5258..74aa8f1 100644
--- a/kms++/inc/kms++/videomode.h
+++ b/kms++/inc/kms++/videomode.h
@@ -49,7 +49,9 @@ struct Videomode
 	void set_hsync(SyncPolarity pol);
 	void set_vsync(SyncPolarity pol);
 
-	std::string to_string() const;
+	std::string to_string_short() const;
+	std::string to_string_long() const;
+	std::string to_string_long_padded() const;
 };
 
 struct Videomode videomode_from_timings(uint32_t clock_khz,
diff --git a/kms++/src/videomode.cpp b/kms++/src/videomode.cpp
index b8bd797..7099898 100644
--- a/kms++/src/videomode.cpp
+++ b/kms++/src/videomode.cpp
@@ -2,6 +2,7 @@
 #include <xf86drmMode.h>
 #include <math.h>
 #include <sstream>
+#include <fmt/format.h>
 
 #include <kms++/kms++.h>
 #include "helpers.h"
@@ -88,14 +89,53 @@ void Videomode::set_vsync(SyncPolarity pol)
 	}
 }
 
-string Videomode::to_string() const
+string Videomode::to_string_short() const
 {
-	std::stringstream ss;
-	ss << hdisplay << "x" << vdisplay;
-	if (interlace())
-		ss << "i";
-	ss << "@" << calculated_vrefresh();
-	return ss.str();
+	return fmt::format("{}x{}{}@{:.2f}", hdisplay, vdisplay, interlace() ? "i" : "", calculated_vrefresh());
+}
+
+static char sync_to_char(SyncPolarity pol)
+{
+	switch (pol) {
+	case SyncPolarity::Positive:
+		return '+';
+	case SyncPolarity::Negative:
+		return '-';
+	default:
+		return '?';
+	}
+}
+
+string Videomode::to_string_long() const
+{
+	string h = fmt::format("{}/{}/{}/{}/{}", hdisplay, hfp(), hsw(), hbp(), sync_to_char(hsync()));
+	string v = fmt::format("{}/{}/{}/{}/{}", vdisplay, vfp(), vsw(), vbp(), sync_to_char(vsync()));
+
+	string str = fmt::format("{} {:.3f} {} {} {} ({:.2f}) {:#x} {:#x}",
+				 to_string_short(),
+				 clock / 1000.0,
+				 h, v,
+				 vrefresh, calculated_vrefresh(),
+				 flags,
+				 type);
+
+	return str;
+}
+
+string Videomode::to_string_long_padded() const
+{
+	string h = fmt::format("{}/{}/{}/{}/{}", hdisplay, hfp(), hsw(), hbp(), sync_to_char(hsync()));
+	string v = fmt::format("{}/{}/{}/{}/{}", vdisplay, vfp(), vsw(), vbp(), sync_to_char(vsync()));
+
+	string str = fmt::format("{:<16} {:7.3f} {:<18} {:<18} {:2} ({:.2f}) {:#10x} {:#6x}",
+				 to_string_short(),
+				 clock / 1000.0,
+				 h, v,
+				 vrefresh, calculated_vrefresh(),
+				 flags,
+				 type);
+
+	return str;
 }
 
 Videomode videomode_from_timings(uint32_t clock_khz,
diff --git a/kms++util/inc/kms++util/strhelpers.h b/kms++util/inc/kms++util/strhelpers.h
index 2c540f3..2d35e93 100644
--- a/kms++util/inc/kms++util/strhelpers.h
+++ b/kms++util/inc/kms++util/strhelpers.h
@@ -28,6 +28,3 @@ std::string join(const std::vector<T>& values, const std::string& delim, std::fu
 	}
 	return ss.str();
 }
-
-std::string sformat(const char *fmt, ...)
-	__attribute__ ((format (printf, 1, 2)));
diff --git a/kms++util/src/strhelpers.cpp b/kms++util/src/strhelpers.cpp
index f59bb6d..5eba2a9 100644
--- a/kms++util/src/strhelpers.cpp
+++ b/kms++util/src/strhelpers.cpp
@@ -11,17 +11,3 @@ string to_lower(const string& str)
 	transform(data.begin(), data.end(), data.begin(), ::tolower);
 	return data;
 }
-
-string sformat(const char *fmt, ...)
-{
-	static char s_format_buf[1024];
-
-	va_list args;
-	va_start(args, fmt);
-
-	vsnprintf(s_format_buf, sizeof(s_format_buf), fmt, args);
-
-	va_end(args);
-
-	return string(s_format_buf);
-}
diff --git a/py/pykms/pykmsbase.cpp b/py/pykms/pykmsbase.cpp
index 668e6e3..b4dc090 100644
--- a/py/pykms/pykmsbase.cpp
+++ b/py/pykms/pykmsbase.cpp
@@ -229,6 +229,9 @@ void init_pykmsbase(py::module &m)
 
 			.def_property("hsync", &Videomode::hsync, &Videomode::set_hsync)
 			.def_property("vsync", &Videomode::vsync, &Videomode::set_vsync)
+
+			.def("to_string_short", &Videomode::to_string_short)
+			.def("to_string_long", &Videomode::to_string_long)
 			;
 
 
diff --git a/py/tests/testmodes.py b/py/tests/testmodes.py
new file mode 100755
index 0000000..0508f31
--- /dev/null
+++ b/py/tests/testmodes.py
@@ -0,0 +1,47 @@
+#!/usr/bin/python3
+
+import sys
+import pykms
+import argparse
+
+parser = argparse.ArgumentParser()
+parser.add_argument("-c", "--connector", default="")
+args = parser.parse_args()
+
+card = pykms.Card()
+
+res = pykms.ResourceManager(card)
+conn = res.reserve_connector(args.connector)
+crtc = res.reserve_crtc(conn)
+plane = res.reserve_generic_plane(crtc)
+
+card.disable_planes()
+
+modes = conn.get_modes()
+
+def even(i):
+	return i & ~1
+
+for mode in modes:
+	long_str = mode.to_string_long()
+	short_str = mode.to_string_short()
+
+	print(long_str)
+
+	modeb = mode.to_blob(card)
+
+	fb = pykms.DumbFramebuffer(card, mode.hdisplay, mode.vdisplay, "XR24");
+	pykms.draw_test_pattern(fb);
+	pykms.draw_text(fb, even((fb.width // 2) - (8 * len(short_str)) // 2), 4, short_str, pykms.white)
+
+	req = pykms.AtomicReq(card)
+
+	req.add_connector(conn, crtc)
+	req.add_crtc(crtc, modeb)
+	req.add_plane(plane, fb, crtc, dst=(0, 0, mode.hdisplay, mode.vdisplay))
+
+	req.commit_sync(allow_modeset = True)
+
+	input("press enter to show next videomode\n")
+
+print("done")
diff --git a/utils/kmsprint.cpp b/utils/kmsprint.cpp
index 116fead..61baec0 100644
--- a/utils/kmsprint.cpp
+++ b/utils/kmsprint.cpp
@@ -4,6 +4,7 @@
 #include <iostream>
 #include <string>
 #include <unistd.h>
+#include <fmt/format.h>
 
 #include <kms++/kms++.h>
 #include <kms++util/kms++util.h>
@@ -22,11 +23,11 @@ static string format_mode(const Videomode& m, unsigned idx)
 {
 	string str;
 
-	str = sformat("  %2u ", idx);
+	str = fmt::format("  {:2} ", idx);
 
 	if (s_opts.x_modeline) {
-		str += sformat("%12s %6u %4u %4u %4u %4u %4u %4u %4u %4u  %2u %#x %#x",
-			       m.name.c_str(),
+		str += fmt::format("{:12} {:6} {:4} {:4} {:4} {:4} {:4} {:4} {:4} {:4} {:3} {:#x} {:#x}",
+			       m.name,
 			       m.clock,
 			       m.hdisplay, m.hsync_start, m.hsync_end, m.htotal,
 			       m.vdisplay, m.vsync_start, m.vsync_end, m.vtotal,
@@ -34,16 +35,7 @@ static string format_mode(const Videomode& m, unsigned idx)
 			       m.flags,
 			       m.type);
 	} else {
-		string h = sformat("%u/%u/%u/%u", m.hdisplay, m.hfp(), m.hsw(), m.hbp());
-		string v = sformat("%u/%u/%u/%u", m.vdisplay, m.vfp(), m.vsw(), m.vbp());
-
-		str += sformat("%-12s %7.3f %-16s %-16s %2u (%.2f) %#10x %#6x",
-			       m.name.c_str(),
-			       m.clock / 1000.0,
-			       h.c_str(), v.c_str(),
-			       m.vrefresh, m.calculated_vrefresh(),
-			       m.flags,
-			       m.type);
+		str += m.to_string_long_padded();
 	}
 
 	return str;
@@ -51,13 +43,13 @@ static string format_mode(const Videomode& m, unsigned idx)
 
 static string format_mode_short(const Videomode& m)
 {
-	string h = sformat("%u/%u/%u/%u", m.hdisplay, m.hfp(), m.hsw(), m.hbp());
-	string v = sformat("%u/%u/%u/%u", m.vdisplay, m.vfp(), m.vsw(), m.vbp());
+	string h = fmt::format("{}/{}/{}/{}", m.hdisplay, m.hfp(), m.hsw(), m.hbp());
+	string v = fmt::format("{}/{}/{}/{}", m.vdisplay, m.vfp(), m.vsw(), m.vbp());
 
-	return sformat("%s %.3f %s %s %u (%.2f)",
-		       m.name.c_str(),
+	return fmt::format("{} {:.3f} {} {} {} ({:.2f})",
+		       m.name,
 		       m.clock / 1000.0,
-		       h.c_str(), v.c_str(),
+		       h, v,
 		       m.vrefresh, m.calculated_vrefresh());
 }
 
@@ -65,8 +57,8 @@ static string format_connector(Connector& c)
 {
 	string str;
 
-	str = sformat("Connector %u (%u) %s",
-		      c.idx(), c.id(), c.fullname().c_str());
+	str = fmt::format("Connector {} ({}) {}",
+		      c.idx(), c.id(), c.fullname());
 
 	switch (c.connector_status()) {
 	case ConnectorStatus::Connected:
@@ -85,15 +77,15 @@ static string format_connector(Connector& c)
 
 static string format_encoder(Encoder& e)
 {
-	return sformat("Encoder %u (%u) %s",
-		       e.idx(), e.id(), e.get_encoder_type().c_str());
+	return fmt::format("Encoder {} ({}) {}",
+		       e.idx(), e.id(), e.get_encoder_type());
 }
 
 static string format_crtc(Crtc& c)
 {
 	string str;
 
-	str = sformat("Crtc %u (%u)", c.idx(), c.id());
+	str = fmt::format("Crtc {} ({})", c.idx(), c.id());
 
 	if (c.mode_valid())
 		str += " " + format_mode_short(c.mode());
@@ -105,17 +97,17 @@ static string format_plane(Plane& p)
 {
 	string str;
 
-	str = sformat("Plane %u (%u)", p.idx(), p.id());
+	str = fmt::format("Plane {} ({})", p.idx(), p.id());
 
 	if (p.fb_id())
-		str += sformat(" fb-id: %u", p.fb_id());
+		str += fmt::format(" fb-id: {}", p.fb_id());
 
 	string crtcs = join<Crtc*>(p.get_possible_crtcs(), " ", [](Crtc* crtc) { return to_string(crtc->idx()); });
 
-	str += sformat(" (crtcs: %s)", crtcs.c_str());
+	str += fmt::format(" (crtcs: {})", crtcs);
 
 	if (p.card().has_atomic()) {
-		str += sformat(" %u,%u %ux%u -> %u,%u %ux%u",
+		str += fmt::format(" {},{} {}x{} -> {},{} {}x{}",
 			       (uint32_t)p.get_prop_value("SRC_X") >> 16,
 			       (uint32_t)p.get_prop_value("SRC_Y") >> 16,
 			       (uint32_t)p.get_prop_value("SRC_W") >> 16,
@@ -128,20 +120,20 @@ static string format_plane(Plane& p)
 
 	string fmts = join<PixelFormat>(p.get_formats(), " ", [](PixelFormat fmt) { return PixelFormatToFourCC(fmt); });
 
-	str += sformat(" (%s)", fmts.c_str());
+	str += fmt::format(" ({})", fmts);
 
 	return str;
 }
 
 static string format_fb(Framebuffer& fb)
 {
-	return sformat("FB %u %ux%u",
+	return fmt::format("FB {} {}x{}",
 		       fb.id(), fb.width(), fb.height());
 }
 
 static string format_property(const Property* prop, uint64_t val)
 {
-	string ret = sformat("%s (%u) = ", prop->name().c_str(), prop->id());
+	string ret = fmt::format("{} ({}) = ", prop->name(), prop->id());
 
 	switch (prop->type()) {
 	case PropertyType::Bitmask:
@@ -151,10 +143,11 @@ static string format_property(const Property* prop, uint64_t val)
 		for (auto kvp : prop->get_enums()) {
 			if (val & (1 << kvp.first))
 				v.push_back(kvp.second);
-			vall.push_back(sformat("%s=0x%x", kvp.second.c_str(), 1 << kvp.first));
+			vall.push_back(fmt::format("{}={:#x}", kvp.second, 1 << kvp.first));
 		}
 
-		ret += sformat("0x%" PRIx64 " (%s) [%s]", val, join(v, "|").c_str(), join(vall, "|").c_str());
+		// XXX
+		ret += fmt::format("{:#x} ({}) [{}]", val, join(v, "|"), join(vall, "|"));
 
 		break;
 	}
@@ -167,9 +160,9 @@ static string format_property(const Property* prop, uint64_t val)
 			Blob blob(prop->card(), blob_id);
 			auto data = blob.data();
 
-			ret += sformat("blob-id %u len %zu", blob_id, data.size());
+			ret += fmt::format("blob-id {} len {}", blob_id, data.size());
 		} else {
-			ret += sformat("blob-id %u", blob_id);
+			ret += fmt::format("blob-id {}", blob_id);
 		}
 
 		break;
@@ -183,17 +176,17 @@ static string format_property(const Property* prop, uint64_t val)
 		for (auto kvp : prop->get_enums()) {
 			if (val == kvp.first)
 				cur = kvp.second;
-			vall.push_back(sformat("%s=%" PRIu64, kvp.second.c_str(), kvp.first));
+			vall.push_back(fmt::format("{}={}", kvp.second, kvp.first));
 		}
 
-		ret += sformat("%" PRIu64 " (%s) [%s]", val, cur.c_str(), join(vall, "|").c_str());
+		ret += fmt::format("{} ({}) [{}]", val, cur, join(vall, "|"));
 
 		break;
 	}
 
 	case PropertyType::Object:
 	{
-		ret += sformat("object id %u", (uint32_t)val);
+		ret += fmt::format("object id {}", val);
 		break;
 	}
 
@@ -201,7 +194,7 @@ static string format_property(const Property* prop, uint64_t val)
 	{
 		auto values = prop->get_values();
 
-		ret += sformat("%" PRIu64 " [%" PRIu64 " - %" PRIu64 "]",
+		ret += fmt::format("{} [{} - {}]",
 			       val, values[0], values[1]);
 
 		break;
@@ -211,7 +204,7 @@ static string format_property(const Property* prop, uint64_t val)
 	{
 		auto values = prop->get_values();
 
-		ret += sformat("%" PRIi64 " [%" PRIi64 " - %" PRIi64 "]",
+		ret += fmt::format("{} [{} - {}]",
 			       (int64_t)val, (int64_t)values[0], (int64_t)values[1]);
 
 		break;
@@ -317,14 +310,16 @@ static const map<TreeGlyph, string> glyphs_ascii = {
 
 };
 
-const char* get_glyph(TreeGlyph glyph)
+const string& get_glyph(TreeGlyph glyph)
 {
+	static const string s_empty = "  ";
+
 	if (s_glyph_mode == TreeGlyphMode::None)
-		return "  ";
+		return s_empty;
 
 	const map<TreeGlyph, string>& glyphs = s_glyph_mode == TreeGlyphMode::UTF8 ? glyphs_utf8 : glyphs_ascii;
 
-	return glyphs.at(glyph).c_str();
+	return glyphs.at(glyph);
 }
 
 static void print_entry(const Entry& e, const string& prefix, bool is_child, bool is_last)
@@ -337,7 +332,7 @@ static void print_entry(const Entry& e, const string& prefix, bool is_child, boo
 		prefix2 = prefix + (is_last ? get_glyph(TreeGlyph::Space) : get_glyph(TreeGlyph::Vertical));
 	}
 
-	printf("%s%s\n", prefix1.c_str(), e.title.c_str());
+	fmt::print("{}{}\n", prefix1, e.title);
 
 	bool has_children = e.children.size() > 0;
 
@@ -345,7 +340,7 @@ static void print_entry(const Entry& e, const string& prefix, bool is_child, boo
 
 	for (const string& str : e.lines) {
 		string p = data_prefix + get_glyph(TreeGlyph::Space);
-		printf("%s%s\n", p.c_str(), str.c_str());
+		fmt::print("{}{}\n", p, str);
 	}
 
 	for (const Entry& child : e.children) {
@@ -399,16 +394,16 @@ static void print_as_list(Card& card)
 	}
 
 	for (DrmPropObject* ob: obs) {
-		printf("%s\n", format_ob(ob).c_str());
+		fmt::print("{}\n", format_ob(ob));
 
 		if (s_opts.print_props) {
 			for (string str : format_props(ob))
-				printf("    %s\n", str.c_str());
+				fmt::print("    {}\n", str);
 		}
 	}
 
 	for (Framebuffer* fb: fbs) {
-		printf("%s\n", format_ob(fb).c_str());
+		fmt::print("{}\n", format_ob(fb));
 	}
 }
 
@@ -473,11 +468,11 @@ static void print_modes(Card& card)
 		if (!conn->connected())
 			continue;
 
-		printf("%s\n", format_ob(conn).c_str());
+		fmt::print("{}\n", format_ob(conn));
 
 		auto modes = conn->get_modes();
 		for (unsigned i = 0; i < modes.size(); ++i)
-			printf("%s\n", format_mode(modes[i], i).c_str());
+			fmt::print("{}\n", format_mode(modes[i], i));
 	}
 }
 
diff --git a/utils/kmstest.cpp b/utils/kmstest.cpp
index 8144117..2e3d054 100644
--- a/utils/kmstest.cpp
+++ b/utils/kmstest.cpp
@@ -691,32 +691,6 @@ static vector<OutputInfo> setups_to_outputs(Card& card, ResourceManager& resman,
 	return outputs;
 }
 
-static char sync_to_char(SyncPolarity pol)
-{
-	switch (pol) {
-	case SyncPolarity::Positive:
-		return '+';
-	case SyncPolarity::Negative:
-		return '-';
-	default:
-		return '?';
-	}
-}
-
-static std::string videomode_to_string(const Videomode& m)
-{
-	string h = sformat("%u/%u/%u/%u/%c", m.hdisplay, m.hfp(), m.hsw(), m.hbp(), sync_to_char(m.hsync()));
-	string v = sformat("%u/%u/%u/%u/%c", m.vdisplay, m.vfp(), m.vsw(), m.vbp(), sync_to_char(m.vsync()));
-
-	return sformat("%s %.3f %s %s %u (%.2f) %#x %#x",
-		       m.name.c_str(),
-		       m.clock / 1000.0,
-		       h.c_str(), v.c_str(),
-		       m.vrefresh, m.calculated_vrefresh(),
-		       m.flags,
-		       m.type);
-}
-
 static void print_outputs(const vector<OutputInfo>& outputs)
 {
 	for (unsigned i = 0; i < outputs.size(); ++i) {
@@ -735,7 +709,7 @@ static void print_outputs(const vector<OutputInfo>& outputs)
 			printf(" %s=%" PRIu64, prop.prop->name().c_str(),
 			       prop.val);
 
-		printf(": %s\n", videomode_to_string(o.mode).c_str());
+		printf(": %s\n", o.mode.to_string_long().c_str());
 
 		if (!o.legacy_fbs.empty()) {
 			auto fb = o.legacy_fbs[0];
diff --git a/utils/wbcap.cpp b/utils/wbcap.cpp
index 5a94a70..886fe36 100644
--- a/utils/wbcap.cpp
+++ b/utils/wbcap.cpp
@@ -342,9 +342,9 @@ int main(int argc, char** argv)
 	if (src_mode.interlace())
 		dst_height /= 2;
 
-	printf("src %s, crtc %s\n", src_conn->fullname().c_str(), src_mode.to_string().c_str());
+	printf("src %s, crtc %s\n", src_conn->fullname().c_str(), src_mode.to_string_short().c_str());
 
-	printf("dst %s, crtc %s\n", dst_conn->fullname().c_str(), dst_mode.to_string().c_str());
+	printf("dst %s, crtc %s\n", dst_conn->fullname().c_str(), dst_mode.to_string_short().c_str());
 
 	printf("src_fb %ux%u, dst_fb %ux%u\n", src_width, src_height, dst_width, dst_height);
 
diff --git a/utils/wbm2m.cpp b/utils/wbm2m.cpp
index b69bb28..71a26a5 100644
--- a/utils/wbm2m.cpp
+++ b/utils/wbm2m.cpp
@@ -6,6 +6,7 @@
 #include <fstream>
 #include <map>
 #include <system_error>
+#include <fmt/format.h>
 
 #include <kms++/kms++.h>
 #include <kms++util/kms++util.h>
@@ -101,8 +102,13 @@ int main(int argc, char** argv)
 		exit(-1);
 	}
 
-	const string filename = sformat("wb-out-%ux%u_%4.4s.raw", dst_width, dst_height,
-					PixelFormatToFourCC(dst_fmt).c_str());
+	printf("%ux%u-%s -> %ux%u-%s\n", src_width, src_height, PixelFormatToFourCC(src_fmt).c_str(),
+	       dst_width, dst_height, PixelFormatToFourCC(dst_fmt).c_str());
+
+	const string filename = fmt::format("wb-out-{}x{}-{}.raw", dst_width, dst_height,
+					PixelFormatToFourCC(dst_fmt));
+
+	printf("writing to %s\n", filename.c_str());
 
 	VideoDevice vid("/dev/video10");