summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMitch Curtis <mitch.curtis@qt.io>2019-10-01 15:09:01 +0200
committerMitch Curtis <mitch.curtis@qt.io>2019-10-02 17:25:43 +0200
commit788865b805bc91151ac8fe18bf7b92b1212ee07d (patch)
treef9f599863cfcd3e4e0598fb5cd3213e702b22daf
parentcc3ef6cd7af09f7410e29f2fe7e403e25a2768bc (diff)
Imagine: fix crash when switching between 9-patch and regular image
Consider the following changes in source: normal.png => press.9.png => normal.png => focus.png If the last two events happen quickly, pixmapChange() can be called twice with no call to updatePaintNode() inbetween. On the first call, resetNode will be true (because ninePatch is not null since it is still in the process of going from a 9-patch image to a regular image), and on the second call, resetNode would be false if we didn't have this check. This results in the oldNode never being deleted, and QQuickImage tries to static_cast a QQuickNinePatchImage to a QSGInternalImageNode. Only change resetNode when it's false; i.e. when no reset is pending. updatePaintNode() will take care of setting it to false if it's true. Change-Id: I614c172c3e24fda2506f081f8fcdb6acd1c65fb8 Fixes: QTBUG-78790 Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
-rw-r--r--src/imports/controls/imagine/qquickninepatchimage.cpp19
-rw-r--r--tests/auto/qquickimaginestyle/data/tst_imagine.qml48
-rw-r--r--tests/auto/qquickimaginestyle/qquickimaginestyle.pro4
-rw-r--r--tests/auto/qquickimaginestyle/test-assets/button-background-1.pngbin0 -> 211 bytes
-rw-r--r--tests/auto/qquickimaginestyle/test-assets/button-background-2.pngbin0 -> 211 bytes
5 files changed, 69 insertions, 2 deletions
diff --git a/src/imports/controls/imagine/qquickninepatchimage.cpp b/src/imports/controls/imagine/qquickninepatchimage.cpp
index c840c6f8..7d5e4f71 100644
--- a/src/imports/controls/imagine/qquickninepatchimage.cpp
+++ b/src/imports/controls/imagine/qquickninepatchimage.cpp
@@ -397,7 +397,24 @@ void QQuickNinePatchImage::pixmapChange()
d->updatePatches();
} else {
- d->resetNode = !d->ninePatch.isNull();
+ /*
+ Only change resetNode when it's false; i.e. when no reset is pending.
+ updatePaintNode() will take care of setting it to false if it's true.
+
+ Consider the following changes in source:
+
+ normal.png => press.9.png => normal.png => focus.png
+
+ If the last two events happen quickly, pixmapChange() can be called
+ twice with no call to updatePaintNode() inbetween. On the first call,
+ resetNode will be true (because ninePatch is not null since it is still
+ in the process of going from a 9-patch image to a regular image),
+ and on the second call, resetNode would be false if we didn't have this check.
+ This results in the oldNode never being deleted, and QQuickImage
+ tries to static_cast a QQuickNinePatchImage to a QSGInternalImageNode.
+ */
+ if (!d->resetNode)
+ d->resetNode = !d->ninePatch.isNull();
d->ninePatch = QImage();
}
QQuickImage::pixmapChange();
diff --git a/tests/auto/qquickimaginestyle/data/tst_imagine.qml b/tests/auto/qquickimaginestyle/data/tst_imagine.qml
index 03bb9602..b9078d78 100644
--- a/tests/auto/qquickimaginestyle/data/tst_imagine.qml
+++ b/tests/auto/qquickimaginestyle/data/tst_imagine.qml
@@ -54,6 +54,7 @@ import QtTest 1.1
import QtQuick.Templates 2.12 as T
import QtQuick.Controls 2.12
import QtQuick.Controls.Imagine 2.12
+import QtQuick.Controls.Imagine.impl 2.12
TestCase {
id: testCase
@@ -105,4 +106,51 @@ TestCase {
verify(control)
compare(control.font.pixelSize, 80)
}
+
+ Component {
+ id: ninePatchImageComponent
+
+ NinePatchImage {
+ property alias mouseArea: mouseArea
+
+ MouseArea {
+ id: mouseArea
+ anchors.fill: parent
+ // The name of the images isn't important; we just want to check that
+ // going from regular to 9-patch to regular to regular works without crashing.
+ onPressed: parent.source = "qrc:/control-assets/button-background.9.png"
+ onReleased: parent.source = "qrc:/test-assets/button-background-1.png"
+ onClicked: parent.source = "qrc:/test-assets/button-background-2.png"
+ }
+ }
+ }
+
+ Component {
+ id: signalSpyComponent
+
+ SignalSpy {}
+ }
+
+ // QTBUG-78790
+ function test_switchBetween9PatchAndRegular() {
+ var ninePatchImage = createTemporaryObject(ninePatchImageComponent, testCase,
+ { source: "qrc:/test-assets/button-background-1.png" })
+ verify(ninePatchImage)
+
+ var clickSpy = signalSpyComponent.createObject(ninePatchImage,
+ { target: ninePatchImage.mouseArea, signalName: "clicked" })
+ verify(clickSpy.valid)
+
+ var afterRenderingSpy = signalSpyComponent.createObject(ninePatchImage,
+ { target: testCase.Window.window, signalName: "afterRendering" })
+ verify(afterRenderingSpy.valid)
+
+ mousePress(ninePatchImage)
+ // Wait max 1 second - in reality it should take a handful of milliseconds.
+ afterRenderingSpy.wait(1000)
+ mouseRelease(ninePatchImage)
+ compare(clickSpy.count, 1)
+ // Shouldn't result in a crash.
+ afterRenderingSpy.wait(1000)
+ }
}
diff --git a/tests/auto/qquickimaginestyle/qquickimaginestyle.pro b/tests/auto/qquickimaginestyle/qquickimaginestyle.pro
index c421f2dc..4b1a309a 100644
--- a/tests/auto/qquickimaginestyle/qquickimaginestyle.pro
+++ b/tests/auto/qquickimaginestyle/qquickimaginestyle.pro
@@ -7,7 +7,9 @@ SOURCES += \
RESOURCES += \
$$PWD/qtquickcontrols2.conf \
- $$PWD/control-assets/button-background.9.png
+ $$PWD/control-assets/button-background.9.png \
+ $$PWD/test-assets/button-background-1.png \
+ $$PWD/test-assets/button-background-2.png
OTHER_FILES += \
$$PWD/data/*.qml
diff --git a/tests/auto/qquickimaginestyle/test-assets/button-background-1.png b/tests/auto/qquickimaginestyle/test-assets/button-background-1.png
new file mode 100644
index 00000000..244b707b
--- /dev/null
+++ b/tests/auto/qquickimaginestyle/test-assets/button-background-1.png
Binary files differ
diff --git a/tests/auto/qquickimaginestyle/test-assets/button-background-2.png b/tests/auto/qquickimaginestyle/test-assets/button-background-2.png
new file mode 100644
index 00000000..54f5ecd8
--- /dev/null
+++ b/tests/auto/qquickimaginestyle/test-assets/button-background-2.png
Binary files differ