[RFC] Don't draw shadows on quick tiled or maximized edges
Needs ReviewPublic

Authored by davidre on Mar 6 2020, 12:05 PM.

Details

Reviewers
zzag
hpereiradacosta
Group Reviewers
Breeze
VDG
Summary

For a quick tiled window we currently don't draw a border on the edges it is
tiled to but we still draw a shadow. In a one screen case this is cannot be
noticed because the shadows are outside of the screen. In a multi screen
environment however this causes quick tiled windows to cast shadow across the
screen border that they are tiled to. If one has multiple of such tiled windows
it darkens the neighboring area of the bordering screen significantly.
This patch uses the same criteria whether to draw a shadow on a side as is used
to determine if a border should be drawn. Each decoration instance now has its
own shadow instance. The actual shadow texture is still only generated once and
shared, the partial shadows are partial copies of it excluding the parts of the image
that should not be rendered. For example if a window is tiled to the top left corner
the cropped shadow texture will not include the top and left part as well as not the
top left, top right and bottom left corner.

Test Plan

Before:

After:

Before:
After:

Diff Detail

Repository
R31 Breeze
Branch
shadow (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 23360
Build 23378: arc lint + arc unit
davidre created this revision.Mar 6 2020, 12:05 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 6 2020, 12:05 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidre requested review of this revision.Mar 6 2020, 12:05 PM
davidre planned changes to this revision.Mar 6 2020, 12:07 PM
davidre updated this revision to Diff 77094.Mar 6 2020, 12:26 PM

set padding

davidre edited the test plan for this revision. (Show Details)Mar 6 2020, 12:27 PM
davidre edited the test plan for this revision. (Show Details)

What happens is maybe more clear when using custom window shadows:

davidre edited the summary of this revision. (Show Details)Mar 6 2020, 12:39 PM
davidre added reviewers: Breeze, VDG, zzag, hpereiradacosta.
anthonyfieroni added inline comments.
kdecoration/CMakeLists.txt
89

That's too offensive.

davidre added inline comments.Mar 6 2020, 12:57 PM
kdecoration/CMakeLists.txt
89

Sorry, that was just for the initial version of this patch to allow for static inline variables to arrive at the first implementation faster. If the direction of this patch is liked I will rework so it works with C++11

ngraham added a subscriber: ngraham.Mar 6 2020, 1:48 PM

+1 for the concept and resulting appearance. But, should this maybe be done in KWin instead? That way all window decoration themes would get this fix/change, not just Breeze.

+1 for the concept and resulting appearance. But, should this maybe be done in KWin instead? That way all window decoration themes would get this fix/change, not just Breeze.

I don't know about the best place but not drawing a border if quick tiled or maximized is also done in breeze.
Also @davidedmundson said to do it on the decorations level.

+1 for the concept and resulting appearance. But, should this maybe be done in KWin instead? That way all window decoration themes would get this fix/change, not just Breeze.

I don't know about the best place but not drawing a border if quick tiled or maximized is also done in breeze.
Also @davidedmundson said to do it on the decorations level.

Well, the thing is, all the gymnastic you do here for cutting out borders and corners is already done inside kdecoration2, to split the passed image based on the passed padding. So keeping the code here is very redundant with code that already existes elsewhere. It should be much easier (and more generic) to do this upstream. adding @zzag for further comments on this.

zzag added a comment.Mar 16 2020, 8:58 AM

Yes, whether tiled windows should cast shadows is something that must be decided by the compositor or ultimately implemented somewhere in KDecoration2 (not a big fan of this though).

In D27892#628090, @zzag wrote:

Yes, whether tiled windows should cast shadows is something that must be decided by the compositor or ultimately implemented somewhere in KDecoration2 (not a big fan of this though).

Are borders different? If they are drawn or not is something that's decided by the decoration. I would argue that the decoration provides the shadows so it should also decide this?
Can you elaborate why you don't like it? Please note that his doesn't disable shadows completely for quick tiled windows, only on the edges that it is tiled to.

Please note that his doesn't disable shadows completely for quick tiled windows, only on the edges that it is tiled to.

Personally I think that would be worthwhile, if we had it draw a single-pixel line instead of shadows so that adjacent windows continue to not blend into one another.

Please note that his doesn't disable shadows completely for quick tiled windows, only on the edges that it is tiled to.

Personally I think that would be worthwhile, if we had it draw a single-pixel line instead of shadows so that adjacent windows continue to not blend into one another.

Imo it's still beneficial to have shadows because you can still have the usual clutter of windows behind/next to a quick tiled window.

Sorry, I wasn't clear.

I mean, I think it would be nice if adjacent quick-tiled windows didn't show shadows on their shared edges. Quick tiled windows would still show shadows on their edges that are not toughing a screen edge or another quick tiled window edge.

This would probably have to be done at the KWin/KDecoration level though it would require awareness of other windows' positions.