[breeze desktop theme/dialogs] Add rounded corners to dialogs
ClosedPublic

Authored by rooty on Jan 25 2019, 11:41 PM.

Details

Summary

This patch aims to make the corners of the dialog/notification backgrounds (dialogs/background.svgz) more rounded and more in keeping
with the Breeze window decoration theme.

Test Plan

Before:


After:

Before (Breeze Dark, no Background contrast effect):


After (Breeze Dark, no Background contrast effect):

Using the Background Contrast Kwin effect adds a weird dark background to all the dialogs and the corners aren't rounded anymore:

and these strange dots appear instead (a rectangle bleeding through?)

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
rooty created this revision.Jan 25 2019, 11:41 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 25 2019, 11:41 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
rooty requested review of this revision.Jan 25 2019, 11:41 PM
rooty edited the test plan for this revision. (Show Details)Jan 25 2019, 11:44 PM
rooty added reviewers: VDG, ngraham.

Wow, it's amazing how much nicer the After screenshot looks than the Before. Such a difference from a tiny change! +1

Wow, it's amazing how much nicer the After screenshot looks than the Before. Such a difference from a tiny change! +1

Thanks! And thanks to Noah Davis for giving me a boost
What do you think about the corner shadows though - too strong? Or... not an issue?

filipf added a subscriber: filipf.Jan 26 2019, 12:08 AM

Really nice, I think it fits in more with how edges are drawn elsewhere.

Could you also add screenshots with Breeze Dark?

rooty edited the test plan for this revision. (Show Details)Jan 26 2019, 12:12 AM

Really nice, I think it fits in more with how edges are drawn elsewhere.

Could you also add screenshots with Breeze Dark?

Sure thing. Added to test plan.

Really nice, I think it fits in more with how edges are drawn elsewhere.

Could you also add screenshots with Breeze Dark?

Sure thing. Added to test plan.

Hmm, there doesn't seem to be much of a difference if any with Breeze dark, I wonder if that's because I haven't yet modified opaque/dialogs/background.svgz. Stay tuned :D

Codezela added a subscriber: Codezela.EditedJan 26 2019, 8:25 AM

can i talk about other thing
why the vlise button have this circle around it
i think it will look cleaner with simple border strock or with only the close red circle
and the round corner is so beautiful

rooty retitled this revision from [breeze desktop theme/Notifications] WIP, Add rounded corners to notifications (dialogs) to [breeze desktop theme/dialogs] WIP, Add rounded corners to dialogs (mostly notifications).Jan 26 2019, 9:56 AM
rooty updated this revision to Diff 50310.EditedJan 26 2019, 10:14 AM

Change long decimals to integers; fix corner shadows

can i talk about other thing
why the vlise button have this circle around it
i think it will look cleaner with simple border strock or with only the close red circle
and the round corner is so beautiful

You make a good point regarding the close button, but that doesn't seem to have anything to do with this diff, sorry

rooty edited the test plan for this revision. (Show Details)Jan 26 2019, 10:22 AM

Dialog is used in way more places that "mostly notifications"
It's used in applet popups, OSDs, WidgetExplorer, kwin tab switcher.

Let me know if you do want this change to only target notifications.

rooty added a comment.EditedJan 26 2019, 11:20 AM

Dialog is used in way more places that "mostly notifications"
It's used in applet popups, OSDs, WidgetExplorer, kwin tab switcher.

Let me know if you do want this change to only target notifications.

Oh no, I'd like for this to cover all the dialogs, it'll look even more consistent.
I've also figured out why Breeze Dark dialogs don't get rounded thanks to Alex.
He thought it was the blur but it was actually Kwin's Background contrast effect - if I turn it off, the corners turn out rounded:

as opposed to
with the effect on.

I just don't know if this is an actual bug?

P.S. I actually like Breeze Dark like this... it's not pitch black anymore. But hey, no accounting for taste. :D

