Well, it does say "...that are used with the main node..." but let's see if we can make this clearer

For example .ssd class is set on decoration node - and it is not main node.

.ssd is a bit special since GTK+ is not using it at all - mutter uses it for rendering its window decorations. So, it is something that the theme should style, but it is not used by GTK+.

It would be good if we were consistent and put .ssd on the same node as .csd and .solid-csd, though. Florian ?

(In reply to Matthias Clasen from comment #3)
> It would be good if we were consistent and put .ssd on the same node as .csd
> and .solid-csd, though. Florian ?

I'm not sure we can do much better in mutter - nodes are internal to GTK+, all we have are style contexts and gtk_widget_path_iter_set_object_name() to set a single node name. And between "window" and "decoration", the latter just appears much more relevant to styling - otherwise themes (including Adwaita) would have to replace "decoration" with "window.csd, window.solid-csd, window.ssd" all over the place.

(In reply to Florian Müllner from comment #4)
> (In reply to Matthias Clasen from comment #3)
> > It would be good if we were consistent and put .ssd on the same node as .csd
> > and .solid-csd, though. Florian ?
> 
> I'm not sure we can do much better in mutter - nodes are internal to GTK+,
> all we have are style contexts and gtk_widget_path_iter_set_object_name() to
> set a single node name. And between "window" and "decoration", the latter
> just appears much more relevant to styling - otherwise themes (including
> Adwaita) would have to replace "decoration" with "window.csd,
> window.solid-csd, window.ssd" all over the place.

For example .titled class is put on window node - so it is already broken in mutter.

Add new META_STYLE_ELEMENT_WINDOW with object_name "window" and move here all classes from META_STYLE_ELEMENT_FRAME. (window-frame is not used by Adwaita anymore)

Then for META_STYLE_ELEMENT_FRAME add META_STYLE_ELEMENT_WINDOW as parent and it will have no classes.

Something like this:
style_info->styles[META_STYLE_ELEMENT_WINDOW] = create_style_context (G_TYPE_NONE, NULL, provider, "decoration", GTK_STYLE_CLASS_BACKGROUND, "ssd", NULL);

style_info->styles[META_STYLE_ELEMENT_FRAME] = create_style_context (G_TYPE_NONE, style_info->styles[META_STYLE_ELEMENT_WINDOW], provider, "decoration", NULL);

P.S. You don't need META_TYPE_FRAMES, GTK_TYPE_HEADER_BAR, ... as they are not used for matching css, object name is used instead.

(In reply to Alberts Muktupāvels from comment #5)
> For example .titled class is put on window node - so it is already broken in
> mutter.

Only in GTK+ - again, there are no nodes in mutter, just style contexts.


> Add new META_STYLE_ELEMENT_WINDOW with object_name "window" and move here
> all classes from META_STYLE_ELEMENT_FRAME. (window-frame is not used by
> Adwaita anymore)

No, this does not match GTK+. Those are *style contexts*, and even in a CSS node world, widgets still only have one of those. Your suggestion also implies that we paint the window twice, with different style information - *if* we are lucky, the correct style will be drawn on top. For geometry information, it's even worse - what's supposed to happen if both give different padding imformation? Pick the larger/smaller one? Add them up? (that's what we should do if there were actual two nested widgets, but both styles refer to the same widget) Or pick a random one?

So no, using style contexts as if they were nodes doesn't make sense to me. If we want mutter/GTK+ behavior to match more closely, we need additional API in GTK+.


> style_info->styles[META_STYLE_ELEMENT_FRAME] = create_style_context
> (G_TYPE_NONE, style_info->styles[META_STYLE_ELEMENT_WINDOW], provider,
> "decoration", NULL);
> 
> P.S. You don't need META_TYPE_FRAMES, GTK_TYPE_HEADER_BAR, ... as they are
> not used for matching css, object name is used instead.

*Adwaita* stopped using them in favor of CSS node names. We cannot assume the same for all other themes people might be using.

(In reply to Florian Müllner from comment #6)
> (In reply to Alberts Muktupāvels from comment #5)
> > For example .titled class is put on window node - so it is already broken in
> > mutter.
> 
> Only in GTK+ - again, there are no nodes in mutter, just style contexts.
> 
> 
> > Add new META_STYLE_ELEMENT_WINDOW with object_name "window" and move here
> > all classes from META_STYLE_ELEMENT_FRAME. (window-frame is not used by
> > Adwaita anymore)
> 
> No, this does not match GTK+. Those are *style contexts*, and even in a CSS
> node world, widgets still only have one of those. Your suggestion also
> implies that we paint the window twice, with different style information -
> *if* we are lucky, the correct style will be drawn on top. For geometry
> information, it's even worse - what's supposed to happen if both give
> different padding imformation? Pick the larger/smaller one? Add them up?
> (that's what we should do if there were actual two nested widgets, but both
> styles refer to the same widget) Or pick a random one?
> 
> So no, using style contexts as if they were nodes doesn't make sense to me.
> If we want mutter/GTK+ behavior to match more closely, we need additional
> API in GTK+.
> 

Have you looked at the foreign drawing example in gtk3-demo ? It shows how to handle multi-node situations if you don't have widgets to draw with. You *can* make it work with style context stacking; not that it is pretty or obvious...

(In reply to Florian Müllner from comment #6)
> (In reply to Alberts Muktupāvels from comment #5)
> > For example .titled class is put on window node - so it is already broken in
> > mutter.
> 
> Only in GTK+ - again, there are no nodes in mutter, just style contexts.

Have you tried to use mutter with git GTK+ from master? with Adwaita tiled window still has rounded corners, but it should not...

> > Add new META_STYLE_ELEMENT_WINDOW with object_name "window" and move here
> > all classes from META_STYLE_ELEMENT_FRAME. (window-frame is not used by
> > Adwaita anymore)
> 
> No, this does not match GTK+. Those are *style contexts*, and even in a CSS
> node world, widgets still only have one of those. Your suggestion also
> implies that we paint the window twice, with different style information -
> *if* we are lucky, the correct style will be drawn on top. For geometry
> information, it's even worse - what's supposed to happen if both give
> different padding imformation? Pick the larger/smaller one? Add them up?
> (that's what we should do if there were actual two nested widgets, but both
> styles refer to the same widget) Or pick a random one?
> 
> So no, using style contexts as if they were nodes doesn't make sense to me.
> If we want mutter/GTK+ behavior to match more closely, we need additional
> API in GTK+.

Is not those style context created to build correct path?

You have:
decoration.ssd.background.tiled ...
but need:
window.background.titled decoration.sdd ...

And Matthias wants to move .sdd to window node.

> > style_info->styles[META_STYLE_ELEMENT_FRAME] = create_style_context
> > (G_TYPE_NONE, style_info->styles[META_STYLE_ELEMENT_WINDOW], provider,
> > "decoration", NULL);
> > 
> > P.S. You don't need META_TYPE_FRAMES, GTK_TYPE_HEADER_BAR, ... as they are
> > not used for matching css, object name is used instead.
> 
> *Adwaita* stopped using them in favor of CSS node names. We cannot assume
> the same for all other themes people might be using.

Other themes will be updated for GTK+ 3.20, no? Themes without updating will not work with GTK+ 3.20, or am I wrong. I tried Ambiance theme and it does not work.

(In reply to Florian Müllner from comment #6)
> > P.S. You don't need META_TYPE_FRAMES, GTK_TYPE_HEADER_BAR, ... as they are
> > not used for matching css, object name is used instead.
> 
> *Adwaita* stopped using them in favor of CSS node names. We cannot assume
> the same for all other themes people might be using.

No, GTK+ stopped using them and also Mutter!. Only one thing is used to match style - type (GtkLabel) or object name (label). Mutter sets object names for all style contexts that means it will match only this style:

decoration {}
headerbar {}
label {}

and not 

MetaFrames {}
GtkHeaderBar {}
GtkLabel {}

So you can replace all types with G_TYPE_NONE and it will still work.

(In reply to Matthias Clasen from comment #7)
> Have you looked at the foreign drawing example in gtk3-demo ? It shows how
> to handle multi-node situations if you don't have widgets to draw with. You
> *can* make it work with style context stacking; not that it is pretty or
> obvious...

So you're saying we should create two style contexts for the "window", then:
 - use margin/padding/border from "decoration" and ignore those from "window"
 - use border-radius from "window", ignore that from "decoration"
 - use "decoration" for drawing, ignore "window"

This looks fairly random, do we have any guarantees that this isn't going to break again in a couple of months?

(In reply to Florian Müllner from comment #10)
> This looks fairly random, do we have any guarantees that this isn't going to
> break again in a couple of months?

Apparently this doesn't work with GTK+ master, so before doing anything, we'll need some theme changes first ...

Can you push wip branch with your changes? In current GTK+ master .ssd class is on decoration node - maybe that is reason why it does not work.

(In reply to Alberts Muktupāvels from comment #12)
> In current GTK+ master .ssd class
> is on decoration node - maybe that is reason why it does not work.

No, the issue occurs when removing .background from 'decoration' - it doesn't even matter whether the 'window' or 'decoration' context is used for drawing. Anyway, pushed as the top commit in wip/fmuellner/theme-updates - as-is, it needs the "wrong" .background class on the decoration context.

(In reply to Florian Müllner from comment #10)

> This looks fairly random, do we have any guarantees that this isn't going to
> break again in a couple of months?

"Garantien gibt dir keiner"

However, there's a couple of things we've done to make breakage less likely:

- CSS nodes, names and style classes are documented in some detail now

- Our testsuite spot-checks expected CSS node tree layout: testsuite/css/nodes

- There is a "Foreign Drawing" example in gtk3-demo that codifies the best-practice approach to drawing without widgets that we will keep working

I would suggest a few things that would make this safer for mutter:

- Double-check that the CSS node documentation in GtkWindow matches reality and the use in mutter

- Add window testcases in testsuite/css/nodes

- Expand the Foreign Drawing example to cover window decoration drawing

(In reply to Florian Müllner from comment #13)
> (In reply to Alberts Muktupāvels from comment #12)
> > In current GTK+ master .ssd class
> > is on decoration node - maybe that is reason why it does not work.
> 
> No, the issue occurs when removing .background from 'decoration' - it
> doesn't even matter whether the 'window' or 'decoration' context is used for
> drawing. Anyway, pushed as the top commit in wip/fmuellner/theme-updates -
> as-is, it needs the "wrong" .background class on the decoration context.

meta_ui_frame_get_mask - change META_STYLE_ELEMENT_FRAME to META_STYLE_ELEMENT_WINDOW and titlebar will appear, but then there are back "background" in rounded corner place.

(In reply to Alberts Muktupāvels from comment #15)
> (In reply to Florian Müllner from comment #13)
> > (In reply to Alberts Muktupāvels from comment #12)
> > > In current GTK+ master .ssd class
> > > is on decoration node - maybe that is reason why it does not work.
> > 
> > No, the issue occurs when removing .background from 'decoration' - it
> > doesn't even matter whether the 'window' or 'decoration' context is used for
> > drawing. Anyway, pushed as the top commit in wip/fmuellner/theme-updates -
> > as-is, it needs the "wrong" .background class on the decoration context.
> 
> meta_ui_frame_get_mask - change META_STYLE_ELEMENT_FRAME to
> META_STYLE_ELEMENT_WINDOW and titlebar will appear, but then there are back
> "background" in rounded corner place.

Changed this:

  gtk_render_background (frame->style_info->styles[META_STYLE_ELEMENT_FRAME], cr,
                         borders.invisible.left / scale,
                         borders.invisible.top / scale,
                         frame_rect.width / scale, frame_rect.height / scale);

to:

  gtk_render_background (frame->style_info->styles[META_STYLE_ELEMENT_FRAME], cr,
                         borders.invisible.left / scale,
                         borders.invisible.top / scale,
                         frame_rect.width / scale, frame_rect.height / scale);
  gtk_render_background (frame->style_info->styles[META_STYLE_ELEMENT_TITLEBAR], cr,
                         borders.invisible.left / scale,
                         borders.invisible.top / scale,
                         frame_rect.width / scale, frame_rect.height / scale);

and looks like decorations works fully.

P.S. tiled window (when margin is 0) window lose box-shadow border... Should not GTK+ expose some function to get box-shadow size? I once asked about this, but then no one wanted to do it...

Created attachment 319304 [details] [review]
gtkstylecontext: add gtk_style_context_get_box_shadow

Function to retrieve box-shadow size as GtkBorder. This could be used by mutter and metacity to get extra visible size.

For example if draggable-border-width is set to 0 then box-shadow border is not visible. Mutter does not know that there is extra visible content and currently there is no way to get this info.

Company	mclasen: the thing we really want is something like gtk_render_background_get_area (context, x, y, width, height, out_area);
Company	mclasen: it answers "how many pixels will i touch when i draw this background?"
mclasen: If you do a function like this, it should probably be named gtk_render_background_get_clip() or gtk_render_background_get_clip_area()

(In reply to Alberts Muktupāvels from comment #17)
> Created attachment 319304 [details] [review] [review]
> gtkstylecontext: add gtk_style_context_get_box_shadow
> 
I don't like this because it exposes an implementation detail of drawing (the fact that we do have a thing called box shadows). I don't think public API should concern itself with those implementation details. Public API should ideally not even know that we're using CSS internally.

What you are interested in is the area drawn by a call to gtk_render_background(), so we want an API that can give you this information. Which is why I suggest something like
  gtk_render_background_get_clip(context, x, y, width, height, out_clip_rect);
or
  gtk_render_get_background_clip(context, x, y, width, height, out_clip_rect);
that would return the area that would be touched by a call to gtk_render_background().

(In reply to Benjamin Otte (Company) from comment #19)
> (In reply to Alberts Muktupāvels from comment #17)
> > Created attachment 319304 [details] [review] [review] [review]
> > gtkstylecontext: add gtk_style_context_get_box_shadow
> > 
> I don't like this because it exposes an implementation detail of drawing
> (the fact that we do have a thing called box shadows). I don't think public
> API should concern itself with those implementation details. Public API
> should ideally not even know that we're using CSS internally.

gtk_style_context_get_shadow_extents? There is get_border, get_margin, get_padding, why not function for shadow size?

> What you are interested in is the area drawn by a call to
> gtk_render_background(), so we want an API that can give you this
> information. Which is why I suggest something like
>   gtk_render_background_get_clip(context, x, y, width, height,
> out_clip_rect);
> or
>   gtk_render_get_background_clip(context, x, y, width, height,
> out_clip_rect);
> that would return the area that would be touched by a call to
> gtk_render_background().

Not sure if that really will work... For example client window with pos 0x0 and size 200x120px (margin: 10px; border: 2px solid #333: box-shadow: 0 0 0 1px #888).

That would return what? x - (-3) y - (-3) width - 206, height - 126?

Mutter and Metacity have info about border size for each window size - top, right, bottom, left. That would mean that we need to do extra work to get usable data - we need 3px on left, 3px on right.

Then we can have also this box-shadow: 2px 2px 0 1px #888?

GTK+ would calculate and return data that directly would not be usable. We will need to do extra calculation.

Function _gtk_css_shadows_value_get_extents (shadows, box_shadow) returns info that I am interested.

> gtk_style_context_get_shadow_extents? There is get_border, get_margin,
> get_padding, why not function for shadow size?
> 
Those functions probably shouldn't exist. The fact that they take a total nonsensical @state argument shows how old they are.

> Not sure if that really will work... For example client window with pos 0x0
> and size 200x120px (margin: 10px; border: 2px solid #333: box-shadow: 0 0 0
> 1px #888).
>
That would return { -1, -1, 202, 122 } because gtk_render_background() is drawn on the border edge, so amrgin is ignored.
 
> Mutter and Metacity have info about border size for each window size - top,
> right, bottom, left. That would mean that we need to do extra work to get
> usable data - we need 3px on left, 3px on right.
> 
You do this extra work anyway once you call gtk_render_background(), no?

(In reply to Benjamin Otte (Company) from comment #21)
> > gtk_style_context_get_shadow_extents? There is get_border, get_margin,
> > get_padding, why not function for shadow size?
> > 
> Those functions probably shouldn't exist. The fact that they take a total
> nonsensical @state argument shows how old they are.

And how about this:
GtkBorder border;
gtk_style_context_get (context, state, "box-shadow", &border, NULL);

In similar way to this:
https://git.gnome.org/browse/gtk+/commit/?id=ea69bf8c17dd5c9e7f76bf3cb4f56ec07b2e821f

No extra public function, that also will not appear in documentation, but at least can be used in these rare cases.

> > Not sure if that really will work... For example client window with pos 0x0
> > and size 200x120px (margin: 10px; border: 2px solid #333: box-shadow: 0 0 0
> > 1px #888).
> >
> That would return { -1, -1, 202, 122 } because gtk_render_background() is
> drawn on the border edge, so amrgin is ignored.

Ok, and that second box-shadow example would return { 0, 0, 203, 123 }, right?

> > Mutter and Metacity have info about border size for each window size - top,
> > right, bottom, left. That would mean that we need to do extra work to get
> > usable data - we need 3px on left, 3px on right.
> > 
> You do this extra work anyway once you call gtk_render_background(), no?

Is/will returned values be directly usable in gtk_render_background? As I think it size/position without shadow as it is drawn outside, no?

So in our example it has returned { -1, -1, 202, 122 }, mutter/metacity has calculated that total window size is 220x140 (10px resize area on each side). If I will use returned value it will draw in incorrect place as minimum. So returned data can be used just to extract shadow size.

Mutter/Metacity have visible, invisible and total border for decoration.

Currently visible is just visible border (border: 2px...) (it does not include box-shadow), invisible is resize are (where box-shadow appears) and total = visible + invisible.

If invisible border for some reason is set 0, box-shadow will not be visible. box-shadow size is needed so we can make sure that box-shadow is always visible - frame is not create too small.

So if I as user want to change draggable-border-width 10 to 2, but theme use box-shadow that is larger then 2px it will not be fully visible. And what if some crazy user and/or theme author wants use box-shadow to create 12px "border", then even default draggable-border-width is not enough and increasing it is not solution.

(In reply to Alberts Muktupāvels from comment #22)

> So if I as user want to change draggable-border-width 10 to 2, but theme use
> box-shadow that is larger then 2px it will not be fully visible. And what if
> some crazy user and/or theme author wants use box-shadow to create 12px
> "border", then even default draggable-border-width is not enough and
> increasing it is not solution.

We're generally not interested in catering to 'crazy' users. We just expose enough that mutters decorations can match what we do for csd in Adwaita.

(In reply to Matthias Clasen from comment #23)
> (In reply to Alberts Muktupāvels from comment #22)
> 
> > So if I as user want to change draggable-border-width 10 to 2, but theme use
> > box-shadow that is larger then 2px it will not be fully visible. And what if
> > some crazy user and/or theme author wants use box-shadow to create 12px
> > "border", then even default draggable-border-width is not enough and
> > increasing it is not solution.
> 
> We're generally not interested in catering to 'crazy' users. We just expose
> enough that mutters decorations can match what we do for csd in Adwaita.

No need to be crazy user, this is problem in mutter:
https://bugzilla.gnome.org/show_bug.cgi?id=752794
https://bugzilla.gnome.org/show_bug.cgi?id=745060

And it could be fixed if we would know how big box-shadow is.

I have no box-shadow for IBus Preferences window too.

I just tried to patch GTK+ (master) and mutter (wip/fmuellner/theme-updates) and I got box-shadow for both - tiled gnome-terminal and IBus Preferences window.

https://paste.gnome.org/pb93kxly9

I've added gtk_render_background_get_clip now.

(In reply to Matthias Clasen from comment #26)
> I've added gtk_render_background_get_clip now.

Thanks!

Are you going to update Adwaita to move .ssd class to window node? Or it will remain on decoration node?

I've added .ssd decoration in addition to decoration.ssd in Adwaita now.

(In reply to Matthias Clasen from comment #28)
> I've added .ssd decoration in addition to decoration.ssd in Adwaita now.

Florian just moved ssd to window node. Now Adwaita must be updated by removing ssd from decoration node and then this bug can be closed. :)