Bug 403978 - oxygen theme incompatible with qt5 5.12.1-1 libs
Summary: oxygen theme incompatible with qt5 5.12.1-1 libs
Status: RESOLVED UPSTREAM
Alias: None
Product: Oxygen
Classification: Plasma
Component: general (show other bugs)
Version: 5.14.5
Platform: Other Linux
: NOR crash (vote)
Target Milestone: ---
Assignee: Unassigned bugs mailing-list
URL:
Keywords:
: 404109 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-02-06 00:37 UTC by Potomac
Modified: 2019-02-24 16:08 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
patch for qt5-base 5.12.1 (12.16 KB, text/plain)
2019-02-11 04:46 UTC, Potomac
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Potomac 2019-02-06 00:37:41 UTC
SUMMARY
I use archlinux,
since a recent update of qt5 packages (to the 5.12.1-1 version) I notice that kde-plasma is broken if the oxygen theme is used,

the desktop displays but the mouse doesn't move, and the harddisk led blink for a long time, the entire system seems frozen (but I can open a terminal with ctrl-alt-f2),

if I downgrade qt5 to the previous version (5.12.0) then the oxygen theme is usable, no bugs,

and If I use the default theme (breeze) then no problem with the last version of qt5 libs (5.12.1-1)

so there is an incompatibility between oxygen 5.14.5 and qt5 5.12.1-1

STEPS TO REPRODUCE
1. install kde plasma 5.14.5
2. use the oxygen theme 5.14.5
3. update to the last qt5 lib release version (5.12.1-1)
4. reboot your system
5. you will notice that kde-plasma hangs when desktop is displayed (mouse frozen)

OBSERVED RESULT
the desktop is frozen, we can not move the mouse, harddisk led blinks a long time

EXPECTED RESULT
kde plasma should not frozen when oxygen theme is used with the last version of qt5 (5.12.1-1)

SOFTWARE/OS VERSIONS
Windows: 
MacOS: 
Linux/KDE Plasma: archlinux 64 bits, kde plasma 5.14.5
(available in About System)
KDE Plasma Version: 5.14.5
KDE Frameworks Version: 5.54.0
Qt Version: 5.12.1

ADDITIONAL INFORMATION
I use radeon HD4650 pci-e graphic card, with the open source driver (radeon), openGL2.0 acceleration in plasma settings for the compositer
Comment 1 Potomac 2019-02-06 00:44:34 UTC
list of qt5 packages I use in 5.12.1 version (last release version) :

qt5-base
qt5-sensors
qt5-script
qt5-x11extras
qt5-multimedia
qt5-speech
qt5-svg
qt5-declarative
qt5-graphicaleffects
qt5-quickcontrols
qt5-quickcontrols2
qt5-location
qt5-tools
qt5-webchannel
qt5-webengine
qt5-webkit
qt5-xmlpatterns
Comment 2 Johan Klokkhammer Helsing 2019-02-06 10:22:59 UTC
Potomac: Do you know the exact Qt version that worked for you. In particular, is there a difference between 5.12.0-1 and 5.12.0-2?
Comment 3 Potomac 2019-02-06 11:33:00 UTC
the bug occurs with the very recent qt5 libs archlinux package : 5.12.1-1 version, date release : 2019-02-04,

the previous version (5.12.0-1) doesn't have problem with oxygen theme,

all versions before 5.12.1 works with oxygen theme,

you can see here the files that have changed in qt5 5.12.1 version :
https://wiki.qt.io/Qt_5.12.1_Change_Files
Comment 4 Potomac 2019-02-06 11:43:24 UTC
for the reproducibility of the bug it's important to reboot or disconnet/reconnect to a new kde session after switching the theme to oxygen
Comment 5 Johan Klokkhammer Helsing 2019-02-06 11:51:53 UTC
> you can see here the files that have changed in qt5 5.12.1 version :
https://wiki.qt.io/Qt_5.12.1_Change_Files

I know, I wrote that file ;)

Anyways, I was asking because I asked the archlinux maintainer to cherry-pick a patch that fixed a freeze which resulted in version 5.12.0-2, however it was reverted in 5.12.0-3, due to some clients using 100% cpu. But if you used 5.12.0-1, 5.12.1-1, then that fix is unrelated.

