Differential D14784

[scenes/opengl] Fix overlaps in shadow texture atlas
ClosedPublic
Actions

Authored by zzag on Aug 13 2018, 11:00 AM.

Details

Reviewers
davidedmundson
Group Reviewers
KWin
Commits
R108:dadcd5113513: [scenes/opengl] Fix overlaps in shadow texture atlas
Summary

If the corner shadow tiles(top-left, top-right, and so on) tiles are missing,
then the left/top/right/bottom shadow tiles will overlap.

This diff addresses that problem by changing how the shadow texture
atlas is rendered:

  • corner tiles will be drawn in the corners of the atlas(buildQuads method expects them to be at the corners);
  • top, right, bottom, and left tile will be aligned to the top-left corner of the inner shadow rect.

For majority of desktop themes, the shadow texture atlas looks the same.
For example, here's for Aether:

Before:

After:

Depends on D14783

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
zzag created this revision.Aug 13 2018, 11:00 AM
Restricted Application added a project: KWin. · View Herald TranscriptAug 13 2018, 11:00 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Aug 13 2018, 11:00 AM
zzag edited the summary of this revision. (Show Details)Aug 13 2018, 11:16 AM
abetts added a subscriber: abetts.Aug 20 2018, 2:41 PM

Seems good to me! +1

davidedmundson added a subscriber: davidedmundson.Aug 21 2018, 3:11 PM
davidedmundson added inline comments.
plugins/scenes/opengl/scene_opengl.cpp
2425–2435

If left is wider than topleft then we'll end up with a gap between the topleft and top.

Personally I'd call that a corrupt data and not care, which is what the code currently does.
But this whole patch is about handling corrupt shadows.

We could either p.drawPixmap(QRect, QImage) so it scales to fill the quadrant

or

p.drawPixmap( innerRectLeft - shadowElementTopLeft.width(), ....)

so that we align the topLeft element to the right of it's allocated rectangle.

(or the equivalent with style()->drawItemPixmap)

zzag added inline comments.Aug 21 2018, 3:55 PM
plugins/scenes/opengl/scene_opengl.cpp
2425–2435

If left is wider than topleft then we'll end up with a gap between the topleft and top.

Yes, there will be gap. That's fault of whoever provided such corrupted tiles, not ours.

Normally, the texture atlas won't have gaps between tiles.

I don't think that's good idea to stretch tiles. FWIW, plasma provides corner tiles that are bigger than left/top/right/bottom tiles (because of rounded corners).

zzag updated this revision to Diff 40165.Aug 21 2018, 6:16 PM

Rebase.

zzag added a comment.Aug 24 2018, 4:49 PM

Ping.

As I said earlier, I think we shouldn't stretch tiles.

If we have to stretch corner tiles, what about left/top/right/bottom tiles? Should we stretch them to match corner tiles?

  • if so, what about desktop themes with rounded dialog corners? We'll distort top/right/bottom/left tiles(even though that's hard to notice)
  • if not, why then only corner tiles have to be stretched?

In this case, the best what we can do is nothing.

If there is texture bleeding during sliding of popups, that's fault of a desktop theme, not ours.

davidedmundson accepted this revision.Aug 31 2018, 10:29 AM
This revision is now accepted and ready to land.Aug 31 2018, 10:29 AM
Closed by commit R108:dadcd5113513: [scenes/opengl] Fix overlaps in shadow texture atlas (authored by zzag). · Explain WhyAug 31 2018, 2:36 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 40776

View Options

autotests/integration/scene_opengl_shadow_test.cpp

Loading...
View Options

plugins/scenes/opengl/scene_opengl.cpp

Loading...