Merge pull request #1470 from ispyisail/perf/qetz-picture-load

Speed up opening .qetz projects that hold large pictures

sources/container/qetcontainer.cpp

@@ -155,13 +155,19 @@ bool QetContainer::isZip(const QByteArray &content)
 	The project document held in @p content: unzipped and joined for a
 	.qetz, parsed for a .qet.
 */
-bool QetContainer::readDocument(const QByteArray &content, QDomDocument *document, QString *error)
+bool QetContainer::readDocument(const QByteArray &content, QDomDocument *document,
+								QString *error, Pictures *pictures)
 {
 	if (!isZip(content))
 		return parse(content, document, error);
 	QList<QetZip::Entry> entries;
 	return QetZip::fromBytes(content, &entries, error)
-			&& join(entries, document, error);
+			&& join(entries, document, error, pictures);
+}
+
+QString QetContainer::pictureFile(const QDomElement &image)
+{
+	return image.attribute(ImageFileAttribute);
 }
 
 /**
@@ -264,8 +270,11 @@ QList<QetZip::Entry> QetContainer::split(const QDomDocument &project, const QStr
 	Refused, with @p error set: not a .qetz, made for a later QElectroTech
 	(the manifest's min-reader), or a part missing or unreadable. Entries
 	nothing names are ignored, so a later format may add some.
+	With @p pictures, the pictures stay out of the document: see the
+	namespace.
 */
-bool QetContainer::join(const QList<QetZip::Entry> &entries, QDomDocument *project, QString *error)
+bool QetContainer::join(const QList<QetZip::Entry> &entries, QDomDocument *project, QString *error,
+						Pictures *pictures)
 {
 	QHash<QString, QByteArray> files;
 	for (const QetZip::Entry &entry : entries)
@@ -310,7 +319,8 @@ bool QetContainer::join(const QList<QetZip::Entry> &entries, QDomDocument *proje
 		part.parentNode().replaceChild(project->importNode(document.documentElement(), true), part);
 	}
 
-		//Pictures back to base64, as the image's first child
+		//Pictures back to base64, as the image's first child, or into
+		//pictures
 	const QDomNodeList images = project->elementsByTagName(QStringLiteral("image"));
 	for (int i = 0 ; i < images.size() ; ++i)
 	{
@@ -321,6 +331,10 @@ bool QetContainer::join(const QList<QetZip::Entry> &entries, QDomDocument *proje
 			setError(error, QStringLiteral("picture \"%1\" is missing").arg(file));
 			return false;
 		}
+		if (pictures) {
+			pictures->insert(file, files.value(file));
+			continue;
+		}
 		image.removeAttribute(ImageFileAttribute);
 		image.insertBefore(project->createTextNode(QString::fromLatin1(files.value(file).toBase64())),
 						   image.firstChild());

sources/container/qetcontainer.h

@@ -21,6 +21,7 @@
 #include "qetzip.h"
 
 #include <QDomDocument>
+#include <QHash>
 
 /**
 	@brief The QetContainer namespace
@@ -41,10 +42,17 @@
 
 	join(split(d)) is d. Nothing here knows what the parts mean: the
 	project is read and written by the same code as a .qet file.
+
+	Given a Pictures map, join() leaves the pictures out of the document:
+	each <image> keeps the name of its file, pictureFile(), and the bytes
+	go in the map. Turning them back into base64 text costs seconds on a
+	project with large pictures, and the folio would only decode it again.
 */
 namespace QetContainer
 {
 	extern const char MimeType[];
+		/// Picture bytes by file name, see join()
+	using Pictures = QHash<QString, QByteArray>;
 		/// The format written; files of a later format are refused by join()
 		/// unless their min-reader allows this one.
 	constexpr int Format = 2;
@@ -53,14 +61,17 @@ namespace QetContainer
 							   const QString &writer = QString(),
 							   QString *error = nullptr);
 	bool join(const QList<QetZip::Entry> &entries, QDomDocument *project,
-			  QString *error = nullptr);
+			  QString *error = nullptr, Pictures *pictures = nullptr);
+		/// The file join() left the picture of @p image in, or an empty string
+	QString pictureFile(const QDomElement &image);
 
 		/// Parse @p xml as a .qet is read (QETProject::openFile)
 	bool parse(const QByteArray &xml, QDomDocument *document, QString *error = nullptr);
 		/// true if @p content is a zip, so a .qetz rather than a .qet
 	bool isZip(const QByteArray &content);
 		/// The project document in @p content, a .qet or a .qetz
-	bool readDocument(const QByteArray &content, QDomDocument *document, QString *error = nullptr);
+	bool readDocument(const QByteArray &content, QDomDocument *document,
+					  QString *error = nullptr, Pictures *pictures = nullptr);
 }
 
 #endif // QETCONTAINER_H

