From be48b59e8f6e03853a7d62d906e031f961c1a8c5 Mon Sep 17 00:00:00 2001 From: Beat Hangartner Date: Wed, 7 Oct 2026 11:33:22 +0200 Subject: [PATCH] Drop a picture's stale crop when loading a project Before #1310, undoing a crop left the crop rectangle in place, so a project saved afterwards has a but shows the uncropped picture. Since #1368 the picture shown is computed from the original and the crop, so the first mirror or colour key of such a picture applied the old crop again, and undoing that edit showed the cropped picture. The picture shown is always exactly the size of the crop. fromXml() now drops a crop that does not match it: the crop becomes the whole original, and when the shown picture is not the original's size either, the shown picture becomes the original. Valid files load and save as before. New tst_imagestalecrop: a valid crop is kept; a stale one is dropped for a shown picture of the original's size and of another size, where a later crop cuts the picture that was shown. Co-Authored-By: Claude Opus 5.5 --- sources/qetgraphicsitem/diagramimageitem.cpp | 12 ++ tests/qttest/CMakeLists.txt | 14 ++ tests/qttest/tst_imagestalecrop.cpp | 196 +++++++++++++++++++ 3 files changed, 222 insertions(+) create mode 100644 tests/qttest/tst_imagestalecrop.cpp diff --git a/sources/qetgraphicsitem/diagramimageitem.cpp b/sources/qetgraphicsitem/diagramimageitem.cpp index e2dc0f5ce..da8931748 100644 --- a/sources/qetgraphicsitem/diagramimageitem.cpp +++ b/sources/qetgraphicsitem/diagramimageitem.cpp @@ -1595,6 +1595,18 @@ bool DiagramImageItem::fromXml(const QDomElement &e) m_crop_rect = m_crop_rect.intersected(m_base_pixmap.rect()); if (m_crop_rect.isEmpty()) m_crop_rect = m_base_pixmap.rect(); + + // The picture shown is always exactly the crop's size. When it is + // not, the crop is stale: older versions saved it after the crop + // was undone (fixed in #1310), with the uncropped picture shown. + // The next edit would apply that crop again, and undoing the edit + // would show the cropped picture. Keep what was shown instead. + if (hasCrop && pixmap.size() != m_crop_rect.size()) + { + if (pixmap.size() != m_base_pixmap.size()) + m_base_pixmap = pixmap; + m_crop_rect = m_base_pixmap.rect(); + } } // Baseline: the plain "rotation"/"size" attributes, understood by diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index bd8a82c01..5d7e8d30f 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -894,6 +894,20 @@ if(QET_HAS_SCRIPTING) "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") endif() +# A picture whose saved crop does not match the picture shown (an undone +# crop, saved before #1310) loads without that crop; a valid crop is kept. +if(QET_HAS_SCRIPTING) + add_executable( + tst_imagestalecrop + tst_imagestalecrop.cpp) + add_test(NAME tst_imagestalecrop COMMAND tst_imagestalecrop) + add_dependencies(tst_imagestalecrop qelectrotech) + target_link_libraries(tst_imagestalecrop PRIVATE Qt::Test Qt::Gui) + target_compile_definitions(tst_imagestalecrop PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") +endif() + # Saving a project that was just saved changes nothing: runs the real # binary's --resave twice on every project in examples/, and on one whose # title block holds a single-space value (#973). diff --git a/tests/qttest/tst_imagestalecrop.cpp b/tests/qttest/tst_imagestalecrop.cpp new file mode 100644 index 000000000..d2f889493 --- /dev/null +++ b/tests/qttest/tst_imagestalecrop.cpp @@ -0,0 +1,196 @@ +/* + 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 +#include +#include +#include +#include +#include +#include +#include +#include + +/** + Before #1310, a project saved after a crop was undone kept the undone + , with the uncropped picture shown. Loading such a file has to + drop that crop: the picture shown is always exactly the crop's size, + so a picture of another size means the crop is stale. Otherwise the + next edit of the picture applies the crop again. + + Runs the real binary: a script adds and crops a picture and saves; + the test then puts another picture where the shown one is, and lets + the binary load and save that file again. +*/ +class tst_imagestalecrop : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + + // Runs the binary on @p project with a script that saves it to @p out. + bool run(const QString &script, const QString &project) + { + const QString path = m_dir.filePath(QStringLiteral("probe.js")); + const QString home = m_dir.filePath(QStringLiteral("home")); + QDir().mkpath(home); + QFile f(path); + if (!f.open(QIODevice::WriteOnly)) return false; + f.write(script.toUtf8()); + f.close(); + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen")); + env.insert(QStringLiteral("QET_ENABLE_SCRIPTING"), QStringLiteral("1")); + env.insert(QStringLiteral("HOME"), home); + env.insert(QStringLiteral("XDG_CONFIG_HOME"), home + QStringLiteral("/config")); + env.insert(QStringLiteral("XDG_DATA_HOME"), home + QStringLiteral("/data")); + env.insert(QStringLiteral("TMPDIR"), m_dir.path()); + QProcess proc; + proc.setProcessEnvironment(env); + proc.start(QStringLiteral(QET_TEST_BINARY_PATH), + {QStringLiteral("--run"), path, project}); + return proc.waitForFinished(120000) && proc.exitStatus() == QProcess::NormalExit; + } + + // Opens @p in and saves it to @p out. + bool resave(const QString &in, const QString &out) + { + return run(QStringLiteral("qet.save('%1');\n").arg(out), in) && QFile::exists(out); + } + + static QByteArray read(const QString &path) + { + QFile f(path); + return f.open(QIODevice::ReadOnly) ? f.readAll() : QByteArray(); + } + + static QByteArray png(const QSize &size) + { + QImage img(size, QImage::Format_RGB32); + img.fill(Qt::darkGreen); + QByteArray bytes; + QBuffer buffer(&bytes); + buffer.open(QIODevice::WriteOnly); + img.save(&buffer, "PNG"); + return bytes.toBase64(); + } + + // The size of the saved original (), or an empty size. + static QSize baseSize(const QByteArray &xml) + { + const QRegularExpression base(QStringLiteral("\\s*([A-Za-z0-9+/=]+)")); + const QRegularExpressionMatch m = base.match(QString::fromLatin1(xml)); + if (!m.hasMatch()) return {}; + return QImage::fromData(QByteArray::fromBase64(m.captured(1).toLatin1())).size(); + } + + // @p xml with the shown picture (the element's first text) + // replaced by a PNG of @p size. + static QByteArray withShown(QByteArray xml, const QSize &size) + { + const QRegularExpression shown(QStringLiteral("(]*>\\s*)([A-Za-z0-9+/=]+)")); + const QRegularExpressionMatch m = shown.match(QString::fromLatin1(xml)); + if (!m.hasMatch()) return {}; + return xml.replace(m.capturedStart(2), m.capturedLength(2), png(size)); + } + + QString m_cropped; // a picture of 40 x 30, cropped to 10,5,20,10 + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + QImage img(40, 30, QImage::Format_RGB32); + img.fill(Qt::darkGreen); + const QString pic = m_dir.filePath(QStringLiteral("pic.png")); + QVERIFY(img.save(pic)); + + m_cropped = m_dir.filePath(QStringLiteral("cropped.qet")); + QVERIFY(run(QStringLiteral(R"JS( +var i = qet.addImage(0, '%1', 100, 100); +qet.cropImage(0, i, 10, 5, 20, 10); +qet.save('%2'); +)JS").arg(pic, m_cropped), QStringLiteral(QET_EXAMPLES_DIR "/741.qet"))); + const QByteArray xml = read(m_cropped); + QCOMPARE(xml.count("")); + } + + // A valid crop is kept, byte for byte. + void validCropIsKept() + { + const QString out = m_dir.filePath(QStringLiteral("valid-out.qet")); + QVERIFY(resave(m_cropped, out)); + const QByteArray xml = read(out); + // Attributes in any order: Qt before 6.5 writes them unordered. + const QRegularExpression crop(QStringLiteral("]*\\bx=\"10\")(?=[^>]*\\by=\"5\")" + "(?=[^>]*\\bw=\"20\")(?=[^>]*\\bh=\"10\")")); + QVERIFY2(crop.match(QString::fromLatin1(xml)).hasMatch(), "the valid crop was lost"); + QVERIFY(xml.contains("")); + } + + // The shown picture is the whole original (40 x 30): the crop was + // undone before the file was saved. It is dropped, and with nothing + // else to remember the original is not saved twice any more. + void staleCropOfWholeOriginalIsDropped() + { + const QString stale = m_dir.filePath(QStringLiteral("stale.qet")); + const QByteArray xml = withShown(read(m_cropped), QSize(40, 30)); + QVERIFY(!xml.isEmpty()); + QFile f(stale); + QVERIFY(f.open(QIODevice::WriteOnly)); + f.write(xml); + f.close(); + + const QString out = m_dir.filePath(QStringLiteral("stale-out.qet")); + QVERIFY(resave(stale, out)); + const QByteArray saved = read(out); + QCOMPARE(saved.count("")); + } + + // The shown picture matches neither the crop nor the original: it is + // what the user saw, so it becomes the original -- a crop made after + // loading cuts it, not the old original. + void shownPictureOfOtherSizeBecomesTheOriginal() + { + const QString odd = m_dir.filePath(QStringLiteral("odd.qet")); + const QByteArray xml = withShown(read(m_cropped), QSize(25, 25)); + QVERIFY(!xml.isEmpty()); + QFile f(odd); + QVERIFY(f.open(QIODevice::WriteOnly)); + f.write(xml); + f.close(); + + const QString out = m_dir.filePath(QStringLiteral("odd-out.qet")); + QVERIFY(resave(odd, out)); + const QByteArray saved = read(out); + QVERIFY2(!saved.contains("")); + + const QString recropped = m_dir.filePath(QStringLiteral("odd-recropped.qet")); + QVERIFY(run(QStringLiteral("qet.cropImage(0, 0, 0, 0, 10, 10);\nqet.save('%1');\n") + .arg(recropped), odd)); + QCOMPARE(baseSize(read(recropped)), QSize(25, 25)); + } +}; + +QTEST_MAIN(tst_imagestalecrop) +#include "tst_imagestalecrop.moc"