Create shadow tiles on demand
ClosedPublic

Authored by broulik on Aug 17 2018, 10:56 AM.

Details

Summary

Breeze spent 70ms on every application startup loading the shadow tiles, despite them only being used for e.g. context menus. Create them only as needed.

Test Plan
  • Context menu shadows still look fine
  • MDI windows don't have any shadows here but that is also without this patch..

Diff Detail

Repository
R31 Breeze
Lint
Lint Skipped
Unit
Unit Tests Skipped
broulik created this revision.Aug 17 2018, 10:56 AM
Restricted Application added a project: Plasma. · View Herald TranscriptAug 17 2018, 10:56 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Aug 17 2018, 10:56 AM
zzag added a comment.EditedAug 17 2018, 11:51 AM

Breeze spent 70ms

What computer are you using? What shadow size it is? What's typical load on your computer? What's the value of the mean? What's the value of the standard deviation? Is it debug build or release build? How did you measure that?

zzag added a comment.Aug 17 2018, 12:00 PM

MDI windows don't have any shadows here but that is also without this patch

No, they have:

It's hard to see them because of low contrast.

I found Breeze's ShadowHelper::loadConfig() stick out in hotspot on Dolphin startup, so I investigated what it did and found it would needlessly load the shadows.

It's hard to see them because of low contrast.

Right, didn't look closely enough. They still work with this patch.

zzag accepted this revision.Aug 17 2018, 12:50 PM

I found Breeze's ShadowHelper::loadConfig() stick out in hotspot on Dolphin startup, so I investigated what it did and found it would needlessly load the shadows.

Well, I wouldn't trust hotspot/perf in this case. Creation of kstyle shadows is pretty cheap so I think that's fine to create them on demand (in contrast to decoration shadows, which are pretty heavy).

Anyway, codewise, patch looks okay to me.

Please delete "70ms" in the summary because that number doesn't mean anything.

This revision is now accepted and ready to land.Aug 17 2018, 12:50 PM
zzag added a comment.Aug 17 2018, 12:58 PM

Also, please prepend "[kstyle]" to the title.

This revision was automatically updated to reflect the committed changes.