[kstyle] create shadow tiles more explicitly
ClosedPublic

Authored by zzag on Mar 20 2018, 10:53 PM.

Details

Summary

With this changes it's more clear where shadow tiles are created, also it
fixes one possible bug. For example, right now, there are two shady actors
who trigger creation of the shadow tiles:

  • the first one lays inside of Style::loadConfiguration(). It creates

shadow tiles as a side effect of configuration of _mdiWindowShadowFactory

  • the second one lays inside of ShadowHelper::createPixmapHandles()

Please note, ShadowHelper::createPixmapHandles() is invoked only for X11 not Wayland!

Diff Detail

Repository
R31 Breeze
Branch
shadows
Lint
No Linters Available
Unit
No Unit Test Coverage
zzag created this revision.Mar 20 2018, 10:53 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 20 2018, 10:53 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
zzag requested review of this revision.Mar 20 2018, 10:53 PM

Hi Vlad,

First sorry for the late answer !
Second: I agree that the current code is bug prone, but your foreseen change is not satisfactory either.
The idea with the current code was to try create the shadows only "once" and if it fails for some reason, leave it failed for all widgets for which it is requested.
In your modified code however, if the creation of shadowtiles fails once, it keeps being called over and over again for each new widget.
I think this should be avoided.
One solution is possibly to call shadowTiles once and only once, in breezeStyle::loadConfiguration, and just test whether it is valid whenever used.
what do you think ?

zzag added a comment.EditedApr 9 2018, 5:02 PM

Hi Vlad,

Hi! :)

Second: I agree that the current code is bug prone, but your foreseen change is not satisfactory either.
The idea with the current code was to try create the shadows only "once" and if it fails for some reason, leave it failed for all widgets for which it is requested.
In your modified code however, if the creation of shadowtiles fails once, it keeps being called over and over again for each new widget.
I think this should be avoided.

I've tried to preserve old behavior. The current code(not in this patch) is always trying to create shadows.

One solution is possibly to call shadowTiles once and only once, in breezeStyle::loadConfiguration, and just test whether it is valid whenever used.
what do you think ?

Yes, that's a good idea. How about calling shadowTiles in ShadowHelper::reset()?

zzag updated this revision to Diff 31774.Apr 9 2018, 8:55 PM

Call shadowTiles in the reset method.

zzag added a comment.Apr 10 2018, 1:46 AM

Well, after thinking for a while, I think shadowTiles should be called in loadConfig. It doesn't make much sense to create something in reset method.

In D11533#243511, @zzag wrote:

Well, after thinking for a while, I think shadowTiles should be called in loadConfig. It doesn't make much sense to create something in reset method.

I agree

zzag updated this revision to Diff 31802.EditedApr 10 2018, 12:24 PM

Try to create shadow tiles in ShadowHelper::loadConfig().

This revision is now accepted and ready to land.Apr 10 2018, 12:36 PM
This revision was automatically updated to reflect the committed changes.