GNOME Bugzilla – Bug 744667
Shadow is too light for non-CSD windows
Last modified: 2021-07-05 13:46:23 UTC
Created attachment 297028 [details] Border nautilus and firefox This issue is similar to bug 707448, but with the removal of metacity decorations on 3.16 I thought it would need a different bug report. Shadows for non-CSD windows are too light/small when compared to those of CSD windows. See attachment.
This should have a higher priority IMHO. Sometimes it's hard to distinguish windows because of this bug.
Since the gtk3 version of Adwaita was moved into gtk+, these bugs belong there.
Moving to mutter, since the shadow of non csd windows is hardcoded.
Created attachment 338591 [details] [review] Same shadow in different windows I suggest to try this patch. Also see my comparsion screenshot. http://storage3.static.itmages.ru/i/16/1027/h_1477561514_2414847_90d42bcd9f.png It looks almost identical between CSD and non-CSD windows. Why not use something like this?
That looks pretty good, I'd really like mutter to just pick the shadow from the css though.
Created attachment 338603 [details] [review] window-actor: Stop adding shadows to windows with frames The frame decorations can pick up the shadow as defined by the GTK+ theme. (In reply to Lapo Calamandrei from comment #5) > That looks pretty good, I'd really like mutter to just pick the shadow from > the css though. This would be all that's required on the mutter side for that. On the GTK+ side, the theme would need to add a shadow to server-side decorated windows like it does for client-side decorated ones ...
Created attachment 338604 [details] [review] theme: Drop ssd-specific shadows Mutter no longer adds its own shadows to decorated windows, so the theme should provide the same shadows it does for client-side decorations.
I'm getting a bit confused here though. What about GTK+ 2 or Qt?
Thanks Florian, adwaita patch looks good to me, feel free to push it.
(In reply to Florian Müllner from comment #7) > Created attachment 338604 [details] [review] [review] > theme: Drop ssd-specific shadows > > Mutter no longer adds its own shadows to decorated windows, so the theme > should provide the same shadows it does for client-side decorations. But now there is no dofference between focused/unfocused http://storage5.static.itmages.ru/i/16/1027/h_1477574185_8944394_f5d39a98bb.png
Florian, does this have a chance to get backported? It's like the biggest usability/visual defect of gtk2.
Maxim, probably Adwaita's fault, lemme see.
Nope, Adwaita looks correct, Florian?
Yeah, mutter's fault - I figured it out, but I'll go home before writing a proper commit message and attaching a patch ...
(In reply to Florian Müllner from comment #14) > Yeah, mutter's fault - I figured it out, but I'll go home before writing a > proper commit message and attaching a patch ... So I did some optimizations first to make the performance impact not quite as bad. The first three patches are just drive-by fixes while working on theme stuff.
Created attachment 338728 [details] [review] shaped-texture: Only mask alpha channel We currently apply the mask texture to any channel, which means elements drawn outside the actual decorations are blended with gray. This should hardly matter in practice where shadows can be expected to be some shade of gray, but let's still make sure the odd theme with pink shadows or the like works correctly by applying the alpha mask to the alpha channel only.
Created attachment 338729 [details] [review] ui: Render frame in mask as well The theme may render bits of the frame that are outside the rendered area of the background, so don't skip it when painting the frame mask.
Created attachment 338730 [details] [review] theme: Consider frame borders in titlebar We currently use the entire top border area to paint the titlebar, ignoring the frame border set by the theme. Fix this.
Created attachment 338731 [details] [review] window-actor: Update mask on focus changes We currently assume that the frame mask and -shape don't change with the focus style. This is reasonable for the actual decoration, but not for the window shadow that is expected to be more pronounced when focused. We will eventually let the GTK+ theme draw the shadow of regular windows, so update shape region and frame mask on focus changes.
Created attachment 338732 [details] [review] window-actor: Use different shape_regions/masks based on focus state We now update the frame mask and -shape on every focus change, which is expensive and wasteful if we recompute the same two regions over and over again. To address this, use different regions/masks based on the focus state, so that we'll be able to re-use a previously computed one if possible.
Created attachment 338733 [details] [review] window-actor: Only recompute shape region and mask if necessary Rather than recomputing shape region and mask on every focus change, re-use the previously computed ones if possible.
Created attachment 338734 [details] [review] window-actor: Stop adding shadows to windows with frames The frame decorations can pick up the shadow as defined by the GTK+ theme.
Created attachment 338735 [details] [review] theme: Update style classes to match CSD decorations We currently use the "ssd" style class instead of "csd", so themes can avoid adding a window shadow to windows that already got a shadow from the compositor. Now that we no longer add a shadow to regular windows, we should ask for the appropriate style.
The patches are also on wip/fmuellner/gtk-shadows, no gtk+ changes required (as gtk+-3 is supposed to be "pixel-stable" now, I'm not sure the theme change should go on the stable branch anyway ...)
Thanks Florian! Yup, better not to break themes, you may merge on the stable branch the patch to sync the shadow Maxim provided in comment 4 though, no?
Let me play around with it first though please, the menu shadow looks far too big there.
I tried that bra(In reply to Florian Müllner from comment #24) > The patches are also on wip/fmuellner/gtk-shadows, no gtk+ changes required > (as gtk+-3 is supposed to be "pixel-stable" now, I'm not sure the theme > change should go on the stable branch anyway ...) I tried that branch. I found a bug. The window handling now extends outside the window contents into the shadow area. So I can drag or resize a window by clicking outside it over its shadow This is not an issue in CSD windows where handling works in the correct places only.
> I tried that branch. I found a bug. The window handling now extends outside > the window contents into the shadow area. So I can drag or resize a window > by clicking outside it over its shadow > This is not an issue in CSD windows where handling works in the correct > places only. Confirm. This happened only in Xorg session. Also, mutter restarting sometimes.
(In reply to Maxim from comment #28) > > I tried that branch. I found a bug. The window handling now extends outside > > the window contents into the shadow area. So I can drag or resize a window > > by clicking outside it over its shadow > > This is not an issue in CSD windows where handling works in the correct > > places only. > > Confirm. This happened only in Xorg session. > Also, mutter restarting sometimes. I think it is because on Wayland gtk+ applications always used CSD style shadows anyway. But that is just a guess since my Wayland experience is very limited to when I occasionally try nouveau on major Linux kernel releases.
That's technically not an issue with the branch, but https://git.gnome.org/browse/mutter/commit?id=f9db65f47f7e. What's new is that rather than making sure that there's at least 1px of invisible border (for the shadow-as-border of Adwaita), we are now accounting for the huge window shadow, which is bigger than the expected invisible border of 10px. (In reply to Maxim from comment #28) > Confirm. This happened only in Xorg session. No, it happens for all server-side window decorations. Those are simply less common on wayland (for instance, all GTK+-3 programs will use client-side decorations on wayland).
Review of attachment 338728 [details] [review]: See https://bugzilla.gnome.org/show_bug.cgi?id=697758#c10 and further patches and comments - one part of that was committed, but the part that made themes with alpha-transparency work properly never was. This change here is what I said would be needed to make his initial patch work, but would break shaped windows. I have performance concerns about the route that Simon ended up taking with un-premultiplication, and I think it would be worth seeing if the shader approach I mentioned can be made to work.
*** Bug 774313 has been marked as a duplicate of this bug. ***
Created attachment 344720 [details] [review] Update shadows to match Adwaita better This should make Mutter shadows almost indifferentiable to the ones made by Adwaita.
I’m not sure how to credit Maxim though, I don’t even know his/her real name. Note the patch is not finished, but works nicely so far, the only issue being combo-/option-menus getting normal (and not menu) shadows, but I believe Florian has a solution to that.
Created attachment 344722 [details] [review] Update shadows to match Adwaita better This should make Mutter shadows almost indifferentiable to the ones made by Adwaita. Based on an initial patch from Maxim <dead.555@bk.ru>.
Review of attachment 344722 [details] [review]: ACK'ed by Jakub on IRC
Is the issue I mentioned in comment #27 existent also in the updated patch? I'm rebuilding mutter now to check.
(In reply to Hussam Al-Tayeb from comment #37) > Is the issue I mentioned in comment #27 existent also in the updated patch? > > I'm rebuilding mutter now to check. Nope. No issue with new patch :)
Created attachment 344727 [details] [review] window-actor: Special-case COMBO window's shadow class We currently don't have any shadow class for combo box popups, which means the default shadow of normal windows is used. That's clearly odd given that the two are very different, and isn't consistent with GTK+-3's client-side shadows for popups. While we could add a dedicated shadow class, the designers are fine with reusing the existing shadow for dropdown-menus, so let's do that.
Review of attachment 344727 [details] [review]: sure
Attachment 344722 [details] pushed as 91c6a14 - Update shadows to match Adwaita better Attachment 344727 [details] pushed as cb86745 - window-actor: Special-case COMBO window's shadow class
Could this be backported to 3.22?
Sure. It's not a huge change, so asking informally on IRC for a UI freeze break should be fine. (Although it's not very likely that there'll be another 3.22.x release by now)
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new ticket at https://gitlab.gnome.org/GNOME/mutter/-/issues/ Thank you for your understanding and your help.