Merge pull request #1255 from ispyisail/fix/pdf-link-order

Fix PDF cross-reference links coming out in a different order each run

sources/pdf_links.cpp

@@ -35,6 +35,8 @@
 #include <QUrl>
 #include <QVector>
 
+#include <algorithm>
+
 namespace PdfLinks {
 
 void injectCrossRefLinks(QPdfEngine *engine, Diagram *diagram,
@@ -83,6 +85,14 @@ void injectCrossRefLinks(QPdfEngine *engine, Diagram *diagram,
 					  QPointF(qMax(a.x(), b.x()), qMax(a.y(), b.y())));
 	};
 
+	// The links are collected first and drawn sorted, not in the order the
+	// loop below finds them: a cross-reference's contacts are keyed by
+	// Element pointer and the scene's items come in stacking order, so that
+	// order changes from run to run and the same project gave a different
+	// PDF each time it was exported.
+	struct Link { QRectF rect; QUrl url; };
+	QList<Link> links;
+
 	auto injectLink = [&](const QRectF &sceneRect, Element *targetElmt) {
 		if (!targetElmt || !targetElmt->diagram()) return;
 		const int targetPage = pageMap.value(targetElmt->diagram(), -1);
@@ -99,7 +109,7 @@ void injectCrossRefLinks(QPdfEngine *engine, Diagram *diagram,
 
 		QUrl url = QUrl::fromLocalFile(outputFileName);
 		url.setFragment(frag);
-		engine->drawHyperlink(devRect, url);
+		links.append({devRect, url});
 	};
 
 	for (auto *item : diagram->items()) {
@@ -165,6 +175,22 @@ void injectCrossRefLinks(QPdfEngine *engine, Diagram *diagram,
 			continue;
 		}
 	}
+
+	// Top to bottom, then left to right; the target breaks a tie between
+	// two links on the same spot.
+	std::sort(links.begin(), links.end(), [](const Link &a, const Link &b) {
+		if (a.rect.top() != b.rect.top())
+			return a.rect.top() < b.rect.top();
+		if (a.rect.left() != b.rect.left())
+			return a.rect.left() < b.rect.left();
+		if (a.rect.bottom() != b.rect.bottom())
+			return a.rect.bottom() < b.rect.bottom();
+		if (a.rect.right() != b.rect.right())
+			return a.rect.right() < b.rect.right();
+		return a.url.fragment() < b.url.fragment();
+	});
+	for (const Link &link : std::as_const(links))
+		engine->drawHyperlink(link.rect, link.url);
 }
 
 void convertUriToGoTo(const QString &pdfPath)

tests/qttest/CMakeLists.txt

@@ -407,6 +407,19 @@ target_link_libraries(tst_pdfpagesize PRIVATE Qt::Test Qt::Gui)
 target_compile_definitions(tst_pdfpagesize PRIVATE
   "QET_TEST_BINARY_PATH=\"$<TARGET_FILE:qelectrotech>\"")
 
+# --export-pdf writes the cross-reference links in the same order in every
+# run, rather than in Element pointer order. Exports examples/industrial.qet
+# several times, in separate processes.
+add_executable(
+  tst_pdflinkorder
+  tst_pdflinkorder.cpp)
+add_test(NAME tst_pdflinkorder COMMAND tst_pdflinkorder)
+add_dependencies(tst_pdflinkorder qelectrotech)
+target_link_libraries(tst_pdflinkorder PRIVATE Qt::Test)
+target_compile_definitions(tst_pdflinkorder PRIVATE
+  "QET_TEST_BINARY_PATH=\"$<TARGET_FILE:qelectrotech>\""
+  "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"")
+
 # One text per potential: the number goes on the same conductor in every
 # run when two conductors tie for longest. Exports
 # fixtures/one_text_per_potential_tie.qet through the real binary, several

tests/qttest/tst_pdflinkorder.cpp

@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+	--export-pdf writes a project's cross-reference links in the same order
+	every time. They used to come out in the order of Element pointers, so
+	the same project gave a different PDF in each run. Exported in separate
+	processes, since pointers only change between runs.
+*/
+#include <QDir>
+#include <QFile>
+#include <QProcess>
+#include <QRegularExpression>
+#include <QTemporaryDir>
+#include <QtTest>
+
+class tst_pdflinkorder : public QObject
+{
+	Q_OBJECT
+
+	QTemporaryDir m_dir;
+
+	// The link annotations' rectangles and destinations, in file order.
+	QStringList exportLinks(const QString &project, int run)
+	{
+		const QString home = m_dir.filePath(QStringLiteral("home%1").arg(run));
+		const QString tmp = m_dir.filePath(QStringLiteral("tmp%1").arg(run));
+		const QString out = m_dir.filePath(QStringLiteral("out%1.pdf").arg(run));
+		QDir().mkpath(home);
+		QDir().mkpath(tmp);
+		QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
+		env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen"));
+		env.insert(QStringLiteral("HOME"), home);
+		env.insert(QStringLiteral("XDG_CONFIG_HOME"), home + QStringLiteral("/.config"));
+		env.insert(QStringLiteral("XDG_DATA_HOME"), home + QStringLiteral("/.local/share"));
+		env.insert(QStringLiteral("TMPDIR"), tmp);
+
+		QProcess proc;
+		proc.setProcessEnvironment(env);
+		proc.start(QStringLiteral(QET_TEST_BINARY_PATH),
+				   {QStringLiteral("--export-pdf"), project, out});
+		if (!proc.waitForFinished(120000) || proc.exitCode() != 0)
+			return {};
+
+		QFile file(out);
+		if (!file.open(QIODevice::ReadOnly))
+			return {};
+		const QString pdf = QString::fromLatin1(file.readAll());
+		static const QRegularExpression link(QStringLiteral(
+			R"(/Subtype /Link\s*/Rect \[([^\]]*)\][\s\S]*?/D \[\d+ 0 R ([^\]]*)\])"));
+		QStringList links;
+		auto it = link.globalMatch(pdf);
+		while (it.hasNext()) {
+			const auto m = it.next();
+			links << m.captured(1).simplified() + QStringLiteral(" -> ")
+					 + m.captured(2).simplified();
+		}
+		return links;
+	}
+
+private slots:
+	void initTestCase()
+	{
+		QVERIFY(m_dir.isValid());
+		QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH)));
+	}
+
+	void sameOrderEveryRun()
+	{
+		const QString project =
+			QStringLiteral(QET_EXAMPLES_DIR) + QStringLiteral("/industrial.qet");
+		QVERIFY2(QFile::exists(project), "examples/industrial.qet not found");
+
+		const QStringList first = exportLinks(project, 0);
+		QVERIFY2(first.size() >= 2, "expected several cross-reference links");
+		for (int run = 1; run < 5; ++run)
+			QCOMPARE(exportLinks(project, run), first);
+	}
+};
+
+QTEST_MAIN(tst_pdflinkorder)
+#include "tst_pdflinkorder.moc"