rooty retitled this revision from [breeze desktop theme/dialogs] WIP, Add rounded corners to dialogs (mostly notifications) to [breeze desktop theme/dialogs] WIP, Add rounded corners to dialogs.Jan 26 2019, 2:01 PM
rooty edited the test plan for this revision. (Show Details)Jan 26 2019, 5:49 PM
ngraham added a comment.EditedJan 29 2019, 4:40 PM

I really like the effect that this gives--not only for notifications, but also for plasma popups, OSDs, window switchers, etc. It's subtle and classy. Let us know when the remaining bugs are worked out!

rooty added a comment.Jan 29 2019, 6:17 PM

I really like the effect that this gives--not only for notifications, but also for plasma popups, OSDs, window switchers, etc. It's subtle and classy. Let us know when the remaining bugs are worked out!

Actually, there's nothing more to be done here.

This actually uncovered a bug (feature?) in Kwin.
"Background Contrast" darkens the background of all the UI elements by adding rectangles (instead of just darkening their backgrounds).

There are two ways forward here:
(1) Ditch the background contrast effect by default - make Breeze Dark take on its natural color (I'm in favor of this one)
or
(2) Fix the Kwin background contrast effect - which I have no idea how to do, but might be able to learn

Either way this patch remains the same, so... it's basically ready :D

davidedmundson added a comment.EditedJan 29 2019, 6:23 PM

"Background Contrast" darkens the background of all the UI elements by adding rectangles (instead of just darkening their backgrounds).

That's both true and not true.

We take the frame SVG and run:

QBitmap(d->alphaMask().mask())
Which is a list of rectangles, but there's no reason why it wouldn't be an exact representation.

Is the alphamask of the edited frame updated?

If you open the file, the mask is the black rectangle on the right.

rooty added a comment.EditedJan 29 2019, 6:28 PM

"Background Contrast" darkens the background of all the UI elements by adding rectangles (instead of just darkening their backgrounds).

That's both true and not true.

We take the frame SVG and run:

QBitmap(d->alphaMask().mask())
Which is a list of rectangles, but there's no reason why it wouldn't be an exact representation.

Is the alphamask of the edited frame updated?

I've only edited the three parts (the transparent squircle - is that the alphamask? sorry I'm not familiar with the terminology, the shadows and the black squircle).
There was a part of the svg with strong rectangular borders i didn't change - should I have?

Also, I have no idea why this happens (whitish dot in the corner, on top of a black background):

rooty added a comment.EditedJan 29 2019, 6:33 PM


Before on top
After below

(I did the same thing to translucent/dialogs/background.svgz too)

EDIT: I didn't remove the transparent arrowhead it just didn't fit in the screenshot :D

rooty retitled this revision from [breeze desktop theme/dialogs] WIP, Add rounded corners to dialogs to [breeze desktop theme/dialogs] Add rounded corners to dialogs.Jan 30 2019, 8:13 AM

So this is now working for me as intended, yay! The effect is very strongly positive. Huge improvement IMO.

But do we know why? It there indeed a bug in the KWin effect somewhere? Or was the SVG previously broken somehow? Or was the prior corner radius too small to make a difference so nobody saw it? Etc.

rooty added a comment.Jan 31 2019, 6:31 AM

So this is now working for me as intended, yay! The effect is very strongly positive. Huge improvement IMO.

But do we know why? It there indeed a bug in the KWin effect somewhere? Or was the SVG previously broken somehow? Or was the prior corner radius too small to make a difference so nobody saw it? Etc.

There's something about the Kwin effect that renders a rectangle without the rounded corners. I hope David's able to uncover why because I don't understand it at all.
And yeah I think you're right - seeing as it wasn't rounded before, nobody could notice.

The corner were very slightly rounded, but the circle arc didn't extend beyond the borders of a single pixel so it really was a true rectangle, for all intents and purposes.

It works for me too, both dark and light, if I turn off the effect.
I really like Breeze Dark this way. It's not pitch black without the effect hahaha

zzag added a subscriber: zzag.Jan 31 2019, 9:11 AM

