Bug 427311

Summary: Kolourpaint crashes when opening "More effects" window
Product: [Plasma] Breeze Reporter: Kai Uwe Broulik <kde>
Component: QStyleAssignee: Janet Blackquill <uhhadd>
Status: RESOLVED FIXED    
Severity: crash CC: kde, nate
Priority: VHI Keywords: regression
Version: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In: 5.21
Sentry Crash Report:

Description Kai Uwe Broulik 2020-10-03 22:19:43 UTC
SUMMARY
With latest Breeze header area QStyle, Kolourpaint crashes when opening "More effects" (Ctrl+M) window.

STEPS TO REPRODUCE
1. Run kolourpaint with Breeze git master
2. Open "more effects" window (Ctrl+M)

OBSERVED RESULT
Kolourpaint crashes

EXPECTED RESULT
Kolourpaint does not crash

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 
KDE Plasma Version: 5.20.80
KDE Frameworks Version: 5.75.0
Qt Version: 5.15.0

ADDITIONAL INFORMATION
Does not crash when run with fusion style

Console output:
QPaintDevice: Cannot destroy paint device that is being painted
QWidget::repaint: Recursive repaint detected

Backtrace:
Thread 1 (Thread 0x7f9a8bf72800 (LWP 49692)):
[KCrash Handler]
#4  0x000000ee00000001 in ?? ()
#5  0x00007f9a904f39a0 in QPainter::setPen (this=this@entry=0x7ffc5ae67ef0, pen=...) at painting/qpainter.cpp:3923
#6  0x00007f9a89b42923 in Breeze::Style::drawWidgetPrimitive (option=<optimized out>, widget=0x7ffc5ae689a0, painter=0x7ffc5ae67ef0, this=0x5595a2665140) at ./kstyle/breezetoolsareamanager.h:53
#7  Breeze::Style::drawWidgetPrimitive (this=0x5595a2665140, option=<optimized out>, painter=0x7ffc5ae67ef0, widget=0x7ffc5ae689a0) at ./kstyle/breezestyle.cpp:916
#8  0x00007f9a89b52a1f in std::function<bool (Breeze::Style const&, QStyleOption const*, QPainter*, QWidget const*)>::operator()(Breeze::Style const&, QStyleOption const*, QPainter*, QWidget const*) const (__args#3=<optimized out>, __args#2=<optimized out>, __args#1=<optimized out>, __args#0=..., this=0x7ffc5ae67c80) at /usr/include/c++/9/bits/std_function.h:683
#9  Breeze::Style::drawPrimitive (this=0x5595a2665140, element=QStyle::PE_Widget, option=0x7ffc5ae67d80, painter=0x7ffc5ae67ef0, widget=0x7ffc5ae689a0) at ./kstyle/breezestyle.cpp:909
#10 0x00007f9a909660e1 in QWidgetPrivate::paintBackground (this=this@entry=0x5595a3114870, painter=painter@entry=0x7ffc5ae67ef0, rgn=..., flags=...) at kernel/qwidget.cpp:2281
#11 0x00007f9a9096a483 in QWidgetPrivate::drawWidget (this=this@entry=0x5595a3114870, pdev=0x7f9a84005d70, rgn=..., offset=..., flags=flags@entry=..., sharedPainter=sharedPainter@entry=0x0, repaintManager=0x5595a3233550) at kernel/qwidget.cpp:5358
#12 0x00007f9a909405e9 in QWidgetRepaintManager::paintAndFlush (this=this@entry=0x5595a3233550) at ../../include/QtCore/../../src/corelib/tools/qpoint.h:122
#13 0x00007f9a90940eef in QWidgetRepaintManager::sync (this=0x5595a3233550, exposedWidget=0x7ffc5ae689a0, exposedRegion=...) at kernel/qwidgetrepaintmanager.cpp:743
Comment 1 Kai Uwe Broulik 2020-10-17 22:13:25 UTC
It appears to be const_cast<QDialog*>(dialog)->setContentsMargins(margins.left(), qMax(margins.top(), 1), margins.right(), margins.bottom());
It changes the content margins which probably causes the preview to repaint in response to a change of available area.

Imho it's quite bad to tamper with widgets like this in the paint pass. Isn't polish meant to be used for such preparation?
Comment 2 Nate Graham 2020-10-28 15:57:19 UTC
Carson, please investigate.
Comment 3 Bug Janitor Service 2020-11-02 11:50:58 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/breeze/-/merge_requests/47
Comment 4 Bug Janitor Service 2020-11-02 15:34:10 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/breeze/-/merge_requests/48
Comment 5 Nate Graham 2020-11-19 18:56:36 UTC
Git commit 3be21543a75e747862f3a2a5f876d23e9f66f13a by Nate Graham, on behalf of David Edmundson.
Committed on 19/11/2020 at 18:54.
Pushed by ngraham into branch 'master'.

Move dialog margin settings to polish event

Paint events MUST not change the state of things. This is especially
important within style code that is used in many apps that we do not
control.

Doing it in polish is better. It's still not ideal to adjust geometry in
the style, but at least this is where we have a similar hook for
QDockWidget so hopefully we know this pattern is acceptable.

M  +11   -3    kstyle/breezestyle.cpp

https://invent.kde.org/plasma/breeze/commit/3be21543a75e747862f3a2a5f876d23e9f66f13a