D13384: FrameSvg: Do not wreck shared mask frames

2018-06-27 Thread Vlad Zagorodniy 
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&id=36758

REVISION DETAIL
  https://phabricator.kde.org/D13384

AFFECTED FILES
  src/plasma/framesvg.cpp
  src/plasma/private/framesvg_p.h

To: zzag, #plasma, #frameworks, mart
Cc: mart, kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

2018-06-27 Thread Marco Martin 
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 maximize the amount of people looking 
at it)

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D13384

To: zzag, #plasma, #frameworks, mart
Cc: mart, kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

2018-06-26 Thread Vlad Zagorodniy 
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

2018-06-20 Thread Vlad Zagorodniy 
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

2018-06-13 Thread Vlad Zagorodniy 
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

2018-06-12 Thread Vlad Zagorodniy 
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&id=36056

REVISION DETAIL
  https://phabricator.kde.org/D13384

AFFECTED FILES
  src/plasma/framesvg.cpp
  src/plasma/private/framesvg_p.h

To: zzag, #plasma, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

2018-06-12 Thread Vlad Zagorodniy 
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

2018-06-07 Thread Vlad Zagorodniy 
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 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

2018-06-07 Thread Vlad Zagorodniy 
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 address the problem of corrupting
  already shared mask frames.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13384?vs=35749&id=35806

BRANCH
  dont-wreck-shared-mask-frames

REVISION DETAIL
  https://phabricator.kde.org/D13384

AFFECTED FILES
  src/plasma/framesvg.cpp
  src/plasma/private/framesvg_p.h

To: zzag, #plasma, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

2018-06-07 Thread Vlad Zagorodniy 
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

2018-06-07 Thread Vlad Zagorodniy 
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&id=35749

BRANCH
  dont-wreck-shared-mask-frames

REVISION DETAIL
  https://phabricator.kde.org/D13384

AFFECTED FILES
  src/plasma/framesvg.cpp
  src/plasma/private/framesvg_p.h

To: zzag, #plasma, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

2018-06-07 Thread Vlad Zagorodniy 
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 
statement, e.g.
  
if (maskFrame->enabledBorders != frame->enabledBorders || 
maskFrame->frameSize != frameSize(frame)) {
if (maskFrame->refcount() == 1) {
// ...
} else {
// ...
}
updateSizes(maskFrame);
}

if (maskFrame->cachedBackground.isNull()) {
generateBackground(maskFrame);
}

return maskFrame->cachedBackground;
  
  ?

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

2018-06-07 Thread Vlad Zagorodniy 
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, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

2018-06-06 Thread Vlad Zagorodniy 
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
  https://phabricator.kde.org/D13384?vs=35704&id=35705

BRANCH
  dont-wreck-shared-mask-frames

REVISION DETAIL
  https://phabricator.kde.org/D13384

AFFECTED FILES
  src/plasma/framesvg.cpp
  src/plasma/private/framesvg_p.h

To: zzag, #plasma, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

2018-06-06 Thread Vlad Zagorodniy 
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&id=35704

BRANCH
  dont-wreck-shared-mask-frames

REVISION DETAIL
  https://phabricator.kde.org/D13384

AFFECTED FILES
  src/plasma/framesvg.cpp
  src/plasma/private/framesvg_p.h

To: zzag, #plasma, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D13384: FrameSvg: Do not wreck shared mask frames

2018-06-06 Thread Vlad Zagorodniy 
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

2018-06-06 Thread Vlad Zagorodniy 
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

2018-06-06 Thread Vlad Zagorodniy 
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

2018-06-06 Thread Vlad Zagorodniy 
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

2018-06-06 Thread Vlad Zagorodniy 
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 several instances of
  FrameSvg. In that case, do not wreck maskFrame and instead act as
  follows:
  
  - try to lookup shared frames
  - if there is matching frame, use it
  - otherwise create a new one

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  dont-wreck-shared-mask-frames

REVISION DETAIL
  https://phabricator.kde.org/D13384

AFFECTED FILES
  src/plasma/framesvg.cpp
  src/plasma/private/framesvg_p.h

To: zzag, #plasma, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns