| mart |
| Plasma | |
| Frameworks |
When trying to update the maskFrame, there is a chance that it is already
shared by several instances of FrameSvg. In that case, do not wreck maskFrame
and instead act as follows:
| Automatic diff as part of commit; lint not applicable. |
| Automatic diff as part of commit; unit tests not applicable. |
Move maskFrame->cachedBackground = QPixmap(); to the maskFrame->refcount() == 1
branch. Do we really need this assignment?
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;
?
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.
As it turns out, that's not necessary. The code is pretty much "self-explanatory".
Anyway, I'd like to proceed with code review.
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)