From 6751fc6f1998ada0ef2dce62e3413a00e348347e Mon Sep 17 00:00:00 2001 From: ispyisail Date: Wed, 14 Oct 2026 12:09:09 +1300 Subject: [PATCH] Speed up opening folios with photos: decode their pictures in parallel Each picture of a folio was decoded in turn while its item was built, a quarter of a second for a 12 megapixel photo. Diagram::fromXml() now reads every picture of the folio first, decodes them all at once on a pool of its own (one thread per core, at most one per picture), then builds the items in file order in the GUI thread as before. The decoding (base64 text, then the picture file) moves out of DiagramImageItem::fromXml() into decodePicture(), which touches only its argument. QImage::fromData() then QPixmap::fromImage() is what QPixmap::loadFromData() does, so the pixels are the same. On a 263 MB project with 22 photos on one folio: --info 21.3 s -> 12.5 s, the folio's content 10.6 s -> 3.2 s, opening its .qetz 15.6 s -> 9.3 s. Peak memory +220 MB (1.81 -> 2.03 GB), the pictures' text being converted by several threads at once. A resave is byte-identical to master's, and so is a PNG export. tst_folioimageload: six pictures come back with their own pixels and positions, and an without a picture does not stop the others. It fails when the decoded pictures are handed to the wrong items. Co-Authored-By: Claude Opus 5.5 --- sources/diagram.cpp | 41 ++++-- sources/qetgraphicsitem/diagramimageitem.cpp | 74 ++++++++--- sources/qetgraphicsitem/diagramimageitem.h | 13 ++ tests/qttest/CMakeLists.txt | 4 + tests/qttest/tst_folioimageload.cpp | 132 +++++++++++++++++++ 5 files changed, 240 insertions(+), 24 deletions(-) create mode 100644 tests/qttest/tst_folioimageload.cpp diff --git a/sources/diagram.cpp b/sources/diagram.cpp index 13a62e7af..d28d8a543 100644 --- a/sources/diagram.cpp +++ b/sources/diagram.cpp @@ -53,6 +53,8 @@ #include "foliogrid.h" #include #include +#include +#include #include #include #include @@ -2006,16 +2008,37 @@ bool Diagram::fromXml(QDomElement &document, } // Load image - QList 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 images_xml = QET::findInDomElement( + root, QStringLiteral("images"), QStringLiteral("image")); + QList 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(QThread::idealThreadCount(), + pictures.size())); + QtConcurrent::blockingMap(&pool, pictures, + &DiagramImageItem::decodePicture); + } else { + for (DiagramImageItem::Picture &picture : pictures) + DiagramImageItem::decodePicture(picture); + } + + QList 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); diff --git a/sources/qetgraphicsitem/diagramimageitem.cpp b/sources/qetgraphicsitem/diagramimageitem.cpp index 4c5f05145..bc5aefac1 100644 --- a/sources/qetgraphicsitem/diagramimageitem.cpp +++ b/sources/qetgraphicsitem/diagramimageitem.cpp @@ -1798,6 +1798,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 '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); @@ -1807,23 +1861,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 '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()) { diff --git a/sources/qetgraphicsitem/diagramimageitem.h b/sources/qetgraphicsitem/diagramimageitem.h index df8f77b1a..067bddb3c 100644 --- a/sources/qetgraphicsitem/diagramimageitem.h +++ b/sources/qetgraphicsitem/diagramimageitem.h @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -119,8 +120,20 @@ class DiagramImageItem : public QetGraphicsItem { */ int type() const override { return Type; } + /// A picture as read from its element, before and after + /// decodePicture(), which may run outside the GUI thread. + struct Picture { + QString base64; ///< text of , 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();} diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index b5be0cf96..3d6536558 100644 --- a/tests/qttest/CMakeLists.txt +++ b/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 with no picture does not stop the others. +qet_add_app_test(tst_folioimageload tst_folioimageload.cpp) + # TextResizeHandles: the corner handles that change the width of a text box, # on a plain QGraphicsTextItem with the "textWidth" property QET texts have. add_executable( diff --git a/tests/qttest/tst_folioimageload.cpp b/tests/qttest/tst_folioimageload.cpp new file mode 100644 index 000000000..15b7c2e6a --- /dev/null +++ b/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 . +*/ +#include "qetapptest.h" +#include "diagramcontent.h" +#include "qetgraphicsitem/diagramimageitem.h" + +#include +#include + +/** + 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 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"