Created attachment 253300 [details] [review]
gtkwindow: Consistently set the style classes for window-frame

In one place, we forgot to remove the BACKGROUND style class before
adding window-frame. Add a helper method so these are all the same.

Created attachment 253301 [details] [review]
gdk: Add opaque region setters

Created attachment 253302 [details] [review]
gtkwindow: Calculate the opaque window based off of the style

Comment on attachment 253301 [details] [review]
gdk: Add opaque region setters

This needs API docs. Those docs should mention that this is a "fire-and-forget" function, ie that GDK doesn't keep track of the opaque region and you need to manage it yourself. And they should mention that it's just an optimization that is useful but not necessary. Not sure if we should mention that it only works on toplevels?

Comment on attachment 253302 [details] [review]
gtkwindow: Calculate the opaque window based off of the style

- Needs update in style_updated()
- Needs to unset opaque region when CSD is turned off
- Did you check that we do the right thing in a hide/unrealize/show cycle of the GtkWindow? Ie do we call set_allocation there? Because we do create a new GdkWindow...

In fact, do window properties survive an unmap/remap of an XWindow?

Review of attachment 253301 [details] [review]:

Should this warn or fail for non-toplevel windows ?

I think it should be a no-op for non-toplevels just like it is for toplevels that don't implement this operation (Broadway, Windows, ...). We generally treat functions called on non-toplevels that don't make sense on them as no-ops.

What does the code do for non-toplevels actually? I actually have no idea what GET_IMPL_CLASS() does in that case.

(In reply to comment #5)
> - Needs update in style_updated()

I don't understand why -- if the style updates, we'll do a move resize to adjust borders anyway, which should cause the set_allocation() path to get triggered again.

> - Needs to unset opaque region when CSD is turned off

Oops, right now, update_opaque_region only happens inside client_decorated because I'm an idiot and that was testing code. We should indeed always set it to the best of our knowledge.

If the user replaces CSD with a headerbar, we should still set the opaque region. And if there's no CSD at all, but an argb32 visual, with a background, we should still set the opaque region to the best of our knowledge.

The only case where the user needs to set the opaque region him/herself is when the background of the window is transparent, where we can't autodetect this.

> - Did you check that we do the right thing in a hide/unrealize/show cycle of
> the GtkWindow? Ie do we call set_allocation there? Because we do create a new
> GdkWindow...

Hm. I'll double-check, but do you know of any tests that do this so I don't have to write one myself?

(In reply to comment #6)
> In fact, do window properties survive an unmap/remap of an XWindow?

Yes.

(In reply to comment #9)
> (In reply to comment #5)
> > - Needs update in style_updated()
> 
> I don't understand why -- if the style updates, we'll do a move resize to
> adjust borders anyway, which should cause the set_allocation() path to get
> triggered again.
> 
Because we only queue a resize when properties affecting the size change: https://git.gnome.org/browse/gtk+/tree/gtk/gtkwidget.c?h=3.9.10#n7579

If you're interested, those are set via the GTK_STYLE_PROPERTY_NO_RESIZE flag in https://git.gnome.org/browse/gtk+/tree/gtk/gtkcssstylepropertyimpl.c?h=3.9.10#n69 and a similar optimization exists for fonts as you can see.

And neither the border-radius nor the background-color properties require a resize when they change.

Does that explain it?

Attachment 253300 [details] pushed as 64cf8b7 - gtkwindow: Consistently set the style classes for window-frame
Attachment 253301 [details] pushed as 08fbba4 - gdk: Add opaque region setters
Attachment 253302 [details] pushed as 3c2c3ab - gtkwindow: Calculate the opaque window based off of the style


added doc comment; discussed on IRC why the style_updated handler was not necessary -- GtkWindow's style_updated handler already queues a resize as a brute-force approach to CSD. For the next cycle, we'll look into making that more tame, and then we'll need to calculate stuff better.