Also, it would probably be useful for whoever is going to debug this if you could attach a log with WAYLAND_DEBUG=1 set in the environment.
Comment 6 Potomac 2019-02-06 12:12:07 UTC
(In reply to Johan Klokkhammer Helsing from comment #5)

> Also, it would probably be useful for whoever is going to debug this if you
> could attach a log with WAYLAND_DEBUG=1 set in the environment.

Ok I will test with WAYLAND_DEBUG=1 set in the environment,

but I don't know if I use currently wayland in plasma, I use the default settings in archlinux, and I think by default it's the x11 session for plasma in archlinux
Comment 7 Johan Klokkhammer Helsing 2019-02-06 12:15:27 UTC
Ah, my bad. I was searching the tracker for wayland changes, and somehow this showed up, and I didn't really check the description itself.

I guess you can probably disregard everything I said :/

Sorry for the noise.
Comment 8 Potomac 2019-02-07 20:02:32 UTC
some archlinux users reported an excessive memory consumption with Qt5 5.12.1 when oxygen theme is used, which leds to an extreme slowness (mouse doesn't move, high usage of the swap file) :

https://bbs.archlinux.org/viewtopic.php?pid=1830703
Comment 9 Potomac 2019-02-11 04:43:58 UTC
I did a git bisect and I manage to find the faulty Qt5 commit, it's the commit :

[9d90c0edac91b35ec96646fd3e6cdd339639ca79] QImage: merge the size calculations with proper (non-UB) checks

https://github.com/qt/qtbase/commit/9d90c0edac91b35ec96646fd3e6cdd339639ca79

If I revert this commit then the bug disapears, all is ok, no memory leak with oxygen theme,
I created a patch which reverts the faulty commit :

https://bugreports.qt.io/secure/attachment/80855/revert_size_calculations_with_proper_non-UB_checks.patch
Comment 10 Potomac 2019-02-11 04:46:03 UTC
Created attachment 117975 [details]
patch for qt5-base 5.12.1

patch which reverts the faulty qt5 commit 9d90c0edac91b35ec96646fd3e6cdd339639ca79
Comment 11 Benjamin Robin 2019-02-19 23:25:52 UTC
Reverting 9d90c0edac91b "fixes" the issue, but this part of code is not the problem.

I did add some tracing, and there are 2 allocations that are realized in QImageData with the following properties:

===> nbytes: 8589934552, bpl: 4, w: 1, h: 2147483638, d: 32
===> nbytes: 42949672760, bpl: 20, w: 5, h: 2147483638, d: 32

A detailed callstack:

#0 0x00007ffff5673d7f in raise () at /usr/lib/libc.so.6
#1 0x00007ffff5f9ba9a in QImageData::create(QSize const&, QImage::Format) (size=..., format=QImage::Format_RGB32) at image/qimage.cpp:156
#2 0x00007fffbbffe670 in ()
#3 0x00007fffbbffe820 in ()
#4 0x00007ffff602b14d in QImage::QImage(QSize const&, QImage::Format) (this=0x4a2e9539e508d00, size=..., format=3154110064) at image/qimage.cpp:779
#5 0x00007ffff602b186 in QImage::QImage(int, int, QImage::Format) (this=<optimized out>, width=<optimized out>, height=<optimized out>, format=<optimized out>) at ../../include/QtCore/../../src/corelib/tools/qsize.h:119
#6 0x00007ffff606e40e in QRasterPlatformPixmap::resize(int, int) (this=0x7fffb431c6c0, width=-1140857264, height=2147483638) at image/qpixmap_raster.cpp:112
#7 0x00007ffff606d879 in QPlatformPixmap::create(int, int, QPlatformPixmap::PixelType) (w=1, h=2147483638, type=<optimized out>) at image/qplatformpixmap.cpp:65
#8 0x00007ffff6064948 in QPixmap::doInit(int, int, int) (this=0x7fffbbffe820, w=<optimized out>, h=<optimized out>, type=<optimized out>) at image/qpixmap.cpp:95
#9 0x00007ffff7b3a19a in Plasma::SvgPrivate::findInCache(QString const&, double, QSizeF const&) (this=0x55555703a3d0, elementId=..., ratio=1, s=...) at /usr/src/debug/plasma-framework-5.55.0/src/plasma/svg.cpp:396
#10 0x00007ffff7b3b1f4 in Plasma::Svg::image(QSize const&, QString const&) (this=<optimized out>, size=..., elementID=...) at /usr/include/qt/QtCore/qsize.h:134
#11 0x00007fffdcf4eb40 in Plasma::FrameItemNode::updateTexture(QSize const&, QString const&) (elementId=..., size=..., this=0x7fffb4310650) at /usr/src/debug/plasma-framework-5.55.0/src/declarativeimports/core/framesvgitem.cpp:501
#12 0x00007fffdcf4eb40 in Plasma::FrameItemNode::reposition(QRect const&, QSize&) (fullSize=..., frameGeometry=..., this=0x7fffb4310650) at /usr/src/debug/plasma-framework-5.55.0/src/declarativeimports/core/framesvgitem.cpp:160
#13 0x00007fffdcf4eb40 in Plasma::FrameSvgItem::updatePaintNode(QSGNode*, QQuickItem::UpdatePaintNodeData*) (this=<optimized out>, oldNode=<optimized out>) at /usr/src/debug/plasma-framework-5.55.0/src/declarativeimports/core/framesvgitem.cpp:565
#14 0x00007ffff77f9bd0 in QQuickWindowPrivate::updateDirtyNode(QQuickItem*) () at /usr/lib/libQt5Quick.so.5
#15 0x00007ffff77fa044 in QQuickWindowPrivate::updateDirtyNodes() () at /usr/lib/libQt5Quick.so.5
#16 0x00007ffff77fb577 in QQuickWindowPrivate::syncSceneGraph() () at /usr/lib/libQt5Quick.so.5
#17 0x00007ffff77a0e79 in () at /usr/lib/libQt5Quick.so.5
#18 0x00007ffff77a224d in () at /usr/lib/libQt5Quick.so.5
#19 0x00007ffff77a5b58 in () at /usr/lib/libQt5Quick.so.5
#20 0x00007ffff5a4d96c in () at /usr/lib/libQt5Core.so.5
#21 0x00007ffff4b4ca9d in start_thread () at /usr/lib/libpthread.so.0
#22 0x00007ffff5737b23 in clone () at /usr/lib/libc.so.6

     
 - frameGeometry parameter of Plasma::FrameItemNode::reposition() is equal to "1x2147483638+5+5"
 - In FrameSvgItem::updatePaintNode() of framesvgitem.cpp:561, The local variable QSize frameSize(width(), height());is equal to: wd=11 and ht=-2147483648
 - Looks like it is related to imagePath=widgets/scrollbar  prefix=slider
Comment 12 Patrick Silva 2019-02-20 14:42:30 UTC
bug 404109 seems duplicate
Comment 13 Benjamin Robin 2019-02-20 19:35:59 UTC
The bug is triggered by the line 193 of /usr/lib/qt/qml/QtQuick/Controls/Styles/Plasma/ScrollViewStyle.qml

handle: PlasmaCore.FrameSvgItem {
        imagePath:"widgets/scrollbar"

If the imagePath is changed to an invalid path, the bug is hidden
Comment 14 Christoph Feck 2019-02-20 23:44:59 UTC
*** Bug 404109 has been marked as a duplicate of this bug. ***
Comment 15 Graham Perrin 2019-02-23 03:54:54 UTC
Thanks folks. 

As far as I can tell, this bug is not specific to Linux. 

See for example 
<https://bugreports.qt.io/browse/QTBUG-73691?focusedCommentId=447953&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-447953> (with a screenshot from me). Also: 

Full swap partition
<https://lists.freebsd.org/pipermail/freebsd-questions/2019-February/284475.html>
Comment 16 Benjamin Robin 2019-02-23 10:19:15 UTC
The negative number come from the following line of code of GenericBinding::write():

doStore<int>(result.doubleValue(), pd, flags);

The variable "result" is equal to "nan", converted to an int give -2147483648
Comment 17 Benjamin Robin 2019-02-23 14:05:27 UTC
I suggest the following patch, maybe this is not the proper fix (it's hide the problem), but it makes the code more robust:

--- framesvgitem.cpp.orig       2019-02-03 01:05:33.000000000 +0100
+++ framesvgitem.cpp    2019-02-23 14:54:56.000000000 +0100
@@ -558,7 +558,7 @@
 
         if (m_sizeChanged) {
             FrameNode* frameNode = static_cast<FrameNode*>(oldNode);
-            QSize frameSize(width(), height());
+            QSize frameSize(int(qMax(width(), 0.0)), int(qMax(height(), 0.0)));
             QRect geometry = frameNode->contentsRect(frameSize);
             QSGNode *node = oldNode->firstChild();
             while (node) {
@@ -581,7 +581,7 @@
         if ((m_textureChanged || m_sizeChanged) || textureNode->texture()->textureSize() != m_frameSvg->size()) {
             QImage image = m_frameSvg->framePixmap().toImage();
             textureNode->setTexture(s_cache->loadTexture(window(), image));
-            textureNode->setRect(0, 0, width(), height());
+            textureNode->setRect(0, 0, qMax(width(), 0.0), qMax(height(), 0.0));
 
             m_textureChanged = false;
             m_sizeChanged = false;
@@ -603,7 +603,7 @@
     CheckMarginsChange checkFixedMargins(m_fixedMargins);
 
     QQuickItem::componentComplete();
-    m_frameSvg->resizeFrame(QSize(width(), height()));
+    m_frameSvg->resizeFrame(QSizeF(width(), height()));
     m_frameSvg->setRepaintBlocked(false);
     m_textureChanged = true;
 }
Comment 18 Nate Graham 2019-02-23 14:07:06 UTC
Thank you, Benjamin! Please feel free to submit your patch here: https://community.kde.org/Infrastructure/Phabricator#Posting_a_Patch_using_the_website

Even if it's not complete or 100% working, you can put [RFC] in the title and people will offer helpful comments.
Comment 19 Benjamin Robin 2019-02-23 16:45:50 UTC
Here the created patch on phabricator for review: https://phabricator.kde.org/D19256
Comment 20 Nate Graham 2019-02-23 17:58:56 UTC
Thanks!
Comment 21 Benjamin Robin 2019-02-24 08:50:43 UTC
I finally found the true origin of this bug (I have to learn how to debug QML...)
The bug is triggered by the following QML line of code:

height: __styleData.horizontal ? implicitHeight : extent

from /usr/lib/qt/qml/QtQuick/Controls/Styles/Base/ScrollViewStyle.qml line 380

The problem comes from the computation of the "extent" variable. When the bug occurs (more than once, 14 times on my system):
 - The flickableItem is valid (We are not using bg.width or bg.height)
 - A division by 0 occurs (flickableItem.contentWidth and flickableItem.contentHeight are eaquals to 0)
Comment 22 Benjamin Robin 2019-02-24 09:41:09 UTC
I am moving the discussion back to the Qt bug report (https://bugreports.qt.io/browse/QTBUG-73691). The web site is down for now...

Here the patch in order to properly fix the problem:

diff --git a/src/controls/Styles/Base/ScrollViewStyle.qml b/src/controls/Styles/Base/ScrollViewStyle.qml
index 6750399d..36b518d3 100644
--- a/src/controls/Styles/Base/ScrollViewStyle.qml
+++ b/src/controls/Styles/Base/ScrollViewStyle.qml
@@ -370,8 +370,8 @@ Style {
 
         property var flickableItem: control.flickableItem
         property int extent: Math.max(minimumHandleLength, __styleData.horizontal ?
-                                          Math.min(1, (flickableItem ? flickableItem.width/flickableItem.contentWidth : 1)) * bg.width :
-                                          Math.min(1, (flickableItem ? flickableItem.height/flickableItem.contentHeight : 1)) * bg.height)
+                                          Math.min(1, ((flickableItem && flickableItem.contentWidth > 0.0) ? flickableItem.width/flickableItem.contentWidth : 1)) * bg.width :
+                                          Math.min(1, ((flickableItem && flickableItem.contentHeight > 0.0) ? flickableItem.height/flickableItem.contentHeight : 1)) * bg.height)
         readonly property real range: __control.maximumValue - __control.minimumValue
         readonly property real begin: __control.value - __control.minimumValue
Comment 23 Nate Graham 2019-02-24 16:08:39 UTC
Thanks for the sleuthing, and for submitting a Qt patch! Closing this as RESOLVED UPSTREAM then, and let's focus on fixing the issue in Qt. Thanks again!