Draw markers better next to the position slider (#3005).

3 files changed, 125 insertions, 109 deletions

diff --git a/src/wx/markers.cc b/src/wx/markers.cc
index a2ef1e9e7..97e6ac41b 100644
--- a/src/wx/markers.cc
+++ b/src/wx/markers.cc
@@ -30,16 +30,16 @@ vector<pair<wxString, dcp::Marker>>
 all_editable_markers()
 {
 	return vector<pair<wxString, dcp::Marker>>{
-		{ _("First frame of ratings band"), dcp::Marker::FFOB },
-		{ _("Last frame of ratings band"), dcp::Marker::LFOB },
-		{ _("First frame of title credits"), dcp::Marker::FFTC },
-		{ _("Last frame of title credits"), dcp::Marker::LFTC },
-		{ _("First frame of intermission"), dcp::Marker::FFOI },
-		{ _("Last frame of intermission"), dcp::Marker::LFOI },
-		{ _("First frame of end credits"), dcp::Marker::FFEC },
-		{ _("Last frame of end credits"), dcp::Marker::LFEC },
-		{ _("First frame of moving credits"), dcp::Marker::FFMC },
-		{ _("Last frame of moving credits"), dcp::Marker::LFMC }
+		{ _("First frame of ratings band (FFOB)"), dcp::Marker::FFOB },
+		{ _("Last frame of ratings band (LFOB)"), dcp::Marker::LFOB },
+		{ _("First frame of title credits (FFTC)"), dcp::Marker::FFTC },
+		{ _("Last frame of title credits (LFTC)"), dcp::Marker::LFTC },
+		{ _("First frame of intermission (FFOI)"), dcp::Marker::FFOI },
+		{ _("Last frame of intermission (LFOI)"), dcp::Marker::LFOI },
+		{ _("First frame of end credits (FFEC)"), dcp::Marker::FFEC },
+		{ _("Last frame of end credits (LFEC)"), dcp::Marker::LFEC },
+		{ _("First frame of moving credits (FFMC)"), dcp::Marker::FFMC },
+		{ _("Last frame of moving credits (LFMC)"), dcp::Marker::LFMC }
 	};
 }
 
diff --git a/src/wx/markers_panel.cc b/src/wx/markers_panel.cc
index 3a5e8c30b..675aa5e77 100644
--- a/src/wx/markers_panel.cc
+++ b/src/wx/markers_panel.cc
@@ -34,7 +34,10 @@ LIBDCP_ENABLE_WARNINGS
 #include <boost/version.hpp>
 
 
+using std::make_pair;
+using std::pair;
 using std::shared_ptr;
+using std::vector;
 using std::weak_ptr;
 #if BOOST_VERSION >= 106100
 using namespace boost::placeholders;
@@ -56,11 +59,12 @@ static constexpr auto line_to_label_gap = 2;
 
 
 MarkersPanel::MarkersPanel(wxWindow* parent, FilmViewer& viewer)
-	: wxPanel(parent, wxID_ANY, wxDefaultPosition, wxSize(-1, 16))
+	: wxPanel(parent, wxID_ANY, wxDefaultPosition, wxSize(-1, 40))
 	, _viewer(viewer)
 {
 	Bind(wxEVT_PAINT, boost::bind(&MarkersPanel::paint, this));
 	Bind(wxEVT_MOTION, boost::bind(&MarkersPanel::mouse_moved, this, _1));
+	Bind(wxEVT_SIZE, boost::bind(&MarkersPanel::size, this));
 
 	Bind(wxEVT_LEFT_DOWN, boost::bind(&MarkersPanel::mouse_left_down, this));
 	Bind(wxEVT_RIGHT_DOWN, boost::bind(&MarkersPanel::mouse_right_down, this, _1));
@@ -72,13 +76,19 @@ MarkersPanel::MarkersPanel(wxWindow* parent, FilmViewer& viewer)
 
 
 void
+MarkersPanel::size()
+{
+	layout();
+}
+
+
+void
 MarkersPanel::set_film(weak_ptr<Film> weak_film)
 {
 	_film = weak_film;
-	auto film = weak_film.lock();
-	if (film) {
+	if (auto film = weak_film.lock()) {
 		film->Change.connect(boost::bind(&MarkersPanel::film_changed, this, _1, _2));
-		update_from_film(film);
+		layout();
 	}
 }
 
@@ -96,54 +106,52 @@ MarkersPanel::film_changed(ChangeType type, FilmProperty property)
 	}
 
 	if (property == FilmProperty::MARKERS || property == FilmProperty::CONTENT || property == FilmProperty::CONTENT_ORDER || property == FilmProperty::VIDEO_FRAME_RATE) {
-		update_from_film(film);
+		layout();
 	}
 }
 
 
 void
-MarkersPanel::update_from_film(shared_ptr<Film> film)
+MarkersPanel::layout()
 {
-	_markers.clear();
-	for (auto const& marker: film->markers()) {
-		_markers[marker.first] = Marker(
-			marker.second,
-			marker.first == dcp::Marker::FFOC ||
-			marker.first == dcp::Marker::FFTC ||
-			marker.first == dcp::Marker::FFOI ||
-			marker.first == dcp::Marker::FFEC ||
-			marker.first == dcp::Marker::FFMC
-			);
-
+	auto film = _film.lock();
+	if (!film || !film->length().get()) {
+		_components.clear();
+		return;
 	}
-	Refresh();
-}
 
+	wxClientDC dc(this);
+	auto const panel_width = GetSize().GetWidth();
 
-int
-MarkersPanel::position(dcpomatic::DCPTime time, int width) const
-{
 #ifdef DCPOMATIC_LINUX
-	/* Number of pixels between the left/right bounding box edge of a wxSlider
-	 * and the start of the "track".
-	 */
-	auto constexpr end_gap = 12;
+		/* Number of pixels between the left/right bounding box edge of a wxSlider
+		 * and the start of the "track".
+		 */
+		auto constexpr end_gap = 12;
 #else
-	auto constexpr end_gap = 0;
+		auto constexpr end_gap = 0;
 #endif
-	auto film = _film.lock();
-	if (!film) {
-		return 0;
-	}
 
-	return end_gap + time.get() * (width - end_gap * 2) / film->length().get();
+	_components = layout_markers(
+		film->markers(),
+		panel_width - end_gap,
+		film->length(),
+		12,
+		4,
+		[&dc](std::string text) { return dc.GetTextExtent(std_to_wx(text)).GetWidth(); }
+	);
+
+	_over = nullptr;
+	_menu_marker = nullptr;
+
+	Refresh();
 }
 
 
 void
 MarkersPanel::mouse_moved(wxMouseEvent& ev)
 {
-	_over = boost::none;
+	_over = nullptr;
 
 	auto film = _film.lock();
 	if (!film) {
@@ -151,28 +159,27 @@ MarkersPanel::mouse_moved(wxMouseEvent& ev)
 	}
 
 	auto const panel_width = GetSize().GetWidth();
+
 #if !defined(DCPOMATIC_LINUX)
 	auto const panel_height = GetSize().GetHeight();
 	auto const factor = GetContentScaleFactor();
 #endif
 
+	auto const scale = static_cast<float>(panel_width) / film->length().get();
+
 	auto const x = ev.GetPosition().x;
-	for (auto const& marker: _markers) {
-		auto const pos = position(marker.second.time, panel_width);
-		auto const width = marker.second.width ? marker.second.width : 4;
-		auto const x1 = marker.second.line_before_label ? pos : pos - width - line_to_label_gap;
-		auto const x2 = marker.second.line_before_label ? pos + width + line_to_label_gap : pos;
-		if (x1 <= x && x < x2) {
-			_over = marker.first;
+
+	for (auto const& marker: _components) {
+		auto const p = marker.t1.get() * scale;
+		if (marker.marker && std::abs(p - x) < 16) {
+			_over = &marker;
 /* Tooltips flicker really badly on Wayland for some reason, so only do this on Windows/macOS for now */
 #if !defined(DCPOMATIC_LINUX)
 			if (!_tip) {
 				auto mouse = ClientToScreen(ev.GetPosition());
-				auto rect = wxRect(mouse.x, mouse.y, width * factor, panel_height * factor);
-				auto hmsf = marker.second.time.split(film->video_frame_rate());
-				char timecode_buffer[64];
-				snprintf(timecode_buffer, sizeof(timecode_buffer), " %02d:%02d:%02d:%02d", hmsf.h, hmsf.m, hmsf.s, hmsf.f);
-				auto tip_text = dcp::marker_to_string(marker.first) + std::string(timecode_buffer);
+				auto rect = wxRect(mouse.x, mouse.y, 8 * factor, panel_height * factor);
+				auto hmsf = marker.t1.split(film->video_frame_rate());
+				auto const tip_text = fmt::format("{} {:02d}:{:02d}:{:02d}:{:02d}", dcp::marker_to_string(*marker.marker), hmsf.h, hmsf.m, hmsf.s, hmsf.f);
 				_tip = new wxTipWindow(this, std_to_wx(tip_text), 100, &_tip, &rect);
 			}
 #endif
@@ -192,42 +199,61 @@ MarkersPanel::paint()
 	}
 
 	gc->SetAntialiasMode(wxANTIALIAS_DEFAULT);
-	gc->SetPen(wxPen(wxColour(200, 0, 0)));
-	gc->SetFont(gc->CreateFont(*wxSMALL_FONT, wxColour(200, 0, 0)));
+	auto const colour = gui_is_dark() ? wxColour(199, 139, 167) : wxColour(200, 0, 0);
+	gc->SetPen(colour);
+	gc->SetFont(gc->CreateFont(*wxSMALL_FONT, colour));
+	gc->SetBrush(GetBackgroundColour());
 
-	auto const panel_width = GetSize().GetWidth();
 	auto const panel_height = GetSize().GetHeight();
 
-	for (auto& marker: _markers) {
-		auto label = std_to_wx(dcp::marker_to_string(marker.first));
-		if (marker.second.width == 0) {
-			/* We don't know the width of this marker label yet, so calculate it now */
-			wxDouble width, height, descent, external_leading;
-			gc->GetTextExtent(label, &width, &height, &descent, &external_leading);
-			marker.second.width = width;
-		}
-		auto line = gc->CreatePath();
-		auto const pos = position(marker.second.time, panel_width);
-		line.MoveToPoint(pos, 0);
-		line.AddLineToPoint(pos, panel_height);
-		gc->StrokePath(line);
-
-		auto label_x = 0;
-
-		if (GetLayoutDirection() == wxLayout_RightToLeft) {
-			auto matrix = dc.GetTransformMatrix();
-			matrix.Translate(0, 0);
-			matrix.Mirror(wxHORIZONTAL);
-			dc.SetTransformMatrix(matrix);
-			label_x = marker.second.line_before_label ? (pos + line_to_label_gap + marker.second.width) : (pos - line_to_label_gap);
-			label_x = -label_x;
-		} else {
-			label_x = marker.second.line_before_label ? (pos + line_to_label_gap) : (pos - line_to_label_gap - marker.second.width);
+	int rows = 0;
+	for (auto const& component: _components) {
+		rows = std::max(rows, component.y + 1);
+	}
+
+	auto const row_height = std::min(static_cast<float>(panel_height) / rows, 16.0f);
+	auto const row_gap = 3;
+
+	auto const base = [row_height, panel_height](MarkerLayoutComponent const& component) {
+		return panel_height - (component.y + 1) * row_height;
+	};
+
+	for (auto const& component: _components) {
+		if (component.type == MarkerLayoutComponent::Type::LINE) {
+			gc->CreatePath();
+			auto line = gc->CreatePath();
+			line.MoveToPoint(component.x1, base(component) + (row_height - row_gap) / 2);
+			line.AddLineToPoint(component.x2, base(component) + (row_height - row_gap) / 2);
+			gc->StrokePath(line);
 		}
+	}
 
-		gc->DrawText(label, label_x, 0);
+	for (auto const& component: _components) {
+		switch (component.type) {
+		case MarkerLayoutComponent::Type::LEFT:
+		case MarkerLayoutComponent::Type::RIGHT:
+		{
+			gc->CreatePath();
+			auto line = gc->CreatePath();
+			line.MoveToPoint(component.x1, base(component));
+			line.AddLineToPoint(component.x1, base(component) + row_height - row_gap);
+			gc->StrokePath(line);
+			break;
+		}
+		case MarkerLayoutComponent::Type::LABEL:
+		{
+			gc->CreatePath();
+			auto rectangle = gc->CreatePath();
+			rectangle.MoveToPoint(component.x1 - 2, base(component));
+			rectangle.AddRectangle(component.x1 - 2, base(component), component.x2 - component.x1, row_height);
+			gc->FillPath(rectangle);
+			gc->DrawText(std_to_wx(component.text), component.x1, base(component) - 4);
+			break;
+		}
 
-		dc.ResetTransformMatrix();
+		case MarkerLayoutComponent::Type::LINE:
+			break;
+		}
 	}
 }
 
@@ -236,7 +262,7 @@ void
 MarkersPanel::mouse_left_down()
 {
 	if (_over) {
-		_viewer.seek(_markers[*_over].time, true);
+		_viewer.seek(_over->t1, true);
 	}
 }
 
@@ -246,8 +272,9 @@ MarkersPanel::mouse_right_down(wxMouseEvent& ev)
 {
 	wxMenu menu;
 	if (_over) {
-		menu.Append(ID_move_marker_to_current_position, wxString::Format(_("Move %s marker to current position"), std_to_wx(dcp::marker_to_string(*_over))));
-		menu.Append(ID_remove_marker, wxString::Format(_("Remove %s marker"), std_to_wx(dcp::marker_to_string(*_over))));
+		DCPOMATIC_ASSERT(_over->marker);
+		menu.Append(ID_move_marker_to_current_position, wxString::Format(_("Move %s marker to current position"), std_to_wx(dcp::marker_to_string(*_over->marker))));
+		menu.Append(ID_remove_marker, wxString::Format(_("Remove %s marker"), std_to_wx(dcp::marker_to_string(*_over->marker))));
 	}
 
 	auto add_marker = new wxMenu();
@@ -269,7 +296,8 @@ MarkersPanel::move_marker_to_current_position()
 		return;
 	}
 
-	film->set_marker(*_menu_marker, _viewer.position());
+	DCPOMATIC_ASSERT(static_cast<bool>(_menu_marker->marker));
+	film->set_marker(*_menu_marker->marker, _viewer.position());
 }
 
 
@@ -281,7 +309,8 @@ MarkersPanel::remove_marker()
 		return;
 	}
 
-	film->unset_marker(*_menu_marker);
+	DCPOMATIC_ASSERT(static_cast<bool>(_menu_marker->marker));
+	film->unset_marker(*_menu_marker->marker);
 }
 
 
diff --git a/src/wx/markers_panel.h b/src/wx/markers_panel.h
index 45caf9c4e..760df23ac 100644
--- a/src/wx/markers_panel.h
+++ b/src/wx/markers_panel.h
@@ -21,6 +21,7 @@
 
 #include "lib/dcpomatic_time.h"
 #include "lib/film_property.h"
+#include "lib/layout_markers.h"
 #include <dcp/warnings.h>
 LIBDCP_DISABLE_WARNINGS
 #include <wx/wx.h>
@@ -40,36 +41,22 @@ public:
 
 private:
 	void paint();
+	void size();
 	void mouse_moved(wxMouseEvent& ev);
 	void mouse_left_down();
 	void mouse_right_down(wxMouseEvent& ev);
-	int position(dcpomatic::DCPTime time, int width) const;
 	void move_marker_to_current_position();
 	void remove_marker();
 	void add_marker(wxCommandEvent& ev);
 	void film_changed(ChangeType type, FilmProperty property);
-	void update_from_film(std::shared_ptr<Film> film);
+	void layout();
 
 	wxTipWindow* _tip = nullptr;
 
-	class Marker {
-	public:
-		Marker() {}
-
-		Marker(dcpomatic::DCPTime t, bool b)
-			: time(t)
-			, line_before_label(b)
-		{}
-
-		dcpomatic::DCPTime time;
-		int width = 0;
-		bool line_before_label = false;
-	};
-
 	std::weak_ptr<Film> _film;
-	std::map<dcp::Marker, Marker> _markers;
-	boost::optional<dcp::Marker> _over;
+	std::vector<MarkerLayoutComponent> _components;
+	MarkerLayoutComponent const* _over = nullptr;
 	FilmViewer& _viewer;
-	boost::optional<dcp::Marker> _menu_marker;
+	MarkerLayoutComponent const* _menu_marker = nullptr;
 };