fix(kcm): avoid recursive OpacityMask which leads to teardown crash
ClosedPublic

Authored by davidedmundson on May 1 2020, 3:12 PM.

Details

Summary

In the current code OpacityMask's source is the parent which contains
itself, which doesn't make sense. Docs explicitly say this. [1]

This leads to a crash in teardown.

A quick reshuffle fixes it.

BUG: 419625

[1] https://doc.qt.io/qt-5/qml-qtgraphicaleffects-opacitymask.html#source-prop

Test Plan

Found reproducible steps (see bug)
No longer crashes

Diff Detail

Repository
R104 KScreen
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidedmundson created this revision.May 1 2020, 3:12 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 1 2020, 3:12 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidedmundson requested review of this revision.May 1 2020, 3:12 PM
romangg accepted this revision.May 1 2020, 3:36 PM
romangg added a subscriber: romangg.

Docs explicitly say this.

When you reference some "docs" please take 20 seconds and directly link it. Every piece you write here and in git commit messages is potentially read by hundreds of other people. If you link the source once you save dozens of man-hours of other people looking the information up afterwards. Or they might decide to not look it up because it's too much hassle -> onboarding.

Thanks for looking into that issue. lgtm without further delay, I trust you on your QML expertise.

kcm/package/contents/ui/Output.qml
60

Is the change making this unnecessary? I.e. do the graphics still look "smooth"?

This revision is now accepted and ready to land.May 1 2020, 3:36 PM
romangg edited the summary of this revision. (Show Details)May 1 2020, 3:37 PM
This revision was automatically updated to reflect the committed changes.
davidedmundson added inline comments.May 2 2020, 9:54 PM
kcm/package/contents/ui/Output.qml
60

Default is true so it makes no difference.

There's generally no reason to set it to false.