sources/diagram.cpp

@@ -49,6 +49,7 @@
 #include "diagramsortkeys.h"
 #include "itemgroups.h"
 #include "textgrid.h"
+#include "container/qetcontainer.h"
 #include "foliogrid.h"
 #include <QGraphicsView>
 #include <QTextStream>
@@ -2010,7 +2011,11 @@ bool Diagram::fromXml(QDomElement &document,
 												QStringLiteral("images"),
 												QStringLiteral("image"))) {
 		DiagramImageItem *dii = new DiagramImageItem ();
-		dii -> fromXml(image_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);
 		settle_uuid(dii, image_xml, QStringLiteral("image"), added_images.size());
 		ItemGroups::setGroup(dii, ItemGroups::read(image_xml));
 		addItem(dii);

sources/qetgraphicsitem/diagramimageitem.cpp

@@ -1787,6 +1787,17 @@ const QByteArray &DiagramImageItem::encodedPng(const QPixmap &pixmap, QByteArray
 	@return true if successfully loaded.
 */
 bool DiagramImageItem::fromXml(const QDomElement &e)
+{
+	return fromXml(e, QByteArray());
+}
+
+/**
+	@brief DiagramImageItem::fromXml
+	Load this image from xml element e, its picture being @p picture, not
+	the base64 text of @p e, unless @p picture is empty. A .qetz keeps its
+	pictures as files (QetContainer::join()).
+*/
+bool DiagramImageItem::fromXml(const QDomElement &e, const QByteArray &picture)
 {
 	if (e.tagName() != "image") {
 		return (false);
@@ -1796,28 +1807,27 @@ bool DiagramImageItem::fromXml(const QDomElement &e)
 	const QUuid uuid(e.attribute(QStringLiteral("uuid")));
 	if (!uuid.isNull() && uuid != m_uuid) setUuid(uuid);
 
-	QDomNode image_node = e.firstChild();
-	if (!image_node.isText()) {
-		return (false);
+	// 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());
 	}
 
-	//load xml image to QByteArray
-	QByteArray array;
-	array = QByteArray::fromBase64(e.text().toLatin1());
-
 	//Set QPixmap from the array
 	QPixmap pixmap;
 	pixmap.loadFromData(array);
 	setPixmap(pixmap);
-	// 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. loadFromData() stops at the end
-	// of the first PNG and never notices; a cache written back verbatim
-	// would.
-	const QByteArray ownArray = QByteArray::fromBase64(image_node.toText().data().toLatin1());
-	if (isPng(ownArray) && !pixmap_.isNull())
+	if (isPng(array) && !pixmap_.isNull())
 	{
-		m_png_cache = ownArray;
+		m_png_cache = array;
 		m_png_cache_key = pixmap_.cacheKey();
 	}
 

sources/qetgraphicsitem/diagramimageitem.h

@@ -120,6 +120,7 @@ class DiagramImageItem : public QetGraphicsItem {
 	int type() const override { return Type; }
 	
 	virtual bool fromXml(const QDomElement &);
+	bool fromXml(const QDomElement &, const QByteArray &picture);
 	virtual QDomElement toXml(QDomDocument &) const;
 	QUuid uuid() const {return m_uuid;}
 	void setUuid(const QUuid &uuid) {m_uuid = uuid; emit uuidChanged();}

sources/qetproject.cpp

@@ -278,6 +278,20 @@ QUuid QETProject::uuid() const
 	return m_uuid;
 }
 
+/**
+	@brief QETProject::containerPicture
+	The folio pictures of a .qetz are kept as bytes beside the document,
+	not as base64 text in it (QetContainer::join()), for as long as the
+	project is open: a folio built after opening still finds its pictures.
+	They cost almost no memory, sharing their bytes with the pictures
+	built from them.
+	@return the picture in @p file, or an empty array when there is none
+*/
+QByteArray QETProject::containerPicture(const QString &file) const
+{
+	return m_container_pictures.value(file);
+}
+
 /**
 	@brief QETProject::derivedItemUuid
 	A uuid for an item of this project that was saved without one, the same
@@ -456,9 +470,10 @@ QETProject::ProjectState QETProject::openFile(QFile *file)
 	// QetContainer::parse() does exactly that, for a .qet and for each part
 	// of a .qetz alike.
 	QString read_error;
-	if (!QetContainer::readDocument(content, &xml_project, &read_error))
+	if (!QetContainer::readDocument(content, &xml_project, &read_error, &m_container_pictures))
 	{
 		qWarning().noquote() << "Unable to read" << fi.fileName() << ":" << read_error;
+		m_container_pictures.clear();
 		if(opened_here) {
 			file->close();
 		}

sources/qetproject.h

@@ -119,6 +119,7 @@ class QETProject : public QObject
 		projectDataBase *dataBase();
 		QUuid uuid() const;
 		QUuid derivedItemUuid(const QString &kind, const QString &key);
+		QByteArray containerPicture(const QString &file) const;
 		ProjectState state() const;
 		QList<Diagram *> diagrams() const;
 			//The folios whether their items are built or not
@@ -448,6 +449,9 @@ class QETProject : public QObject
 		static bool m_backup_enabled;
 			/// Something changed since the last backup, see writeBackup()
 		bool m_backup_needed = true;
+			/// The pictures of the .qetz this project was opened from, see
+			/// containerPicture()
+		QHash<QString, QByteArray> m_container_pictures;
 			/// File path this project is saved to
 		QString m_file_path;
 			/// Current state of the project

tests/qttest/tst_qetcontainer.cpp

@@ -242,6 +242,43 @@ private slots:
 		QVERIFY(pictures < 121);   // the 121 placed pictures repeat a few files
 	}
 
+	// Given a map, join() hands the pictures over as bytes instead of
+	// putting them back in the document as base64
+	void picturesKeptAside()
+	{
+		QDomDocument original;
+		QList<QetZip::Entry> entries = splitFile(
+					QStringLiteral(QET_EXAMPLES_DIR "/weneedpolonez-Polonez_MR89_wiring_diagram.qet"), &original);
+		QVERIFY(!entries.isEmpty());
+		QHash<QString, QByteArray> files;
+		for (const QetZip::Entry &e : entries) files.insert(e.name, e.data);
+
+		QDomDocument back;
+		QetContainer::Pictures pictures;
+		QString error;
+		QVERIFY2(QetContainer::join(entries, &back, &error, &pictures), qPrintable(error));
+		const QDomNodeList images = back.elementsByTagName(QStringLiteral("image"));
+		const QDomNodeList original_images = original.elementsByTagName(QStringLiteral("image"));
+		QCOMPARE(images.size(), 121);
+		QCOMPARE(images.size(), original_images.size());
+		for (int i = 0 ; i < images.size() ; ++i) {
+			const QDomElement image = images.at(i).toElement();
+			const QString file = QetContainer::pictureFile(image);
+			QVERIFY(!file.isEmpty());
+			QVERIFY(!image.firstChild().isText());
+			QCOMPARE(pictures.value(file), files.value(file));
+			QCOMPARE(pictures.value(file),
+					 QByteArray::fromBase64(original_images.at(i).firstChild().toText().data().toLatin1()));
+		}
+
+		// still refused when a picture is missing
+		for (int i = 0 ; i < entries.size() ; ++i)
+			if (entries.at(i).name.startsWith(QStringLiteral("images/"))) { entries.removeAt(i); break; }
+		QDomDocument broken;
+		QVERIFY(!QetContainer::join(entries, &broken, &error, &pictures));
+		QVERIFY2(error.contains(QStringLiteral("missing")), qPrintable(error));
+	}
+
 	// A file whose manifest needs a later reader is refused, not half read
 	void laterFormatRefused()
 	{