Created attachment 137652 [details] [review]
First stab at this

Not completely finished, but would like to get comments about the general direction of this anyway if anyone can provide it.

Interesting experiment. 

Some things I noticed:

- No focus color change, which feels very disorienting to me

- The buttons take focus, which is probably unintentional. It might be useful to have a way to move focus up to the frame controls,  but they should probably not take focus on click

- You seem to always show all buttons right now, even if the window is not acutally iconifiable/maximizable.

- Similarly, you always change to the resize cursors, even if a window is not resizable.

- You don't show a window icon. People will probably miss it; whether to have it should probably  be controlled by the theme in some way

- Boo, no round corners :-)

- Metacity has tooltips on the frame buttons, which is a nice touch

- As you mentioned in irc, thinking about some way to let apps add widgets in the frame area (mostly titlebar) might be interesting.

"No focus color change, which feels very disorienting to me"

I just committed a change for this to my local repo (I'll post a new patch after I get some more of your items addressed).  But I'm not sure how to deal with this in the default theme, so my change only uses a different detail string to let themes draw the window differently for focused/non-focused windows:

+  if (window->has_toplevel_focus && window->is_active)
+    {
+      /* The window currently has focus */
+      gtk_paint_flat_box (widget->style, widget->window, GTK_STATE_NORMAL,
+                          GTK_SHADOW_NONE, area, widget, "base", 0, 0, -1, -1);
+    }
+  else
+    {
+      /* The window does not currently have focus */
+      gtk_paint_flat_box (widget->style, widget->window, GTK_STATE_NORMAL,
+                          GTK_SHADOW_NONE, area, widget, "base-unfocused", 0, 0, -1, -1);
+    }

Few more things:

The resize area needs to be larger than just the tiny corner rectangle. Probably needs a separate knob for that, and make it available to themes (maybe just a style property), so that themes can draw resize handles if they want.

As mentioned earlier, we should add something to the ewmh to trigger wm menu, so that we can have a menu button and right-click menu on the frame as usual.

Need to flip sides in LTR -- try running LANG=he_IL metacity

Finally: visual bell. Currently metacity is handling the visual bell, which either flashes the entire screen or just the window frame. With csd, metacity won't be able to do that. Not sure how metacity currently handles the window frame flashing for undecorated windows. Something to check out.

Looking at the code:

Should disable-client-side-decorations be a property (maybe construct-only) ?

Might want to use a single border-valued style propery instead of the top/bottom/left/right quadruple.

Should probably have separate icon names for min/max/close button images. Also, it is fairly common to have separate images for maximize/restore.

"Should disable-client-side-decorations be a property (maybe construct-only) ?"

There is already a style property on GtkWindow called "client-side-decorated" that defaults to false.  But that is the property that controls if this feature is enabled or not.

Other stuff I'll look at in the morning.  Thanks so much for looking at it!

> There is already a style property on GtkWindow called "client-side-decorated"
> that defaults to false.  But that is the property that controls if this feature
> is enabled or not.

I know. This is a separate thing. It just looked to me like gtk_window_disable_client_side_decorations should be the setter of a property.

Any update on this ? I would love to see this get in together with csw...

Created attachment 138009 [details] [review]
Updated patch

Sure, here's an updated patch that fixes some of the issues you raised (but not all yet).

- Buttons taking focus (now they can't be focused at all--we can figure out a better solution later)

- Don't always show buttons now (this is based right now on GdkWMDecoration and maybe should be based on GdkWMFunction instead)

- Fix so it doesn't show resize cursors all the time

- Added a window icon.

- Added tooltips on buttons.

Will look into other issues this week in Gran Canaria.

branch at http://github.com/bratsche/gtk/tree/csdeco (or I can push it to a branch on git.gnome.org if you prefer)

+  return client_side_decorated && window->decorated && priv->disable_client_side_decorations;

If you make that !priv->disable_client_side_decorations, stuff works much better.


Some other observations:

- need to optimize out some of the is_client_side_decorated calls. In some places you call that multiple times from a single function. 


- in ltr, the buttons now get reordered, but they are still on the right side... should move them to the left (and the window icon to the right).


- window titles need ellipsizing. E.g look a the 'color selection' testcase in testgtk.


- metacity still does a bit better when it comes to resize cursors. If you look at the testgtk main window, it can only be resized vertically due to wm size hints. Metacity is smart about not showing resize cursors on the left/right, etc.



Having a branch in git.gnome.org would be cool.

Created attachment 138079 [details] [review]
focus color patch

Here is a quick hack that makes focus color work for the default theme.

We should probably use this occasion to deprecate the old frame stuff:      
gtk_window_set_has_frame
gtk_window_get_has_frame
gtk_window_set_frame_dimensions 
gtk_window_get_frame_dimensions


Looking at your api:
gtk_window_set_client_side_decorations
gtk_window_get_client_side_decorations
gtk_window_disable_client_side_decorations

I don't think it makes sense like this. 

Just because we do decorations on the client side, we should not start to encourage app developers to micromanage window decorations like that.

Instead, we should do the same things the wm does to derive the right decorations from window type, geometry and other hints.

For forcing a window to be not decorated, gtk_window_set_decorated should be fine.

In short, I don't think we should have any new api for client-side decorations.

Wrt to figuring out what decorations to show, window managers actually do most of the hard work for us by exporting their supported actions in _NET_WM_ALLOWED_ACTIONS

Created attachment 138086 [details] [review]
max button toggle patch

Another quick patch to make the max button toggle its image.

If you don't mind, I might start putting some of these things on the branch...

I've committed some more fixes to the branch, many things work well now. 
Here is whats leftover:

- Implement a window menu

- Respect _NET_WM_ALLOWED_ACTIONS both for the menu and for click actions

- Use other icon names / stock ids

- Get rid of the new api; instead use _NET_WM_ALLOWED_ACTIONS + window type to determine decorations

- Consider allowing no frame (only title bar) when maximized, that seems popular among metacity themes

Few more things to figure out:

- window size hints need to be adjusted for the frame dimensions; right now, a client-side decorated window will come up with the decorations in the same area that the interior is given in the non-client-side decorated case.

- it seems the synchronized resizing does not quite work ? when rapidly dragging the right edge of gtk-demo outwards, I clearly see a stripe of window background appear before the window contents and client-side decorations are redrawn at the correct size.

- right-clicks should probably not start drags (since we want the have a menu there eventually), and clicks on the interior should probably never trigger a drag unless they are the button-modifier combination that the window manager already grabs for that purpose.

And one more:

- We need to draw window decorations even if the window is set as app-paintable.

And more:

- When decorations are turned off (e.g. xembed, like statusicons), don't do the cursor change dance either. 

- At some places we set the cursor to arrow - it is either pointless (ie when leaving the window) or trampling over the user-set window cursor

Created attachment 138358 [details] [review]
Proposed patch for cursor stuff

Regarding the cursor issues, do you think the following patch would be a good/correct solution?  If so I can commit.

Unfortunately, that still won't be good enough, I fear. It will still run over an application-provided cursor. We may have to add a gdk_window_get_cursor() to fix that, since there is no gtk-level api to set window cursors, so the common practise is gdk_window_set_cursor (widget->window).

TODO: Client-side drop shadows

This is going to depend on RGBA (or ARGB, or whatever order it's supposed to be) windows I think, which in turn will need us to mark certain regions of the window as opaque and communicate this to the compositor for performance optimizations.

If we're doing RGBA windows then this will also make it easy to do round corners with nice antialiasing, rather than using a shaped window.

Also, how do we measure the window size in that scenario?  Does the window size reflect the actual window primitive (which includes the drop shadows) or does it reflect the content size + window border sizes only?

We can do this with Xfixes (X11/extensions/Xfixes.h).. XFixesCreateRegion() will give us an XID and we can decide on a WM property name.

This is new to me, I'm just making a note of it here so I don't forget later.  krh is telling me this is the right way to do it. :)

Yeah, I think krh has a good handle on how to this in principle; just a small matter of writing the code to do it all.

The main reason for doing the shadows on the client as part of the decoration is that we want to make sure the window edge is well inside a GL texture.  This means that when transforming the window (rotating, wobbly, geenie effects etc) we get bilinear sampling along the window edge and thus avoid ugly jaggies.  The shadow fades to translucent towards the edge which means that the non-antialised edge of the GL primitive will be near-translucent and wont show.  Also, the shadow is part of the theme and overall look of the window, and it makes sense that we can theme it along with the rest of the window.  Doing the shadow client side also lets us piece it together from pixmap slices, whereas the wm would have to do an expensive blur of the alpha channel of the window to get the right shadow shape.  Finally, having every part of the window in one texture, makes the compositing simpler and more efficient (probably not much though).

To make this work we need to set two Xfixes regions as window properties (to be standardized in EWMH).  One that defines the *window content* region (_NET_WM_CONTENT_REGION), that is, the "real" part of the window, specifically, not the drop shadows or other padding.  The window manager use this for tiling windows, respecting strut and snapping etc when moving the window.  We can set this as the input region used by the X server as well using the XFixesSetWindowShapeRegion() request, so the X server will pass through clicks and other events in the shadow area of the window.  If the property isn't set, the WM should assume that the window content region is the window rectangle.

The other property we set (_NET_WM_OPAQUE_REGION) is a region that allows compositing mangers to optimize redrawing.  Most windows will have a large opaque area, even if their decoration, menu bar or drop shadow is transparent.  By setting a window property with an Xfixes region that identifies the opaque region to the compositing manager, the compositing mangager can avoid painting windows occluded by the opaque areas.

Matthias, do you have any ideas of what might be causing the resize/redraw to not be synchronized?  I'm not really sure yet how to try to debug that.

Actually, as I'm playing with this both in the client- and server-side decorated versions more I'm not really seeing a difference.  I see the bars on both of them when I expand the window.  It just seems more obvious on the client-side version right now because of the blue border, while the theme of my normal gtk has more of a grey border so it doesn't seem as distinct when you're increasing the window size.

Also, do you think we could have a style property to enable rgba windows now?  Then we could at least get drop-shadow support in now, and then add the optimizations that krh has in mind later.  If rgba defaults to being turned off then it won't have any negative performance impact in the mean time.

Created attachment 138505 [details] [review]
Cursor fu

Another stab at doing cursor support

> Matthias, do you have any ideas of what might be causing the resize/redraw to
> not be synchronized?  I'm not really sure yet how to try to debug that.

I think this is broken on the metacity side, probably.

Do you have any comment on the cursor fu I posted?  If it looks good then I can push it to git.g.o

Yeah, the cursor patch looks good to me. I think it misses one thing still: if you get a cursor change notification, you need to reset the appropriate cursor for the current position. Otherwise the app may set the cursor to something odd while the pointer is e.g in the titlebar region.

http://mail.gnome.org/archives/wm-spec-list/2009-November/msg00007.html

Created attachment 148862 [details] [review]
Updated patch

There's still stuff to do, but I figured I'd post an updated patch to show the progress in case there's any opportunity for some more incremental review to happen.  I've changed how it's setting the RGBA colormap and I've added initial support for client-side drop shadows.

(In reply to comment #25)
> To make this work we need to set two Xfixes regions as window properties (to be
> standardized in EWMH).  One that defines the *window content* region
> (_NET_WM_CONTENT_REGION), that is, the "real" part of the window, specifically,
> not the drop shadows or other padding.  The window manager use this for tiling
> windows, respecting strut and snapping etc when moving the window.  We can set
> this as the input region used by the X server as well using the
> XFixesSetWindowShapeRegion() request, so the X server will pass through clicks
> and other events in the shadow area of the window.  If the property isn't set,
> the WM should assume that the window content region is the window rectangle.
> 
> The other property we set (_NET_WM_OPAQUE_REGION) is a region that allows
> compositing mangers to optimize redrawing.  Most windows will have a large
> opaque area, even if their decoration, menu bar or drop shadow is transparent. 
> By setting a window property with an Xfixes region that identifies the opaque
> region to the compositing manager, the compositing mangager can avoid painting
> windows occluded by the opaque areas.

Fredrik Höglund responded to my emails about this saying that it would be preferable to store the rectangles directly in the property instead of using an XFixes region.

The reason is that if the window manager wants to fetch the rectangles,
it first has to make a synchronous call to fetch the ID, followed by another
synchronous call to fetch the rectangles, while it only has to make
one such call if the rectangles are stored directly in the property.

If the window manager needs an XserverRegion it would have to create
it, but this is an asynchronous call.

FYI I might try to do the opaque region optimization stuff in a second patch.  I don't think it's critical to have it in at the same time as all of this, and in the interest of time I don't want to block on the WM spec thing if possible.

If we do it using XFixes then I think the code looks something like this:

static void
gdk_x11_window_set_opaque_region (GdkWindow       *window,
                                  const GdkRegion *region)
{
  XserverRegion xregion;
  GdkDisplay *display;
  gint n_rects = 0;
  XRectangle *xrects = NULL;

  display = gdk_drawable_get_display (window);

  _gdk_region_get_xrectangles (region,
                               0, 0,
                               &xrects,
                               &n_rects);

  xregion = XFixesCreateRegion (GDK_DISPLAY_XDISPLAY (display),
                                xrects, n_rects);

  g_free (xrects);

  XChangeProperty (GDK_DISPLAY_XDISPLAY (display),
                   GDK_WINDOW_XWINDOW (window),
                   gdk_x11_get_xatom_by_name_for_display (display, "_NET_WM_OPAQUE_REGION"),
                   XA_CARDINAL, 32,
                   PropModeReplace,
                   (guchar *) &xregion,
                   1);
}


If we do without the XFixes region fu, then I think we're basically just setting xrects on the property instead of xregion.

Some comments from playing for a while with the current c-s-d branch:

The cursor handling needs some more work. 
 
- We are still defaulting to the wrong arrow. I think the code in gtk_window_enter_notify_event that sets the arrow cursor must just reset the cursor to NULL instead.

- When entering the gtk-demo main window from the left at just the right speed, I can get a stuck resize cursor. 


Maximized windows show artifacts.

- When I maximize the gtk-demo main window, the titlebar is drawn too big (the green titlebar extends halfway behind the tabs.


Client-side shadows clearly not ready for prime-time.

- Shadows are drawn much too strong, compared to e.g. the shadows of the metacity compositor.

- The only way to have shadows rendered without discontinuities seems to set all extents to the same value, e.g. 20 on all sides.

- When I set top and left extents to 0, I get no shadow on the right and bottom, instead the window frame gets stretched to fill the extents there

- When I set nonzero extents on the top and left and zero on the right and bottom, the titlebar background is drawn too short and part of it is missing behind the close button.

- We really should not draw client-side shadows if the wm doesn't understand the shadow region thing, the edge resistance feels very odd when it applies at the edge of the shadow, not at the edge of the window.
 

Sporadic warning I saw while playing around:
(lt-gtk-demo:12136): Gdk-WARNING **: gdk_window_begin_resize_drag: bad resize edge 11!


And yes, I do think we need the opaque region support if we want to turn rgba on by default.

Yeah, I was intending to pull the drop-shadows code out and try to push that into gtk+ as a different branch/patch later on.  I'll do this and fix the maximize draw issue right now.

The rgba thing is a little more of a problem.  Is there some way to make this configurable, maybe using gtksettings or something?  I'm not sure how to deal with this yet.  I'm not really sure how wm-spec-list works, but so far I haven't really gotten any resolution to the rgba issue or the shadow region issue.  So the rgba thing is a real blocker for this unless it can be made configurable somehow.

For the rgba thing, I would propose that we need to define an ewmh capability for opaque region (and further down, shadow region), and make gtk not use rgba visuals unless the wm supports them. See _NET_SUPPORTED.

I posted on the wm-spec-list about the rgba hint but it's kind of unclear how to proceed with it now.  But behdad said vuntz can help with this so I'll talk to him next.

punting to 3.0

Mark as duplicate of bug #694381 as the current effort id developed there

*** This bug has been marked as a duplicate of bug 694381 ***