It there indeed a bug in the KWin effect somewhere?

I don't think so. Both the blur and the background contrast effect operate on regions. Rounded corners are approximated by a bunch of smaller rectangles.

Shot in the dark: maybe purge cache?

rooty added a comment.Jan 31 2019, 1:03 PM
In D18545#402668, @zzag wrote:

Shot in the dark: maybe purge cache?

No change unfortunately.

ngraham accepted this revision.Jan 31 2019, 11:12 PM

This works as intended for me on both Breeze and Breeze Dark and looks fantastic. Sooo... is there anything left to do here, or shall we land it?

This revision is now accepted and ready to land.Jan 31 2019, 11:12 PM
zzag added a comment.Jan 31 2019, 11:18 PM

Well, there are still issues with corners

(both the blur and the background contrast effect are disabled)

zzag added a comment.Jan 31 2019, 11:19 PM

... though I'm not sure whether that's a bug.

rooty added a comment.EditedFeb 1 2019, 10:42 AM

Nate, thank you for accepting the patch!

Okay so the corner thing - that bug actually happens everywhere.
All of the following screenshots are of master, not this patch.

For example, Breeze Dark, Background Contrast on, Blur off

I apologize for the tiny screenshot but if you zoom in you'll see that there's a square at the center that's #2C2E31 and the rest of the notification is #2E3235. The colors should be the same but they're not.

If you turn the blur on, it still exhibits the same issue.
If you turn background contrast off, the square becomes brighter, but still different from the rest of the notification.

Seeing as all of these screenshots are the current master, what this patch seems to do for this issue is make it more visible, but it doesn't seem to be causing it. Manipulating the black rectangle (alpha mask) doesn't seem to fix any of these issues so I don't think that it is the culprit.

TLDR: I think it might be safe to land it.

P.S. @zzag while I have had issues with corners, they've never looked like they do in your screenshots, are you sure you've applied both svgz (latest diff), removed the old ones and made sure that it's using the proper breeze-dark colors file (yours looks a little wan). Mine works like Nate's.

And did you take the screenshot using spectacle without using Kwin's zoom? Because Kwin's zoom effect distorts the notification corners too

Wait a few days. Frameworks tagging is the first Saturday of the month.

We want some wider testing given this hits stable releases.

rooty added a comment.EditedFeb 1 2019, 10:49 AM

Wait a few days. Frameworks tagging is the first Saturday of the month.

We want some wider testing given this hits stable releases.

Not a bad idea. Next week then :D
By the way, I didn't think people tested this stuff off of Phabricator though (just the ones in the actual thread) :D I thought I had to land it for the rest to test it (in a beta or neon?)

rooty added a comment.EditedFeb 1 2019, 11:59 AM

By the way guys, it does look sometimes better with the blur off:


(and with the blur on, the corners behave weird
, see bottom left corner)

Maybe Alex was right; maybe the blur effect's the culprit, never having been able to handle rounded corners.

How to reproduce: Open up an application with a big dark title bar (I use chromium) and a white/bright space below the bar (New Tab).
If a corner of the notification dialog happens to overlie a part of the dark title bar the corners will actually turn white (I'm assuming because of the white space / new tab below).

zzag added a comment.Feb 1 2019, 12:07 PM

The blur effect is able to handle rounded corners, a client just has to upload proper blur region. Yes, I had the latest version of the diff. I don't use the zoom effect.

rooty added a comment.Feb 1 2019, 1:27 PM
In D18545#403154, @zzag wrote:

The blur effect is able to handle rounded corners, a client just has to upload proper blur region. Yes, I had the latest version of the diff. I don't use the zoom effect.

I think you're right, yeah. I honestly have no idea why this happens:

rooty added a comment.Feb 5 2019, 3:27 PM

Okay seeing as I'm seeing this corner problem with my icons as well,

vs

I think it's safe to land this. Also it's way past Saturday so :D

This revision was automatically updated to reflect the committed changes.