Adaptive panel opacity
Needs ReviewPublic

Authored by cblack on Apr 6 2020, 3:31 PM.

Details

Summary

This patch introduces adaptive panel opacity, where if a window is maximized, the panel becomes opaque.

Configuration is added in order to allow a user to opt out of adaptive opacity.

Test Plan

Diff Detail

Repository
R119 Plasma Desktop
Branch
arcpatch-D28627
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25137
Build 25155: arc lint + arc unit
niccolove created this revision.Apr 6 2020, 3:31 PM
Restricted Application added a project: Plasma. · View Herald TranscriptApr 6 2020, 3:31 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
niccolove requested review of this revision.Apr 6 2020, 3:31 PM
niccolove planned changes to this revision.Apr 6 2020, 3:41 PM
niccolove added a reviewer: Plasma.
ngraham added a subscriber: ngraham.EditedApr 6 2020, 4:12 PM

Why?

See the discussion in D28353.

Essentially, we ran into a design issue whereby increasing the transparency of the panel looked bad when a window was maximized against it, impeding our ability to do so. Simultaneously, having an even slightly transparent panel with a window maximized against it results in an inconsistency between the opaque window and the transparent panel. With this patch/change, we get the aesthetic benefits of greater transparency when there isn't a window maximized against the panel, and the functional and consistency benefits of an opaque panel when there is a window maximized against the panel.

Thanks Nate for explaining.

Couple of things that hit my mind:

  • Plasmoids should also be opaque, to be fully integrated:


Should I have very similiar code in the applet code as well, or is there a better way to implement it?

  • Currently, when I switch from a fullscreen app to an empty desktop it doesn't get back to transparent, which is weird, I'll investigate that

Yeah plasmoid pop-ups should probably also be opaque when the panel is opaque, good call.

davidedmundson requested changes to this revision.Apr 7 2020, 8:57 PM

Ok, so the logic is - the goal is to blend in. If you're not showing the desktop, you can't blend in to it. I can at least understand the thought process.


Code wise, this needs a lot of work.

You're rendering a transparent panel. we're still even blurring the background very expensively. And then we're drawing something on top. That's very wasteful.
We're also mixing up shadows from one SVG with the output of another.

Also our isMaximised detection is horrific.


We also need to think about how this plays in with other themes. You can't assume everyone uses Breeze.

desktoppackage/contents/views/Panel.qml
59

why

You're rendering a transparent panel. we're still even blurring the background very expensively. And then we're drawing something on top. That's very wasteful.
We're also mixing up shadows from one SVG with the output of another.

Right. Originally I just changed the svg to be opaque, but I couldn't get a transition out of it. Also, I guess this doesn't solve the blurring problem.

Also our isMaximised detection is horrific.

What do you mean with our? Is the way I used TaskManager very wrong (quite possible) or is the window.isMaximized implementation bad in win?

We also need to think about how this plays in with other themes. You can't assume everyone uses Breeze.

I checked this. It always fallback to Breeze opaque background following the theme colors, so it should always work without breaking them.

or is the window.isMaximized implementation bad in win?

The problematic part of this patch is that whenever any property on any window changes we're checking all items.

We only want to:

  • check when isMaximised potentially changes
  • only check the item that has changed

We only want to:

  • check when isMaximised potentially changes

It would also be nice to turn off transparency for a panel that has a window tiled against its screen edge, but not fully maximized. Is that feasible?

I have a question though: we ship the pager widget in the panel that does the same by default. Wouldn't that have the same efficiency impact, if not even more?

cblack commandeered this revision.Apr 12 2020, 10:41 PM
cblack added a reviewer: niccolove.
cblack marked an inline comment as done.
cblack added a reviewer: VDG.
cblack updated this revision to Diff 79969.Apr 12 2020, 11:07 PM

Refactor to state/transitions and don't render two FrameSvgItems at once

cblack updated this revision to Diff 79970.Apr 13 2020, 12:26 AM

Fix errors

cblack updated this revision to Diff 79972.Apr 13 2020, 12:41 AM

Filter by screen

cblack updated this revision to Diff 79973.Apr 13 2020, 1:25 AM

Add configuration

cblack retitled this revision from WIP: Made panel opaque on maximized window to Adaptive panel opacity.
cblack edited the summary of this revision. (Show Details)
cblack edited the test plan for this revision. (Show Details)Apr 13 2020, 1:36 AM

+1 for making this a user-controllable setting, but how does it interact with plasma themes where there isn't any transparency? Can we only show this UI for plasma themes with transparency?

apol added a subscriber: apol.Apr 13 2020, 1:49 PM

+1 for making this a user-controllable setting, but how does it interact with plasma themes where there isn't any transparency? Can we only show this UI for plasma themes with transparency?

Shouldn't it be a setting of the theme rather than a button somewhere?

It's a weird thing for a user to be specific about.

I feel like it makes sense for the user to want to use a opaque panel rather than a transparent one. Also, that relies on themes actually turning on the effect, while users could already benefit from it even on current themes if the setting was exposed. Also, that allows for making only some panels adaptive, unity-stile. However, as far as I know, there's no [easy] way to detect transparency from the theme. I might be wrong here.

  • During the transition from transparent to opaque, the panel becomes immediately gray and then trasitions. I guess that's because you turn off blur at the beginning of the animation?
  • This should also be implemented for plasmoids popups as well, in order for them to integrate correctly:

cblack updated this revision to Diff 80035.Apr 13 2020, 5:20 PM

Choreography fixes

niccolove accepted this revision as: niccolove.Apr 13 2020, 6:37 PM
niccolove accepted this revision.Apr 20 2020, 11:17 AM

Works great!

Actually -- this seems to no longer apply panel internal margins?

Actually -- this seems to no longer apply panel internal margins?

Works for me.

In terms of the UI and the scope, I have to agree with @apol though, and maybe go a bit further: I think this should be either a global setting that applies to all transparent Plasma stuff (dialogs, pop-ups, panels, widgets, etc) or a setting specific to the Breeze Plasma theme--which would first require that we create such a UI (Plasma Theme Explorer doesn't count).

Having it only affect the panel feels a bit off TBH. If I'm the kind of person who hates transparency, I want it off for everything else too. If I'm in the target user group who doesn't change the defaults, I'll think it's weird that the panel itself becomes opaque for a maximized window, but not the pop-ups for widgets on that panel.

Also I see an interesting visual glitch when a window is un-maximized:

@davidedmundson can you take another look at the general direction of the code here? I'd like to know your thoughts on the direction this code is using before I start spending time sanding off the rough parts

Actually -- this seems to no longer apply panel internal margins?

Works for me.

But then you post a video where the panel has no internal margins :P
Give a look to the task manager: normally, there's a margin between it and panel border. This patch currently removes the margin set by the desktop theme.

I still don't notice what you're trying to point out, so I'll take your word for it. :p