Created attachment 202858 [details] [review]
window: Support GTK+'s hide-titlebar-when-maximized hint

For maximized windows, titlebars cannot be used to reposition or
scale the window, so if an application does not use it to convey
useful information (other than the application name), the screen
space occupied by titlebars could be put to better use.
To account for this use case, a setting for requesting that windows'
titlebars should be hidden during maximization has been added to
GTK+, add support for this in the window manager.

So, hmm, how do you unmaximize such a window?

Keyboard shortcut or dragging the toolbar (when using Adwaita). I also have a shell patch locally which extends the dragable area to unused space in the top bar.

The design notes mention Super+Drag to unmaximize, but that doesn't work on arbitrary window content in my tests

(In reply to comment #4)
> The design notes mention Super+Drag to unmaximize, but that doesn't work on
> arbitrary window content in my tests

Nope, because "mouse-button-modifier" is currently set to 'Alt' - I know the designers would like to have it changed to 'Super' (mostly because 'Alt' as mouse button modifier is commonly used by graphics applications), but there's a bug regarding the overview key which would need fixing first (see bug 665616)

Review of attachment 202858 [details] [review]:

I guess we'll see about whether people figure out unmaximize.

It feels a little wasteful to me to use a whole property round-trip for one bit, but I suppose adding a _GTK_FRAME_FLAGS property with one defined bit would be silly future-proofing, and it's good to keep this isolated as a somewhat experimental feature.

::: src/core/constraints.c
@@ +443,3 @@
    */
   if (meta_prefs_get_force_fullscreen() &&
+      !window->hide_titlebar_when_maximized &&

Think this could do with a comment above, e.g., 

 'Don't get fooled by a title-bar hidden, maximized window;
  that's not the same as fullscreen, even if there are no
  struts making the workarea smaller than the monitor.'

::: src/core/window-props.c
@@ +1616,3 @@
+      META_WINDOW_MAXIMIZED (window) &&
+      window->frame)
+    meta_ui_reset_frame_bg (window->screen->ui, window->frame->xwindow);

A) Ugh ... the only current usage of this is inside meta-frame.c, this strikes me as an encapsulation leakage
B) Won't we hit meta_frame_sync_to_window() if it matters and we add or remove the frame
C) At least add some meta_frame_ wrapper to avoid window->frame->xwindow access?
D) Isn't it wrong to have the !window->hide_titlebar_when_maximized - do we need to do this when both adding and removing the frame, not just in one direction?

::: src/meta/common.h
@@ +55,3 @@
   META_FRAME_TILED_LEFT               = 1 << 15,
+  META_FRAME_TILED_RIGHT              = 1 << 16,
+  META_FRAME_TITLEBAR_HIDDEN          = 1 << 17

This strikes me as strongly duplicating META_FRAME_TYPE_BORDER which is a window with titlebar hidden; can we combine them in some fashion? Either get rid of FRAME_TYPE_BORDER in favor of this (if we want to support different appearance for different frame types with the titlebar hidden), or just use FRAME_TYPE_BORDER.

Created attachment 203036 [details] [review]
window: Support GTK+'s hide-titlebar-when-maximized hint

(In reply to comment #6)
> ::: src/core/constraints.c
> @@ +443,3 @@
>     */
>    if (meta_prefs_get_force_fullscreen() &&
> +      !window->hide_titlebar_when_maximized &&
> 
> Think this could do with a comment above

Done.


> ::: src/core/window-props.c
> @@ +1616,3 @@
> +      META_WINDOW_MAXIMIZED (window) &&
> +      window->frame)
> +    meta_ui_reset_frame_bg (window->screen->ui, window->frame->xwindow);
> 
> A) Ugh ... the only current usage of this is inside meta-frame.c, this strikes
> me as an encapsulation leakage

Not sure I agree - I don't see much of a difference to meta_ui_(un)map_frame, meta_ui_set_frame_title, meta_ui_queue_frame_draw or meta_ui_update_frame_style, all of which are used outside of meta-frame.c.


> B) Won't we hit meta_frame_sync_to_window() if it matters and we add or remove
> the frame

Usually yes, but I've seen flashes/artifacts when toggling the hide-titlebar and prefers-dark-theme properties in gtk3-demo (which is admittedly a corner case, as neither property is expected to actually change)


> C) At least add some meta_frame_ wrapper to avoid window->frame->xwindow
> access?

Not sure I'd deem that necessary - there are already precendents for accessing window->frame->xwindow from window-props.c/window.c (see (A))


> D) Isn't it wrong to have the !window->hide_titlebar_when_maximized - do we
> need to do this when both adding and removing the frame, not just in one
> direction?

The assumption was that if hide_titlebar_when_maximized == TRUE, the visible frame area would decrease, so the existing background would remain valid. Can't hurt to update anyway, so I removed that condition.


> ::: src/meta/common.h
> @@ +55,3 @@
>    META_FRAME_TILED_LEFT               = 1 << 15,
> +  META_FRAME_TILED_RIGHT              = 1 << 16,
> +  META_FRAME_TITLEBAR_HIDDEN          = 1 << 17
> 
> [...]
> just use FRAME_TYPE_BORDER.

OK. When I played around with that, it did not remove the titlebar from windows (though the titlebar itself ended up empty) - apparently Adwaita's metacity theme was incomplete there (fixed in master), but it still leaves the question whether we should enforce has_title=false for META_FRAME_TYPE_BORDER.

Review of attachment 203036 [details] [review]:

Looks good to me

Attachment 203036 [details] pushed as 9729a99 - window: Support GTK+'s hide-titlebar-when-maximized hint