Differential D4713

Fix binding loop regression in FrameSVGItem
ClosedPublic
Actions

Authored by davidedmundson on Feb 22 2017, 12:05 AM.

Details

Reviewers
mart
broulik
Group Reviewers
Frameworks
Plasma
Commits
R242:e301b636858b: Fix binding loop regression in FrameSVGItem
Summary

d8a1a9eb084b19e552c789244267f7346e1b27a8 introduces an unintended code
change, resizeFrame() updates the margins and in turns calls
repaintNeeded. This isn't needed and is a binding loop if we ever have a
frameSVGItem whose size depends on it's own margins.

resizeFrame is different from setEnabledBorders / setElementPrefix /
theme changes because even though we need to create a new FrameData we
know any hints and margins won't change. FrameSvgItem::updateSizes
doesn't depend on the size in any way.

The old code did pointlessly call updateSizes, but didn't emit anything.

This patch that introduces a flag to updateFrameData to determine if we
should re-evaluate size hints or not.

Test Plan

GDB showed where the loop was.

Read the old code, and looked for differences

Ran plasmashell, checked I had no binding loop, frames including button which have
composeOverBorder which need the new FrameData all rendered correctly.

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptFeb 22 2017, 12:05 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik added a subscriber: broulik.EditedFeb 22 2017, 9:47 AM

While this fixes the binding loop warnings, it doesn't fix the broken margins in places like NM list delegates.

Also, Plasma crashes frequently with this, e.g. when a notification shows up, if it shows up, it has broken margins, otherwise it just crashes Plasma.

mart requested changes to this revision.Feb 22 2017, 11:27 AM
mart added a subscriber: mart.

i can reproduce kai's problems, luckily the fix seems easy

src/plasma/framesvg.cpp
757–761

in order to restore the behavior before d8a1a9eb084b19e552c789244267f7346e1b27a8
you need to update the sizes anyways in this way (the resize event did that anyways)
otherwise you can get a frame with uninitialized sizes

if (updateType == UpdateFrameAndMargins) {
    updateAndSignalSizes();
} else {
    updateSizes(fd);
}
This revision now requires changes to proceed.Feb 22 2017, 11:27 AM
davidedmundson updated this revision to Diff 11621.Feb 22 2017, 12:32 PM
davidedmundson edited edge metadata.

Updated

mart accepted this revision.Feb 22 2017, 12:37 PM
This revision is now accepted and ready to land.Feb 22 2017, 12:37 PM
broulik accepted this revision.Feb 22 2017, 1:25 PM
Closed by commit R242:e301b636858b: Fix binding loop regression in FrameSVGItem (authored by davidedmundson). · Explain WhyFeb 22 2017, 1:52 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 11623

View Options

src/plasma/framesvg.cpp

Loading...
View Options

src/plasma/private/framesvg_p.h

Loading...