From e7245abe84a1a09a5f680e426b5dfac2de2e472a Mon Sep 17 00:00:00 2001
From: Carl Hetherington <cth@carlh.net>
Date: Fri, 2 Jun 2023 23:38:16 +0200
Subject: Fix incorrect hint about having too many subtitle lines in some cases
 (#2546).

---
 src/lib/hints.cc   | 25 ++++++++++++++++++++++++-
 test/data          |  2 +-
 test/hints_test.cc |  6 ++++++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/lib/hints.cc b/src/lib/hints.cc
index b628efc31..6c574d991 100644
--- a/src/lib/hints.cc
+++ b/src/lib/hints.cc
@@ -584,7 +584,30 @@ Hints::open_subtitle (PlayerText text, DCPTimePeriod period)
 		hint (_("At least one of your subtitles starts less than 2 frames after the previous one.  It is advisable to make the gap between subtitles at least 2 frames."));
 	}
 
-	if (text.string.size() > 3 && !_too_many_subtitle_lines) {
+	struct VPos
+	{
+	public:
+		dcp::VAlign align;
+		float position;
+
+		bool operator<(VPos const& other) const {
+			if (static_cast<int>(align) != static_cast<int>(other.align)) {
+				return static_cast<int>(align) < static_cast<int>(other.align);
+			}
+			return position < other.position;
+		}
+	};
+
+	/* This is rather an approximate way to count distinct lines, but I guess it will do;
+	 * to make it better we need to take into account font metrics, and the SMPTE alignment
+	 * debacle, and so on.
+	 */
+	std::set<VPos> lines;
+	for (auto const& line: text.string) {
+		lines.insert({ line.v_align(), line.v_position() });
+	}
+
+	if (lines.size() > 3 && !_too_many_subtitle_lines) {
 		_too_many_subtitle_lines = true;
 		hint (_("At least one of your subtitles has more than 3 lines.  It is advisable to use no more than 3 lines."));
 	}
diff --git a/test/data b/test/data
index ece8e470d..53fd88a32 160000
--- a/test/data
+++ b/test/data
@@ -1 +1 @@
-Subproject commit ece8e470df37357b9a9c29360c15213b6e71ec30
+Subproject commit 53fd88a32ec14dfed9059095cf5019adb8de04f8
diff --git a/test/hints_test.cc b/test/hints_test.cc
index 55e859e0d..5e59ef924 100644
--- a/test/hints_test.cc
+++ b/test/hints_test.cc
@@ -153,6 +153,12 @@ BOOST_AUTO_TEST_CASE (hint_many_subtitle_lines)
 }
 
 
+BOOST_AUTO_TEST_CASE(hint_many_subtitle_lines2)
+{
+	check(TextType::OPEN_SUBTITLE, "hint_many_subtitle_lines2");
+}
+
+
 BOOST_AUTO_TEST_CASE (hint_subtitle_too_long)
 {
 	check (
-- 
cgit v1.2.3