Merge pull request #1477 from ispyisail/perf/parallel-picture-decode

Speed up opening folios with photos: decode pictures in parallel

sources/diagram.cpp

@@ -53,6 +53,8 @@
 #include "foliogrid.h"
 #include <QGraphicsView>
 #include <QTextStream>
+#include <QThreadPool>
+#include <QtConcurrentMap>
 #include <algorithm>
 #include <climits>
 #include <cassert>
@@ -2006,16 +2008,37 @@ bool Diagram::fromXml(QDomElement &document,
 	}
 
 		// Load image
-	QList<DiagramImageItem *> added_images;
-	for (auto image_xml : QET::findInDomElement(root,
-												QStringLiteral("images"),
-												QStringLiteral("image"))) {
-		DiagramImageItem *dii = new DiagramImageItem ();
+		// Decoding is most of the time of a folio with photos (a quarter of
+		// a second each), so the pictures are decoded first, in parallel,
+		// and the items built afterwards in this thread.
+	const QList<QDomElement> images_xml = QET::findInDomElement(
+				root, QStringLiteral("images"), QStringLiteral("image"));
+	QList<DiagramImageItem::Picture> pictures;
+	pictures.reserve(images_xml.size());
+	for (const QDomElement &image_xml : images_xml) {
 		const QString picture_file = QetContainer::pictureFile(image_xml);
-		if (!picture_file.isEmpty() && project())
-			dii -> fromXml(image_xml, project() -> containerPicture(picture_file));
-		else
-			dii -> fromXml(image_xml);
+		pictures << DiagramImageItem::pictureOf(
+						image_xml,
+						!picture_file.isEmpty() && project()
+						? project() -> containerPicture(picture_file)
+						: QByteArray());
+	}
+	if (pictures.size() > 1) {
+		QThreadPool pool;
+		pool.setMaxThreadCount(std::min<int>(QThread::idealThreadCount(),
+											 pictures.size()));
+		QtConcurrent::blockingMap(&pool, pictures,
+								  &DiagramImageItem::decodePicture);
+	} else {
+		for (DiagramImageItem::Picture &picture : pictures)
+			DiagramImageItem::decodePicture(picture);
+	}
+
+	QList<DiagramImageItem *> added_images;
+	for (qsizetype i = 0 ; i < images_xml.size() ; ++i) {
+		const QDomElement &image_xml = images_xml.at(i);
+		DiagramImageItem *dii = new DiagramImageItem ();
+		dii -> fromXml(image_xml, std::move(pictures[i]));
 		settle_uuid(dii, image_xml, QStringLiteral("image"), added_images.size());
 		ItemGroups::setGroup(dii, ItemGroups::read(image_xml));
 		addItem(dii);

sources/qetgraphicsitem/diagramimageitem.cpp

@@ -1819,6 +1819,60 @@ bool DiagramImageItem::fromXml(const QDomElement &e)
 	pictures as files (QetContainer::join()).
 */
 bool DiagramImageItem::fromXml(const QDomElement &e, const QByteArray &picture)
+{
+	Picture p = pictureOf(e, picture);
+	decodePicture(p);
+	return fromXml(e, std::move(p));
+}
+
+/**
+	@brief DiagramImageItem::pictureOf
+	@return the picture of @p e, @p picture unless it is empty, else the
+	base64 text of @p e, still encoded : decodePicture() does the slow part.
+	Reads the DOM, so GUI thread only.
+*/
+DiagramImageItem::Picture DiagramImageItem::pictureOf(const QDomElement &e,
+													  const QByteArray &picture)
+{
+	Picture p;
+	p.bytes = picture;
+	if (p.bytes.isEmpty())
+	{
+		// From the first text node only, not e.text(): that also collects
+		// the text of every child element, so on a cropped or colour-keyed
+		// picture it carries <image_base>'s bytes too.
+		const QDomNode image_node = e.firstChild();
+		if (image_node.isText())
+			p.base64 = image_node.toText().data();
+		else
+			p.found = false;
+	}
+	return p;
+}
+
+/**
+	@brief DiagramImageItem::decodePicture
+	Decodes the base64 text and the picture file of @p picture. Touches
+	nothing else, so it can run in any thread : a large photo takes about
+	a quarter of a second, and Diagram::fromXml() decodes a folio's
+	pictures in parallel.
+*/
+void DiagramImageItem::decodePicture(Picture &picture)
+{
+	if (picture.bytes.isEmpty() && !picture.base64.isEmpty())
+	{
+		picture.bytes = QByteArray::fromBase64(picture.base64.toLatin1());
+		picture.base64.clear();
+	}
+	picture.image = QImage::fromData(picture.bytes);
+}
+
+/**
+	@brief DiagramImageItem::fromXml
+	Load this image from xml element e, its picture already decoded by
+	decodePicture().
+*/
+bool DiagramImageItem::fromXml(const QDomElement &e, Picture picture)
 {
 	if (e.tagName() != "image") {
 		return (false);
@@ -1828,23 +1882,13 @@ bool DiagramImageItem::fromXml(const QDomElement &e, const QByteArray &picture)
 	const QUuid uuid(e.attribute(QStringLiteral("uuid")));
 	if (!uuid.isNull() && uuid != m_uuid) setUuid(uuid);
 
-	// From the first text node only, not e.text(): that also collects the
-	// text of every child element, so on a cropped or colour-keyed picture
-	// it carries <image_base>'s bytes too. Decoded once: on a project with
-	// large pictures each decoding costs seconds.
-	QByteArray array = picture;
-	if (array.isEmpty())
-	{
-		QDomNode image_node = e.firstChild();
-		if (!image_node.isText()) {
-			return (false);
-		}
-		array = QByteArray::fromBase64(image_node.toText().data().toLatin1());
+	if (!picture.found) {
+		return (false);
 	}
+	const QByteArray array = picture.bytes;
 
-	//Set QPixmap from the array
-	QPixmap pixmap;
-	pixmap.loadFromData(array);
+	//Set QPixmap from the decoded picture
+	QPixmap pixmap = QPixmap::fromImage(std::move(picture.image));
 	setPixmap(pixmap);
 	if (isPng(array) && !pixmap_.isNull())
 	{

sources/qetgraphicsitem/diagramimageitem.h

@@ -24,6 +24,7 @@
 
 #include <QColor>
 #include <QFont>
+#include <QImage>
 #include <QList>
 #include <QUuid>
 #include <QStyleOptionGraphicsItem>
@@ -119,8 +120,20 @@ class DiagramImageItem : public QetGraphicsItem {
 	*/
 	int type() const override { return Type; }
 	
+	/// A picture as read from its <image> element, before and after
+	/// decodePicture(), which may run outside the GUI thread.
+	struct Picture {
+		QString base64;		///< text of <image>, decoded when bytes is empty
+		QByteArray bytes;	///< the picture file (PNG, JPEG...)
+		QImage image;		///< bytes, decoded
+		bool found = true;	///< false : neither bytes nor text
+	};
+	static Picture pictureOf(const QDomElement &, const QByteArray &picture);
+	static void decodePicture(Picture &picture);
+
 	virtual bool fromXml(const QDomElement &);
 	bool fromXml(const QDomElement &, const QByteArray &picture);
+	bool fromXml(const QDomElement &, Picture picture);
 	virtual QDomElement toXml(QDomDocument &) const;
 	QUuid uuid() const {return m_uuid;}
 	void setUuid(const QUuid &uuid) {m_uuid = uuid; emit uuidChanged();}

tests/qttest/CMakeLists.txt

@@ -271,6 +271,10 @@ qet_add_app_test(tst_rotateselectionimage tst_rotateselectionimage.cpp)
 # it selected and records no undo step (#1414); the key goes to the view.
 qet_add_app_test(tst_imagehandleescape tst_imagehandleescape.cpp)
 
+# A folio's pictures, decoded in parallel, each come back with its own
+# pixels and position; an <image> with no picture does not stop the others.
+qet_add_app_test(tst_folioimageload tst_folioimageload.cpp)
+
 # SVG export with smaller pictures (--svg-small-pictures, the export dialog's
 # option): a picture drawn at a tenth of its size is stored at four times
 # that, a small one whole; without the option every picture is whole.

tests/qttest/tst_folioimageload.cpp

@@ -0,0 +1,132 @@
+/*
+	Copyright 2006-2026 The QElectroTech Team
+	This file is part of QElectroTech.
+
+	QElectroTech is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 2 of the License, or
+	(at your option) any later version.
+
+	QElectroTech is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with QElectroTech.  If not, see <http://www.gnu.org/licenses/>.
+*/
+#include "qetapptest.h"
+#include "diagramcontent.h"
+#include "qetgraphicsitem/diagramimageitem.h"
+
+#include <QDomDocument>
+#include <QRandomGenerator>
+
+/**
+	Diagram::fromXml() decodes a folio's pictures in parallel, then builds
+	the items in file order. Every picture must come back with its own
+	pixels and position, whatever thread decoded it.
+*/
+class tst_folioimageload : public QObject
+{
+	Q_OBJECT
+
+	Diagram *m_diagram = nullptr;
+
+	/// A picture no other one equals: its own size and noise.
+	static QImage distinctImage(int n)
+	{
+		QImage image(40 + 7 * n, 30 + 3 * n, QImage::Format_ARGB32);
+		QRandomGenerator random(quint32(n + 1));
+		for (int y = 0 ; y < image.height() ; ++y)
+			for (int x = 0 ; x < image.width() ; ++x)
+				image.setPixel(x, y, random.generate() | 0xFF000000);
+		return image;
+	}
+
+	static QImage pixelsOf(const DiagramImageItem *item)
+	{
+		return item->pixmap().toImage()
+				.convertToFormat(QImage::Format_ARGB32);
+	}
+
+	void removePictures()
+	{
+			// A selected picture's handles are its children, in items()
+			// too: deleting the picture first would leave them dangling.
+		m_diagram->clearSelection();
+		for (QGraphicsItem *item : m_diagram->items())
+			if (item->type() == DiagramImageItem::Type)
+				delete item;
+	}
+
+private slots:
+	void initTestCase()
+	{
+		QVERIFY(QetAppTest::editor());
+		m_diagram = QetAppTest::diagram();
+		QVERIFY(m_diagram);
+	}
+
+	void cleanup()
+	{
+		m_diagram->clearSelection();
+		removePictures();
+		m_diagram->undoStack().clear();
+	}
+
+	void everyPictureComesBackWithItsPixels()
+	{
+		QHash<int, QImage> expected;	// by x position, each its own
+		for (int n = 0 ; n < 6 ; ++n) {
+			const QImage image = distinctImage(n);
+			auto *picture = new DiagramImageItem(QPixmap::fromImage(image));
+			m_diagram->addItem(picture);
+			picture->setPos(100 + 150 * n, 200);
+			picture->setSelected(true);
+			expected.insert(qRound(picture->pos().x()), image);
+		}
+		QDomDocument document = m_diagram->toXml(false);
+		removePictures();
+
+		DiagramContent content;
+		QVERIFY(m_diagram->fromXml(document, QPointF(), false, &content));
+
+		QCOMPARE(content.m_images.size(), expected.size());
+		for (const DiagramImageItem *picture : std::as_const(content.m_images)) {
+			const int x = qRound(picture->pos().x());
+			QVERIFY2(expected.contains(x), "picture moved");
+			QCOMPARE(picture->pos().y(), 200.0);
+			QCOMPARE(pixelsOf(picture), expected.value(x));
+		}
+	}
+
+	void anImageWithoutPictureDoesNotStopTheOthers()
+	{
+		for (int n = 0 ; n < 3 ; ++n) {
+			auto *picture = new DiagramImageItem(
+						QPixmap::fromImage(distinctImage(n)));
+			m_diagram->addItem(picture);
+			picture->setSelected(true);
+		}
+		QDomDocument document = m_diagram->toXml(false);
+		removePictures();
+		QDomElement images = document.documentElement()
+				.firstChildElement(QStringLiteral("images"));
+		QVERIFY(!images.isNull());
+		QDomElement empty = document.createElement(QStringLiteral("image"));
+		images.insertBefore(empty, images.firstChild());
+
+		DiagramContent content;
+		QVERIFY(m_diagram->fromXml(document, QPointF(), false, &content));
+
+		int with_pixels = 0;
+		for (const DiagramImageItem *picture : std::as_const(content.m_images))
+			if (!picture->pixmap().isNull())
+				++with_pixels;
+		QCOMPARE(with_pixels, 3);
+	}
+};
+
+QET_APP_TEST_MAIN(tst_folioimageload)
+#include "tst_folioimageload.moc"