D13384: FrameSvg: Do not wreck shared mask frames

This revision was automatically updated to reflect the committed changes. Closed by commit R242:412a517576d5: FrameSvg: Do not wreck shared mask frames (authored by zzag). REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13384?vs=36056=36758

D13384: FrameSvg: Do not wreck shared mask frames

mart accepted this revision. mart added a comment. This revision is now accepted and ready to land. sorry for making you wait too long on those framesvg changes, looks like i have to tune up phab notifications a bit more as they didn't show up :/ (anyways, adding Plasma in tags should

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a comment. Ping. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a comment. Ping. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a comment. Ping? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag updated this revision to Diff 36056. zzag added a comment. Use `frame`'s imagePath. REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13384?vs=35806=36056 REVISION DETAIL https://phabricator.kde.org/D13384 AFFECTED FILES

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a dependent revision: D13496: FrameSvg: Update mask frame if image path has been changed. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a comment. In D13384#275246 , @zzag wrote: > Add comments describing CFG. As it turns out, that's not necessary. The code is pretty much "self-explanatory". Anyway, I'd like to proceed with code review. REPOSITORY R242

D13384: FrameSvg: Do not wreck shared mask frames

zzag updated this revision to Diff 35806. zzag added a comment. Rename `shouldUpdateSizes` back to `shouldUpdate` That's for a possible future diff: the mask frame should be updated if maskFrame->imagePath != frame->imagePath. That's an unrelated change because this diff tries to

D13384: FrameSvg: Do not wreck shared mask frames

zzag planned changes to this revision. zzag added a comment. Add comments describing CFG. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag updated this revision to Diff 35749. zzag added a comment. Update sizes only if enabled borders or frame size have been changed REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13384?vs=35705=35749 BRANCH

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a comment. In D13384#275224 , @zzag wrote: > Corner case: what if only need to update cachedBackground(i.e. enabledBorders and frameSize are okay)? Delete `shouldUpdate` and wrap if (refcount() == 1) {} into another if

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a comment. Corner case: what if only need to update cachedBackground(i.e. enabledBorders and frameSize are okay)? REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh,

D13384: FrameSvg: Do not wreck shared mask frames

zzag updated this revision to Diff 35705. zzag added a comment. Move `maskFrame->cachedBackground = QPixmap();` to the maskFrame->refcount() == 1 branch. Do we really need this assignment? REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE

D13384: FrameSvg: Do not wreck shared mask frames

zzag updated this revision to Diff 35704. zzag added a comment. Do not invalidate looked up frames. REPOSITORY R242 Plasma Framework (Library) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D13384?vs=35699=35704 BRANCH dont-wreck-shared-mask-frames REVISION DETAIL

D13384: FrameSvg: Do not wreck shared mask frames

zzag added a comment. Sorry for the nasty diff. Caching is hard. :( REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag edited the summary of this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag edited the summary of this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag edited the summary of this revision. REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D13384 To: zzag, #plasma, #frameworks Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

zzag created this revision. zzag added reviewers: Plasma, Frameworks. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. zzag requested review of this revision. REVISION SUMMARY It might happen that the maskFrame is shared by