diff --git a/sources/qetgraphicsitem/diagramimageitem.cpp b/sources/qetgraphicsitem/diagramimageitem.cpp index ac82aa5bd..00f8b030b 100644 --- a/sources/qetgraphicsitem/diagramimageitem.cpp +++ b/sources/qetgraphicsitem/diagramimageitem.cpp @@ -194,6 +194,30 @@ void DiagramImageItem::setPixmap(const QPixmap &pixmap) { emit pixmapChanged(); } +/** + @brief DiagramImageItem::setImageSource + Set the picture's source -- original, crop rectangle and transparent + colours. Only stores them: the displayed pixmap is a property of its + own, set by the same undo command. +*/ +void DiagramImageItem::setImageSource(const ImageSource &source) +{ + m_base_pixmap = source.base; + m_crop_rect = source.crop; + m_transparent_colors = source.colors; +} + +QVariant DiagramImageItem::imageSourceVariant() const +{ + return QVariant::fromValue(imageSource()); +} + +void DiagramImageItem::setImageSourceVariant(const QVariant &source) +{ + if (source.canConvert()) + setImageSource(source.value()); +} + /** @brief DiagramImageItem::setScaleFactorX / setScaleFactorY / setRotationAngle Matching QetShapeItem's own setters for the identical fields -- @@ -1814,21 +1838,13 @@ void DiagramImageItem::replaceImage() // or region was cropped for the old one, so this starts that // memory fresh rather than carrying over choices that would no // longer make sense. - m_base_pixmap = newPixmap; - m_crop_rect = newPixmap.rect(); - m_transparent_colors.clear(); + const ImageSource oldSource = imageSource(); + const ImageSource newSource{newPixmap, newPixmap.rect(), {}}; auto *undo = new QPropertyUndoCommand(this, "pixmap", oldPixmap, newPixmap); undo->setText(tr("Remplacer une image")); - // Every call here is a separate, deliberate menu action with no - // compound child of its own (unlike crop(), which always chains a - // pos/rawPivot change and is naturally immune) -- two of them in a - // row would carry the exact same object, property, and text, which - // is indistinguishable from a legitimate merge to - // QPropertyUndoCommand::mergeWith(). A dummy child (already treated - // as "never merge" by that check) keeps each one its own, separate - // undo step regardless. - new QUndoCommand(undo); + new QPropertyUndoCommand(this, "imageSource", QVariant::fromValue(oldSource), + QVariant::fromValue(newSource), undo); diagram()->undoStack().push(undo); } @@ -1854,7 +1870,9 @@ void DiagramImageItem::mirror(bool horizontal) // colour values don't change when the image is mirrored, only their // positions, so whatever was already keyed transparent should stay // remembered and still apply correctly to the flipped version. - m_base_pixmap = m_base_pixmap.transformed(flip); + const ImageSource oldSource = imageSource(); + ImageSource newSource = oldSource; + newSource.base = m_base_pixmap.transformed(flip); // m_crop_rect, unlike the colour list, DOES need to change: it's // defined in terms of positions within the base, and those @@ -1864,19 +1882,16 @@ void DiagramImageItem::mirror(bool horizontal) // whether this reads m_base_pixmap's size from before or after the // assignment above). if (horizontal) - m_crop_rect = QRect(m_base_pixmap.width() - m_crop_rect.left() - m_crop_rect.width(), + newSource.crop = QRect(m_base_pixmap.width() - m_crop_rect.left() - m_crop_rect.width(), m_crop_rect.top(), m_crop_rect.width(), m_crop_rect.height()); else - m_crop_rect = QRect(m_crop_rect.left(), m_base_pixmap.height() - m_crop_rect.top() - m_crop_rect.height(), + newSource.crop = QRect(m_crop_rect.left(), m_base_pixmap.height() - m_crop_rect.top() - m_crop_rect.height(), m_crop_rect.width(), m_crop_rect.height()); auto *undo = new QPropertyUndoCommand(this, "pixmap", oldPixmap, newPixmap); undo->setText(horizontal ? tr("Miroir horizontal d'une image") : tr("Miroir vertical d'une image")); - // See replaceImage()'s identical comment: a separate, deliberate - // action with no compound child of its own, so a dummy one is - // needed to stop two consecutive same-direction mirrors (identical - // object, property, and text) from silently merging into one. - new QUndoCommand(undo); + new QPropertyUndoCommand(this, "imageSource", QVariant::fromValue(oldSource), + QVariant::fromValue(newSource), undo); diagram()->undoStack().push(undo); } @@ -1919,41 +1934,20 @@ void DiagramImageItem::setTransparentColor() if (dialog.exec() != QDialog::Accepted) return; - m_transparent_colors = dialog.pickedColors(); + const ImageSource oldSource = imageSource(); + ImageSource newSource = oldSource; + newSource.colors = dialog.pickedColors(); const QPixmap oldPixmap = pixmap_; const QPixmap newPixmap = dialog.resultPixmap(); auto *undo = new QPropertyUndoCommand(this, "pixmap", oldPixmap, newPixmap); undo->setText(tr("Définir une couleur transparente")); - // See replaceImage()'s identical comment: a separate, deliberate - // action with no compound child of its own, so a dummy one is - // needed to stop two consecutive transparency edits (identical - // object, property, and text) from silently merging into one. - new QUndoCommand(undo); + new QPropertyUndoCommand(this, "imageSource", QVariant::fromValue(oldSource), + QVariant::fromValue(newSource), undo); diagram()->undoStack().push(undo); } -/** - @brief DiagramImageItem::crop - Context-menu action: opens ImageCropDialog against pixmap_ (the - current, already colour-keyed display, so cropping is WYSIWYG - against whatever is actually visible), then applies the chosen - rectangle to both pixmap_ and m_base_pixmap together -- kept in - sync the same way mirror() keeps them in sync, since cropping is a - permanent, geometric change to the image's own content, unlike - setTransparentColor()'s non-destructive colour keying. - - pos() also needs adjusting, not just pixmap_: setPixmap() (called - via the "pixmap" undo command below) recomputes - transformOriginPoint() from the new, smaller boundingRect(), but - pos() itself is untouched by that -- without fixing it up here too, - the surviving content would visually jump to wherever local (0,0) - happens to land after shrinking, rather than staying exactly where - it already was. Chained into one undo step together with the pixmap - change, since undoing a crop has to restore both, or the restored - (larger) image ends up in the wrong place. -*/ /** @brief DiagramImageItem::crop Context-menu action: opens ImageCropDialog against m_base_pixmap @@ -1962,27 +1956,7 @@ void DiagramImageItem::setTransparentColor() destructive: nothing about the original content is ever discarded, only which region of it is currently being shown, exactly the same principle setTransparentColor() already follows for its own choices. - Recomputes pixmap_ via computeDisplayPixmap() so any already-picked - transparent colours are correctly re-applied to the newly-cropped - region, rather than lost (the crop dialog itself knows nothing - about them). - - pos() also needs adjusting, not just pixmap_: setPixmap() (called - via the "pixmap" undo command below) recomputes - transformOriginPoint() from the new boundingRect(), but pos() itself - is untouched by that -- without fixing it up here too, the - surviving content would visually jump to wherever local (0,0) ends - up after the crop rect changes, rather than staying exactly where - it already was. This has to work whether this is the first crop - ever applied or an adjustment of an existing one, so the position - math is always done relative to the CURRENT crop rect (m_crop_rect, - before it's updated below) -- when there's no previous crop, that's - simply the whole base, which is what the very first version of this - method assumed unconditionally. - - Chained into one undo step together with the pixmap change, since - undoing a crop has to restore both, or the restored (larger) image - ends up in the wrong place. + The crop itself is done by applyCrop(). */ void DiagramImageItem::crop() { @@ -1994,9 +1968,41 @@ void DiagramImageItem::crop() if (dialog.exec() != QDialog::Accepted) return; - const QRect newCropRect = dialog.cropRect(); + applyCrop(dialog.cropRect()); +} + +/** + @brief DiagramImageItem::applyCrop + Show @a cropRect of the original (in the original's own pixels), + keeping the centre of the kept region where it is on the folio. + Recomputes pixmap_ via computeDisplayPixmap() so any already-picked + transparent colours are correctly re-applied to the newly-cropped + region, rather than lost. + + pos() also needs adjusting, not just pixmap_: setPixmap() (called + via the "pixmap" undo command below) recomputes + transformOriginPoint() from the new boundingRect(), but pos() itself + is untouched by that -- without fixing it up here too, the + surviving content would visually jump to wherever local (0,0) ends + up after the crop rect changes, rather than staying exactly where + it already was. This has to work whether this is the first crop + ever applied or an adjustment of an existing one, so the position + math is always done relative to the CURRENT crop rect (m_crop_rect, + before it's updated below) -- when there's no previous crop, that's + simply the whole base. + + One undo step, which restores the pixmap, the position, the pivot + and the crop rectangle together. + @return false if nothing was cropped: read-only folio, or a + rectangle that is empty, outside the original, or the current one. +*/ +bool DiagramImageItem::applyCrop(const QRect &cropRect) +{ + if (!diagram() || diagram()->isReadOnly()) + return false; + const QRect newCropRect = cropRect.intersected(m_base_pixmap.rect()); if (newCropRect.isEmpty() || newCropRect == m_crop_rect) - return; // nothing actually changed + return false; // nothing actually changed // newCropRect is in m_base_pixmap's own coordinates; converting its // center into the CURRENT local space (pixmap_'s own coordinates, @@ -2009,7 +2015,9 @@ void DiagramImageItem::crop() const QPixmap oldPixmap = pixmap_; const QPixmap newPixmap = computeDisplayPixmap(m_base_pixmap, newCropRect, m_transparent_colors); - m_crop_rect = newCropRect; + const ImageSource oldSource = imageSource(); + ImageSource newSource = oldSource; + newSource.crop = newCropRect; // boundingRect() is exactly QRectF(pixmap_.rect()) (confirmed by // reading the actual implementation, not assumed) -- so the new @@ -2027,6 +2035,9 @@ void DiagramImageItem::crop() undo->setText(tr("Rogner une image")); new QPropertyUndoCommand(this, "pos", oldPos, newPos, undo); new QPropertyUndoCommand(this, "rawPivot", oldPivot, newOriginPoint, undo); + new QPropertyUndoCommand(this, "imageSource", QVariant::fromValue(oldSource), + QVariant::fromValue(newSource), undo); m_pivotIsCustom = false; diagram()->undoStack().push(undo); + return true; } diff --git a/sources/qetgraphicsitem/diagramimageitem.h b/sources/qetgraphicsitem/diagramimageitem.h index 570d06770..b3263f9b3 100644 --- a/sources/qetgraphicsitem/diagramimageitem.h +++ b/sources/qetgraphicsitem/diagramimageitem.h @@ -49,6 +49,10 @@ class DiagramImageItem : public QetGraphicsItem { Q_PROPERTY(qreal skewY READ skewY WRITE setSkewY NOTIFY transformChanged) Q_PROPERTY(QPointF pivot READ pivot WRITE setPivot NOTIFY transformChanged) Q_PROPERTY(QString label READ label WRITE setLabel NOTIFY labelChanged) + // The picture's source -- original, crop rectangle, transparent + // colours -- as one value, so that every edit of it (crop, colour + // key, mirror, replace) is undone together with the displayed pixmap. + Q_PROPERTY(QVariant imageSource READ imageSourceVariant WRITE setImageSourceVariant) // A second, deliberately non-compensating property on the SAME // underlying value -- setPivot() (above) intentionally adjusts // pos() to keep the image visually in place, which is exactly @@ -65,6 +69,19 @@ class DiagramImageItem : public QetGraphicsItem { DiagramImageItem(QetGraphicsItem * = nullptr); DiagramImageItem(const QPixmap &pixmap, QetGraphicsItem * = nullptr); ~DiagramImageItem() override; + + struct ImageSource + { + QPixmap base; + QRect crop; + QList colors; + }; + ImageSource imageSource() const { return {m_base_pixmap, m_crop_rect, m_transparent_colors}; } + void setImageSource(const ImageSource &source); + QVariant imageSourceVariant() const; + void setImageSourceVariant(const QVariant &source); + QRect cropRect() const { return m_crop_rect; } + bool applyCrop(const QRect &cropRect); // attributes public: @@ -246,4 +263,6 @@ class DiagramImageItem : public QetGraphicsItem { QPointF m_label_scale{1.0, 1.0}; // scale the label rect was last computed for -- see updateLabelScale() bool m_resizeCenterAnchored = false; // decided once, at press time -- see handlerMousePressEvent()'s comment for why, mirroring the identical fix already made for shape creation }; +Q_DECLARE_METATYPE(DiagramImageItem::ImageSource) + #endif diff --git a/sources/scripting/qetscriptapi.cpp b/sources/scripting/qetscriptapi.cpp index ae16daba6..3ddbb537a 100644 --- a/sources/scripting/qetscriptapi.cpp +++ b/sources/scripting/qetscriptapi.cpp @@ -3475,6 +3475,44 @@ bool QetScriptApi::setImageRotation(int folioIndex, int imageIndex, double angle return true; } +/** + @brief QetScriptApi::cropImage + Show only the rectangle (x, y, width, height) of the image's original, + in the original's own pixels, as the crop tool does: one undo step, + the kept region staying where it is on the folio. + @return false if nothing was cropped: no such image, read-only + project, or a rectangle that is empty, outside the original, or the + current crop. +*/ +bool QetScriptApi::cropImage(int folioIndex, int imageIndex, int x, int y, int width, int height) +{ + if (!m_project) return false; + if (m_project->isReadOnly()) { + log(QStringLiteral("qet.cropImage: project is read-only")); + return false; + } + const QList list = sortedImages(folioIndex); + if (imageIndex < 0 || imageIndex >= list.count()) { + log(QStringLiteral("qet.cropImage: folio %1 has %2 image(s), no index %3") + .arg(folioIndex).arg(list.count()).arg(imageIndex)); + return false; + } + return list.at(imageIndex)->applyCrop(QRect(x, y, width, height)); +} + +/** + @brief QetScriptApi::imageCrop + @return the image's crop rectangle in its original's pixels, as + "x,y,width,height", or an empty string for no such image. +*/ +QString QetScriptApi::imageCrop(int folioIndex, int imageIndex) const +{ + const QList list = sortedImages(folioIndex); + if (imageIndex < 0 || imageIndex >= list.count()) return QString(); + const QRect r = list.at(imageIndex)->cropRect(); + return QStringLiteral("%1,%2,%3,%4").arg(r.x()).arg(r.y()).arg(r.width()).arg(r.height()); +} + bool QetScriptApi::deleteImage(int folioIndex, int imageIndex) { if (!m_project) return false; diff --git a/sources/scripting/qetscriptapi.h b/sources/scripting/qetscriptapi.h index b548e74ec..be9bdf061 100644 --- a/sources/scripting/qetscriptapi.h +++ b/sources/scripting/qetscriptapi.h @@ -540,6 +540,8 @@ class QetScriptApi : public QObject Q_INVOKABLE int addImage(int folioIndex, const QString &filePath, double x, double y); Q_INVOKABLE bool setImageScale(int folioIndex, int imageIndex, double factor); Q_INVOKABLE bool setImageRotation(int folioIndex, int imageIndex, double angle); + Q_INVOKABLE bool cropImage(int folioIndex, int imageIndex, int x, int y, int width, int height); + Q_INVOKABLE QString imageCrop(int folioIndex, int imageIndex) const; Q_INVOKABLE bool deleteImage(int folioIndex, int imageIndex); Q_INVOKABLE int addPdfPage(int folioIndex, const QString &pdfPath, int pageNumber, int dpi, double x, double y); diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 3b7e8c2b5..ec5c53a8b 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -593,6 +593,18 @@ if(QET_HAS_SCRIPTING) "QET_TEST_BINARY_PATH=\"$\"" "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") + # Undoing a crop of a picture restores its crop rectangle too, so a + # project saved after the undo does not record the undone crop. + add_executable( + tst_imagecropundo + tst_imagecropundo.cpp) + add_test(NAME tst_imagecropundo COMMAND tst_imagecropundo) + add_dependencies(tst_imagecropundo qelectrotech) + target_link_libraries(tst_imagecropundo PRIVATE Qt::Test Qt::Gui) + target_compile_definitions(tst_imagecropundo PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") + # The same symbol placed twice in a project whose collection does not # start with its "import" category (examples/lmdg.qet). add_executable( diff --git a/tests/qttest/tst_imagecropundo.cpp b/tests/qttest/tst_imagecropundo.cpp new file mode 100644 index 000000000..5ea009e82 --- /dev/null +++ b/tests/qttest/tst_imagecropundo.cpp @@ -0,0 +1,137 @@ +/* + 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 +#include + +/** + Undoing a crop restores the crop rectangle too, not only the pixels + shown: before, a project saved after Ctrl+Z still recorded the undone + crop, and the picture came back cropped once reopened. Runs the real + binary on a script: add a picture, crop it, undo, save; redo, save. +*/ +class tst_imagecropundo : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + + QJsonObject 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 {}; + 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}); + if (!proc.waitForFinished(120000)) return {}; + const QString out = QString::fromUtf8(proc.readAllStandardOutput() + + proc.readAllStandardError()); + const QString mark = QStringLiteral("PROBE "); + for (const QString &line : out.split(QLatin1Char('\n'))) { + const int i = line.indexOf(mark); + if (i >= 0) + return QJsonDocument::fromJson(line.mid(i + mark.size()).toUtf8()).object(); + } + return {}; + } + + static QByteArray read(const QString &path) + { + QFile f(path); + return f.open(QIODevice::ReadOnly) ? f.readAll() : QByteArray(); + } + +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); + QVERIFY(img.save(m_dir.filePath(QStringLiteral("pic.png")))); + } + + void undoneCropIsNotSaved() + { + const QString undone = m_dir.filePath(QStringLiteral("undone.qet")); + const QString redone = m_dir.filePath(QStringLiteral("redone.qet")); + const QString script = QStringLiteral(R"JS( +var i = qet.addImage(0, '%1', 100, 100); +var r = {full: qet.imageCrop(0, i)}; +r.cropped_ok = qet.cropImage(0, i, 10, 5, 20, 10); +r.cropped = qet.imageCrop(0, i); +r.same_ok = qet.cropImage(0, i, 10, 5, 20, 10); +r.empty_ok = qet.cropImage(0, i, 10, 5, 0, 10); +r.outside_ok = qet.cropImage(0, i, 100, 100, 20, 10); +qet.undo(); +r.undone = qet.imageCrop(0, i); +qet.save('%2'); +qet.redo(); +r.redone = qet.imageCrop(0, i); +qet.save('%3'); +qet.log('PROBE ' + JSON.stringify(r)); +)JS").arg(m_dir.filePath(QStringLiteral("pic.png")), undone, redone); + + const QJsonObject r = run(script, QStringLiteral(QET_EXAMPLES_DIR "/741.qet")); + QVERIFY2(!r.isEmpty(), "the script logged nothing"); + QCOMPARE(r.value("full").toString(), QStringLiteral("0,0,40,30")); + QVERIFY(r.value("cropped_ok").toBool()); + QCOMPARE(r.value("cropped").toString(), QStringLiteral("10,5,20,10")); + // Crops that change nothing report it, and push no undo step: + // the undo below still undoes the real crop. + QCOMPARE(r.value("same_ok").toBool(true), false); + QCOMPARE(r.value("empty_ok").toBool(true), false); + QCOMPARE(r.value("outside_ok").toBool(true), false); + QCOMPARE(r.value("undone").toString(), QStringLiteral("0,0,40,30")); + QCOMPARE(r.value("redone").toString(), QStringLiteral("10,5,20,10")); + + const QByteArray undone_xml = read(undone); + QVERIFY(undone_xml.contains("]*)/>")).match(QString::fromUtf8(read(redone))); + QVERIFY2(crop.hasMatch(), "the redone crop was not saved"); + for (const char *attribute : {R"(x="10")", R"(y="5")", R"(w="20")", R"(h="10")"}) + QVERIFY2(crop.captured(1).contains(QLatin1String(attribute)), attribute); + } +}; + +QTEST_GUILESS_MAIN(tst_imagecropundo) +#include "tst_imagecropundo.moc"