Differential D14450

[effects/slidingpopups] Don't filter window quads
ClosedPublic
Actions

Authored by zzag on Jul 29 2018, 9:04 AM.

Details

Reviewers
davidedmundson
Group Reviewers
KWin
Commits
R108:4a38f15d90e0: [effects/slidingpopups] Don't filter window quads
Summary

Slided popups are clipped twice:

  • first, when filtering window quads in prePaintWindow;
  • then, when doing scissor test (the opengl scene plugin does scissor

test for transformed windows).

Because of that, we don't need to filter window quads. Filtering window
quads only adds overhead.

This change simplifies code and "fixes" incorrect clipping when a slided
popup has shadows.

Test Plan
  • Put Plasma panel on the left screen edge;
  • Open and close the Application Launcher.

(repeat for top/right/bottom screen edge)

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.Jul 29 2018, 9:04 AM
Restricted Application added a project: KWin. · View Herald TranscriptJul 29 2018, 9:04 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
zzag requested review of this revision.Jul 29 2018, 9:04 AM
zzag edited the summary of this revision. (Show Details)Jul 29 2018, 9:08 AM
zzag added a comment.Aug 1 2018, 8:02 AM

Ping.

Also, here's the sliding popups effect "without"(not really) the scissor test:

As you can see, the scissor test has been hiding a bug for many years.

zzag added a subscriber: graesslin.Aug 10 2018, 8:32 AM

@graesslin If you have time, could you please review this patch?

davidedmundson added a subscriber: davidedmundson.Aug 11 2018, 10:48 PM

My concern with this is on X if you have a panel between two screens and you slide on the right screen, I wouldn't be surprised if this now leaves artifacts on the left screen as the second scissor test is relative to the whole desktop not the physical screen.

(on wayland we render per screen so it doesn't apply)

It might not be an issue because paintWindow *also* clips the region to the screen; but that's subtly different to setting the quads.

Have a check if my concern makes sense. If not, go for it.

zzag added a comment.Aug 12 2018, 7:15 AM
In D14450#306815, @davidedmundson wrote:

My concern with this is on X if you have a panel between two screens and you slide on the right screen, I wouldn't be surprised if this now leaves artifacts on the left screen as the second scissor test is relative to the whole desktop not the physical screen.

(on wayland we render per screen so it doesn't apply)

It might not be an issue because paintWindow *also* clips the region to the screen; but that's subtly different to setting the quads.

Have a check if my concern makes sense. If not, go for it.

region is in "global" space. So, I think it would fine. But it's still worth to check your concern. If we have artifacts, then probably I have to resurrect D14412.

zzag added a comment.Aug 12 2018, 11:08 AM

@davidedmundson Because region is in "global"(not sure how to name it) space, scissor test will correctly clip popups.

If for some reason the slide effect(or any other fullscreen effect) is active, the opengl scene plugin will clip quads. That clipping code is buggy, but in most cases, it works as expected.

Anyway, it looks like there are no artifacts on X11:

davidedmundson accepted this revision.Aug 13 2018, 3:10 PM

ack, the region in paintWindow clips to the physical screen and we use that directly for the clip.

Ship it.

This revision is now accepted and ready to land.Aug 13 2018, 3:10 PM
Closed by commit R108:4a38f15d90e0: [effects/slidingpopups] Don't filter window quads (authored by zzag). · Explain WhyAug 13 2018, 4:11 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 39616

View Options

effects/slidingpopups/slidingpopups.cpp

Loading...