D6630: [kstyle] Do not delete the Surface for a QWindow
graesslin closed this revision. graesslin added a comment. https://commits.kde.org/breeze/e02fef0883af3d3b33ce8a9ea8677c82c1973975 REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D6630 To: graesslin, #plasma, mart, davidedmundson Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6630: [kstyle] Do not delete the Surface for a QWindow
graesslin added a comment. In https://phabricator.kde.org/D6630#124201, @davidedmundson wrote: > Whilst this does make sense based on the current design we are effectively building up dead objects during the lifespan of a window doing show/hide/show. Well it was designed when Qt didn't behave like that :-( > Someone somewhere should delete it. Assuming everyone follows the same design pattern of following QWindow::hideEvent everyone calling deleteLater() would be safe, but it relies on all parties dealing with surfaces to do that. No it wouldn't, because KWin doesn't behave like Qt. KWin does not delete a surface when a window gets hidden, but reuses it. Basically KWin behaves exactly as Qt used to, the pattern was taken from QtWayland. Now this diverged. A possibility could be QSharedPointer, so that every user can discard it's own instance, but that requires also changes all over the place. REPOSITORY R31 Breeze BRANCH master REVISION DETAIL https://phabricator.kde.org/D6630 To: graesslin, #plasma, mart, davidedmundson Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6630: [kstyle] Do not delete the Surface for a QWindow
davidedmundson added a comment. Whilst this does make sense based on the current design we are effectively building up dead objects during the lifespan of a window doing show/hide/show. Someone somewhere should delete it. Assuming everyone follows the same design pattern of following QWindow::hideEvent everyone calling deleteLater() would be safe, but it relies on all parties dealing with surfaces to do that. REPOSITORY R31 Breeze BRANCH master REVISION DETAIL https://phabricator.kde.org/D6630 To: graesslin, #plasma, mart, davidedmundson Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6630: [kstyle] Do not delete the Surface for a QWindow
davidedmundson accepted this revision. This revision is now accepted and ready to land. REPOSITORY R31 Breeze BRANCH master REVISION DETAIL https://phabricator.kde.org/D6630 To: graesslin, #plasma, mart, davidedmundson Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6630: [kstyle] Do not delete the Surface for a QWindow
graesslin edited the test plan for this revision. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D6630 To: graesslin, #plasma, mart Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6630: [kstyle] Do not delete the Surface for a QWindow
graesslin created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. REVISION SUMMARY Surface::fromWindow are shared! By deleting the Surface pointer copies in other areas are getting deleted as well. This breaks e.g. KWin in various ways. This fixes a regression introduced with https://phabricator.kde.org/R31:d4940fe692c7be10025bfcb6a118c2cb750039d6 REPOSITORY R31 Breeze BRANCH master REVISION DETAIL https://phabricator.kde.org/D6630 AFFECTED FILES kstyle/breezeshadowhelper.cpp To: graesslin, #plasma Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
D6630: [kstyle] Do not delete the Surface for a QWindow
graesslin added a reviewer: mart. REPOSITORY R31 Breeze REVISION DETAIL https://phabricator.kde.org/D6630 To: graesslin, #plasma, mart Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, lukas
