State why the image can't be edited in the status bar

E.g. for indexed 8-bit images. We could do this upon opening the image by displaying a message box, but message boxes are annoying. This change uses the same label in the status bar to explain why the current layer can't be edited (because it's not visible). Fixes #2
mitchcurtis · May 14, 2021 · 36053be · 36053be
1 parent 0cdc8e5
commit 36053be
Show file tree

Hide file tree

Showing 10 changed files with 148 additions and 38 deletions.
diff --git a/app/qml/qml.qbs b/app/qml/qml.qbs
@@ -60,6 +60,7 @@ Group {
             "ui/NoteDialog.qml",
             "ui/OpacityDialog.qml",
             "ui/OptionsDialog.qml",
+            "ui/OptionalToolSeparator.qml",
             "ui/Panel.qml",
             "ui/ProjectTemplateButton.qml",
             "ui/RenameSwatchColourDialog.qml",

diff --git a/app/qml/ui/OptionalToolSeparator.qml b/app/qml/ui/OptionalToolSeparator.qml
@@ -0,0 +1,54 @@
+/*
+    Copyright 2021, Mitch Curtis
+
+    This file is part of Slate.
+
+    Slate 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 3 of the License, or
+    (at your option) any later version.
+
+    Slate 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 Slate. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+import QtQuick.Controls 2.12
+import QtQuick.Layouts 1.3
+
+
+/*
+    It's likely that ToolSeparator is the tallest item in the
+    StatusBar. If all separators were completely hidden (and not laid
+    out), the size of the status bar would jump around when the various
+    items are shown/hidden. Initially we were solving this by always
+    showing the first separator, but setting its opacity to 0 if it
+    wouldn't be shown. This works, but requires that every separator to
+    the right of it keep track of the visibility of the ones before it
+    (so that we didn't end up with two separators next to each other).
+
+    Another solution would be to give an item that's always visible
+    (like pointerIconLabel) a minimumHeight that matches the height
+    of ToolSeparator, but that would have to be done for each style,
+    and isn't that nice.
+
+    So, to avoid having to hard-code sizes and have complex visible
+    bindings, we do the opacity trick for each separator, and also
+    reduce its width to 0 so it doesn't take up any room.
+*/
+
+ToolSeparator {
+    topPadding: 0
+    bottomPadding: 0
+    opacity: shown ? 1 : 0
+
+    property bool shown
+
+    Layout.maximumWidth: shown ? implicitWidth : 0
+    Layout.fillHeight: true
+    Layout.maximumHeight: 24
+}
diff --git a/app/qml/ui/StatusBar.qml b/app/qml/ui/StatusBar.qml
@@ -54,6 +54,10 @@ Pane {
         width: canvas ? canvas.firstPane.size * canvas.width - statusBarPane.padding * 2 : parent.width
         visible: project && canvas && project.loaded
         anchors.verticalCenter: parent.verticalCenter
+        // Ideally we would set spacing here to avoid having to set margins in individual items,
+        // but since we use the zero-width trick to ensure the height of the status bar doesn't jump around,
+        // we run into https://bugreports.qt.io/browse/QTBUG-93765.
+        spacing: 0
 
         Label {
             id: pointerIconLabel
@@ -64,7 +68,6 @@ Pane {
 
             Layout.preferredWidth: Math.max(26, implicitWidth)
         }
-
         Label {
             id: cursorPixelPosLabel
             objectName: "cursorPixelPosLabel"
@@ -90,27 +93,14 @@ Pane {
             }
         }
 
-        ToolSeparator {
-            padding: 0
-            // Use opacity rather than visible, as it's an easy way of ensuring that the RowLayout
-            // always has a minimum height equal to the tallest item (assuming that that's us)
-            // and hence doesn't jump around when we become hidden.
-            // There's always at least one label and icon visible at all times (cursor pos),
-            // so we don't have to worry about those.
-            opacity: (fpsCounter.visible || lineLengthLabel.visible || selectionSizeLabel.visible) ? 1 : 0
-
-            Layout.fillHeight: true
-            Layout.maximumHeight: 24
+        OptionalToolSeparator {
+            shown: selectionIcon.visible
         }
-
         Image {
             id: selectionIcon
             source: "qrc:/images/selection.png"
             visible: canvas && canvas.tool === ImageCanvas.SelectionTool
-
-            Layout.rightMargin: 6
         }
-
         Label {
             id: selectionSizeLabel
             objectName: "selectionSizeLabel"
@@ -132,7 +122,11 @@ Pane {
             }
         }
 
+        OptionalToolSeparator {
+            shown: lineLengthLabel.visible
+        }
         Rectangle {
+            objectName: "lineLengthIcon"
             implicitWidth: 16
             implicitHeight: 1
             visible: canvas && canvas.lineVisible
@@ -150,7 +144,6 @@ Pane {
                 anchors.right: parent.right
             }
         }
-
         Label {
             id: lineLengthLabel
             objectName: "lineLengthLabel"
@@ -159,6 +152,7 @@ Pane {
 
             Layout.minimumWidth: lineLengthMaxTextMetrics.width
             Layout.maximumWidth: lineLengthMaxTextMetrics.width
+            Layout.leftMargin: 5
 
             TextMetrics {
                 id: lineLengthMaxTextMetrics
@@ -167,6 +161,9 @@ Pane {
             }
         }
 
+        OptionalToolSeparator {
+            shown: lineAngleLabel.visible
+        }
         Canvas {
             implicitWidth: 16
             implicitHeight: 16
@@ -192,7 +189,6 @@ Pane {
                 ctx.closePath();
             }
         }
-
         Label {
             id: lineAngleLabel
             objectName: "lineAngleLabel"
@@ -201,6 +197,7 @@ Pane {
 
             Layout.minimumWidth: lineAngleMaxTextMetrics.width
             Layout.maximumWidth: lineAngleMaxTextMetrics.width
+            Layout.leftMargin: 5
 
             TextMetrics {
                 id: lineAngleMaxTextMetrics
@@ -209,14 +206,9 @@ Pane {
             }
         }
 
-        ToolSeparator {
-            padding: 0
-            visible: fpsCounter.visible && (lineLengthLabel.visible || selectionSizeLabel.visible)
-
-            Layout.fillHeight: true
-            Layout.maximumHeight: 24
+        OptionalToolSeparator {
+            shown: fpsCounter.visible
         }
-
         FpsCounter {
             id: fpsCounter
             visible: settings.fpsVisible
@@ -231,15 +223,9 @@ Pane {
             }
         }
 
-        ToolSeparator {
-            padding: 0
-            visible: currentLayerNameLabel.visible
-                && (fpsCounter.visible || lineLengthLabel.visible || selectionSizeLabel.visible)
-
-            Layout.fillHeight: true
-            Layout.maximumHeight: 24
+        OptionalToolSeparator {
+            shown: currentLayerNameLabel.visible
         }
-
         Image {
             source: "qrc:/images/current-layer.png"
             visible: currentLayerNameLabel.visible
@@ -251,6 +237,23 @@ Pane {
             visible: settings.showCurrentLayerInStatusBar
                 && project && project.type === Project.LayeredImageType && project.currentLayer
 
+            Layout.leftMargin: 3
+        }
+
+        OptionalToolSeparator {
+            shown: toolsForbiddenReasonLabel.visible
+        }
+        Label {
+            text: "\uf06a"
+            font.family: "FontAwesome"
+            visible: toolsForbiddenReasonLabel.visible
+        }
+        Label {
+            id: toolsForbiddenReasonLabel
+            objectName: "toolsForbiddenReasonLabel"
+            text: canvas ? canvas.toolsForbiddenReason : ""
+            visible: canvas && canvas.toolsForbidden
+
             Layout.minimumWidth: lineAngleMaxTextMetrics.width
             Layout.maximumWidth: lineAngleMaxTextMetrics.width
         }

diff --git a/lib/imagecanvas.cpp b/lib/imagecanvas.cpp
@@ -1850,6 +1850,8 @@ void ImageCanvas::reset()
     // don't really need to be reset each time:
     // - tool
     // - toolSize
+    // TODO: ^ why?
+    mToolsForbiddenReason.clear();
 
     requestContentPaint();
 }
@@ -2284,6 +2286,7 @@ ImageCanvas::Tool ImageCanvas::penRightClickTool() const
 
 void ImageCanvas::applyCurrentTool()
 {
+    updateToolsForbidden();
     if (areToolsForbidden())
         return;
 
@@ -2599,6 +2602,8 @@ void ImageCanvas::updateWindowCursorShape()
     if (areGuidesVisible() && !areGuidesLocked() && !overRuler && !overNote)
         overGuide = guideIndexAtCursorPos() != -1;
 
+    updateToolsForbidden();
+
     // Hide the window's cursor when we're in the spotlight; otherwise, use the non-custom arrow cursor.
     const bool nothingOverUs = mProject->hasLoaded() && hasActiveFocus() /*&& !mModalPopupsOpen*/ && mContainsMouse;
     const bool splitterHovered = mSplitter.isEnabled() && mSplitter.isHovered();
@@ -2784,7 +2789,31 @@ void ImageCanvas::restoreToolBeforeAltPressed()
 
 bool ImageCanvas::areToolsForbidden() const
 {
-    return false;
+    return !mToolsForbiddenReason.isEmpty();
+}
+
+QString ImageCanvas::toolsForbiddenReason() const
+{
+    return mToolsForbiddenReason;
+}
+
+void ImageCanvas::updateToolsForbidden()
+{
+    // See: https://github.com/mitchcurtis/slate/issues/2.
+    // QPainter doesn't support drawing into an QImage whose format is QImage::Format_Indexed8.
+    static const QString index8Reason =
+        tr("Image cannot be edited because its format is indexed 8-bit, which does not support modification.");
+    const bool is8Bit = currentProjectImage()->format() == QImage::Format_Indexed8;
+    setToolsForbiddenReason(is8Bit ? index8Reason : QString());
+}
+
+void ImageCanvas::setToolsForbiddenReason(const QString &reason)
+{
+    if (reason == mToolsForbiddenReason)
+        return;
+
+    mToolsForbiddenReason = reason;
+    emit toolsForbiddenChanged();
 }
 
 bool ImageCanvas::event(QEvent *event)

diff --git a/lib/imagecanvas.h b/lib/imagecanvas.h
@@ -85,6 +85,8 @@ class SLATE_EXPORT ImageCanvas : public QQuickItem
     Q_PROPERTY(Tool lastFillToolUsed READ lastFillToolUsed NOTIFY lastFillToolUsedChanged)
     Q_PROPERTY(int toolSize READ toolSize WRITE setToolSize NOTIFY toolSizeChanged)
     Q_PROPERTY(int maxToolSize READ maxToolSize CONSTANT)
+    Q_PROPERTY(bool toolsForbidden READ areToolsForbidden NOTIFY toolsForbiddenChanged FINAL)
+    Q_PROPERTY(QString toolsForbiddenReason READ toolsForbiddenReason NOTIFY toolsForbiddenChanged FINAL)
     Q_PROPERTY(ToolShape toolShape READ toolShape WRITE setToolShape NOTIFY toolShapeChanged)
     Q_PROPERTY(QColor penForegroundColour READ penForegroundColour WRITE setPenForegroundColour NOTIFY penForegroundColourChanged)
     Q_PROPERTY(QColor penBackgroundColour READ penBackgroundColour WRITE setPenBackgroundColour NOTIFY penBackgroundColourChanged)
@@ -231,6 +233,9 @@ class SLATE_EXPORT ImageCanvas : public QQuickItem
     void setToolSize(int toolSize);
     int maxToolSize() const;
 
+    bool areToolsForbidden() const;
+    QString toolsForbiddenReason() const;
+
     QColor penForegroundColour() const;
     void setPenForegroundColour(const QColor &penForegroundColour);
 
@@ -349,6 +354,7 @@ class SLATE_EXPORT ImageCanvas : public QQuickItem
     void toolShapeChanged();
     void lastFillToolUsedChanged();
     void toolSizeChanged();
+    void toolsForbiddenChanged();
     void penForegroundColourChanged();
     void penBackgroundColourChanged();
     void hasBlankCursorChanged();
@@ -475,7 +481,8 @@ protected slots:
     void setPenColourThroughEyedropper(const QColor &colour);
     void setHasBlankCursor(bool hasBlankCursor);
     void restoreToolBeforeAltPressed();
-    virtual bool areToolsForbidden() const;
+    virtual void updateToolsForbidden();
+    void setToolsForbiddenReason(const QString &reason);
     void setCursorPixelColour(const QColor &cursorPixelColour);
     bool isWithinImage(const QPoint &scenePos) const;
     QPoint clampToImageBounds(const QPoint &scenePos, bool inclusive = true) const;
@@ -643,6 +650,7 @@ protected slots:
     Tool mLastFillToolUsed;
     int mToolSize;
     int mMaxToolSize;
+    QString mToolsForbiddenReason;
     QColor mPenForegroundColour;
     QColor mPenBackgroundColour;
 

diff --git a/lib/layeredimagecanvas.cpp b/lib/layeredimagecanvas.cpp
@@ -185,10 +185,11 @@ void LayeredImageCanvas::replaceImage(int layerIndex, const QImage &replacementI
     requestContentPaint();
 }
 
-bool LayeredImageCanvas::areToolsForbidden() const
+void LayeredImageCanvas::updateToolsForbidden()
 {
     // For layered image projects, tools cannot be used on the current layer
     // while it is hidden.
+    static const QString layerHiddenReason = tr("This layer cannot be edited because it is hidden.");
     ImageLayer *currentLayer = mLayeredImageProject->currentLayer();
-    return currentLayer && !currentLayer->isVisible();
+    setToolsForbiddenReason(currentLayer && !currentLayer->isVisible() ? layerHiddenReason : QString());
 }
diff --git a/lib/layeredimagecanvas.h b/lib/layeredimagecanvas.h
@@ -60,7 +60,7 @@ protected slots:
 
     void replaceImage(int layerIndex, const QImage &replacementImage) override;
 
-    bool areToolsForbidden() const override;
+    void updateToolsForbidden() override;
 
 private:
     LayeredImageProject *mLayeredImageProject;

diff --git a/tests/auto/resources.qrc b/tests/auto/resources.qrc
@@ -39,5 +39,6 @@
         <file>resources/version-check-v0.9.0.slp</file>
         <file>resources/animationFrameWidthTooLarge.slp</file>
         <file>resources/simple-colour-animation.slp</file>
+        <file>resources/indexed-8-format.png</file>
     </qresource>
 </RCC>
diff --git a/tests/auto/resources/indexed-8-format.png b/tests/auto/resources/indexed-8-format.png
diff --git a/tests/auto/tst_app.cpp b/tests/auto/tst_app.cpp
@@ -135,6 +135,7 @@ private Q_SLOTS:
     void penToolRightClickBehaviour_data();
     void penToolRightClickBehaviour();
     void splitScreenRendering();
+    void formatNotWritable();
 
     // Rulers, guides, notes, etc.
     void rulersAndGuides_data();
@@ -3272,6 +3273,18 @@ void tst_App::splitScreenRendering()
     QCOMPARE(canvasGrab.pixelColor(layeredImageCanvas->width() * 0.75, layeredImageCanvas->height() / 2), QColor(Qt::white));
 }
 
+// Distinct from a read-only file, this test checks that the UI prevents images with formats like Format_Indexed8
+// from being modified, as QPainter doesn't support it.
+void tst_App::formatNotWritable()
+{
+    QVERIFY2(setupTempProjectDir(), failureMessage);
+    QVERIFY2(copyFileFromResourcesToTempProjectDir("indexed-8-format.png"), failureMessage);
+
+    // Load the image.
+    const QUrl projectUrl = QUrl::fromLocalFile(tempProjectDir->path() + QLatin1String("/indexed-8-format.png"));
+    QVERIFY2(loadProject(projectUrl), failureMessage);
+}
+
 void tst_App::rulersAndGuides_data()
 {
     addAllProjectTypes();