Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Advertise support for _GTK_FRAME_EXTENTS to make Gnome 3 CSD windows display correctly. #8

Closed
wants to merge 2 commits into from

Conversation

blackhole89
Copy link

The client-side decorations rendered by Gnome 3 apps such as nautilus use the _GTK_FRAME_EXTENTS atom to distinguish the window's body from the (also client-rendered) drop shadow. If support for it is not advertised by the window manager, all such apps render with thick solid black borders.

This small patch fixes that problem, though it doesn't take care of the issue that Compiz still uses the "outer" border (including the shadow area) to determine the extents of the window for the purposes of management and so, among others, is reluctant to allow moving an affected window further up than some 20px below the lower edge of a top panel.

@XRevan86
Copy link
Collaborator

XRevan86 commented May 6, 2016

I tried doing so: 92cb144, be31466, 8f8e6ec, but 03873f3.
There are lots issues caused by Compiz not being able to distinguish windows from their shadows.

@blackhole89
Copy link
Author

Hm. I'm not sure I understand what breaks from advertising _GTK_FRAME_EXTENTS support alone (without undertaking any attempt at handling it correctly) - is there anything? I understand it's not a perfect solution, but it seems strictly better than the current situation...

@XRevan86
Copy link
Collaborator

is there anything?

Yes, everything that shows window contents like an application switcher.

it seems strictly better than the current situation

@blackhole89, only if your theme lacks proper solidcsd support :-).

@raveit65
Copy link
Collaborator

Here is an example of proper solid-csd support in themes
https://github.com/mate-desktop/mate-themes/blob/gtk3.18/desktop-themes/BlueMenta/gtk-3.0/gtk-widgets.css#L4823
Note, for gtk+-3.20 the syntax is different.

@lukefromdc
Copy link
Contributor

I just tested this. It works and makes CSD apps (and tooltips) render properly, but unlike in the 0.9 branch breaks several plugins. Grid and animation in particular don't work. Rolling back to 0.9.3 for the moment (with Albert's original patch) but don't know how long Ubuntu will be maintaining it

@lukefromdc
Copy link
Contributor

I've traced the change that breaks plugins interacting with open windows to the actual listing of

Atom frameGtkExtentsAtom;
in compiz-core.h

no matter what else is changed or not changed. That change isn't enough by itself to tell GTK to treat compiz as a compositor but its enough to break the plugins. Probably this means all the plugins requiring window interaction need work to use this. In the 0.9 series by comparsion no plugin seems to be bothered and the equivalent change works fine so long as you value CSD window transparent window borders more than gapless tiling of the same windows.

Ubuntu actually patches GTK itself rather than merging any patches for this in compiz, with a more complex patch set than just disabling the offending line in the GTK source code.

@soreau
Copy link
Member

soreau commented Apr 26, 2017

I installed this and now able to reproduce the size problem with gnome-mines. Gridding the window takes into account the shadow area so it's the wrong size. I cannot reproduce the black-border issue but I can imagine how that might happen. Can anyone outline what's supposed to happen to make the clients get the correct size and function properly? Perhaps point to 0.9 patch(es) that fix the problem.

@lukefromdc
Copy link
Contributor

There is a branch of 0.9 with this change and changes to several plugins to stop the CSD window size problem, I would imagine similar changes would also be needed to plugins like animation that are broken outright here with this commit:

https://code.launchpad.net/~muktupavels/compiz/gtk-frame-extents

@soreau
Copy link
Member

soreau commented Apr 26, 2017

@lukefromdc Thanks but the commits in that repo are pretty vague.. it would be useful to be able to distinguish the gtk csd windows from others.

@lukefromdc
Copy link
Contributor

lukefromdc commented Apr 26, 2017

Honestly I don't know a whole lot about that myself, only that the modified 0.9 branch mostly works with a few tiling issues, notably with vertically maximized CSD windows first coming up too large until taken out of focus and back into focus. At that point they snap to the correct size. Quarter screen tiling (all I use) works perfectly, as do transparent backgrounds for rounded CSD corners and tooltips.

@lukefromdc
Copy link
Contributor

One pointer: look at the commits referring to plugins. Very few actual lines of code are changed in them.

@lukefromdc
Copy link
Contributor

Update: I found out by accident that as of May 28, 2017 and on Debian Unstable, rebuilding the plugins caused the grid and animation plugins (and probably anything else broken by this) to work again. No idea what changed but must be something in the underlying system. Confirmed this by reinstalling old builds, at which point animation and grid stopped working, then installing the new builds, at which point these plugins worked again.

@lukefromdc
Copy link
Contributor

I've now got an entirely funtional build of compiz-reloaded with this applied to master as of 5-28-2017, all new build. Plugins work fine. Modified my theme to limit the area covered by shadows on CSD windows only to minimize issues on moving or tiling them.

The commits to deal with this in 0.9 in
https://code.launchpad.net/~muktupavels/compiz/gtk-frame-extents
rely on w->clientFrame and w->border variable to subtract the necessary area from window rectangles. If someone could point me to equivalent variables or a way to get them into the plugins, I might be able to port these changes over. For now limiting CSD shadows to 3px helps a lot.

lukefromdc added a commit to lukefromdc/compiz that referenced this pull request Sep 26, 2017
@soreau
Copy link
Member

soreau commented Sep 30, 2017

@lukefromdc I'm just now reading your comments from May, sorry for the delay. In your last post, you said the plugins 'work' fine but there are still issues with moving and 'tiling' windows? by tiling do you mean grid or tile plugin? I'm guessing the plugins don't all work fine if more work is required. Can you detail this a bit more? What applications are known to exhibit the problem? I'll work on trying to map those patches to the reloaded code base.

Also I wanted to ask what setup is required to reproduce this? The problem with my current setup is I am not able to reproduce the black border problem, though grid resizes the window with a noticeable shadow gap around the window.

@soreau
Copy link
Member

soreau commented Sep 30, 2017

4108: Search src/window.c for serverBorderWidth.
4109: Likely src/paint.c addWindowGeometry()
4110: Covered by your patch
4111: src/move.c
4112: src/move.c
4113: src/resize.c
4114: plugins-extra/src/grid.c

There's not a one-to-one mapping, so not super helpful but the commits seem to rely on clientFrame left/right/top/bottom for the window pretty heavily. With reloaded, the border (serverBorderWidth) is a single value, so I'm guessing it would need to be split into 4 and set accordingly.

If I could reproduce the problem fully, it would be easier to help fix obviously.

@lukefromdc
Copy link
Contributor

Moving and tiling now work. Not sure what prevented tiling and several other plugins from working AT ALL back in the spring, but I suspect either something in underlying libs got more tolerant or just that I had not rebuilt those packages after applying this PR to compiz. As of now, if I rebuild compiz without this PR and try to run it with plugins built over compiz built with this PR, the result is compiz crashes and launches the fallback WM. Thus to bisect anything else I have to apply this each time when working with my existing plugins builds. Have not retested compiz with this PR and plugins without, but it looks like this is enough of an ABI change to force rebuilding all the plugins.

The remaining issues are simply that tiling and moving CSD windows leave gaps, as compiz is not taking the GtkFrameExtents into account in computing how much space to allocate. This can be limited by setting the space allocated for shadows to be as small as possible in the theme(I use 3px).

https://code.launchpad.net/%7Emuktupavels/compiz/gtk-frame-extents now includes fixes for the gaps in tiled/moved CSD windows, but a few bugs remain. Much of that could be applied here if I could just find the equivalent variables to work with.

I use this PR in my local builds of compiz as my theme really needs full compositing support from GTK, which since 3.16 sets windows it considers CSD (including many popup windows) to be noncomposited if GtkFrameExtents support is not claimed. This was also why the support for panel themes different from the GTK theme I added to mate-panel and its applets had to directly detect RGBA visuals and force transparency support. For me, this is much more important than zero-gapped tiling.

Try this theme (my theme) with MATE 1.18 or later and see what I mean:
https://github.com/lukefromdc/gtk-theme-ubuntustudio-legacy

Set up grid to quarter-tile windows on corners, and tile some CSD windows with some normal SSD windows. The SSD windows will tile without gaps, and you will see 3px gaps on things like gedit or gnome-disks (CSD apps). You will see much larger gaps on CSD tiled windows with some themes that do not contain workarounds for this.

@lukefromdc
Copy link
Contributor

I just did a test, setting serverBorderWidth to 0 in src/window.c every time it appears had no effect at all on gaps in grid tiling CSD windows. Same for a hardcoded test value of 6 for that matter. Gedit and gnome-disks were tested

@soreau
Copy link
Member

soreau commented Oct 1, 2017

Here with gtk3 versions of gedit or gnome-disks without this patch, the windows are resized correctly by grid, resize normally and do not have a black border. gtk 3.18.9, xubuntu 16.04. Is there something I might be able to run in virtualbox to reproduce the problem?

@lukefromdc
Copy link
Contributor

I have literally never experimented with virtualization, all testing I do has always been on bare metal.

First of all, without this PR, those CSD windows should resize just fine, but will only look decent if your theme explicitly styles the .solid style class to use square borders and no box shadows. Otherwise you get wide black borders and/or black tips extending past rounded corners. Thus you are getting the expected result if not using this PR, which fixes GTK 3.16 and later not considering compiz a compositor when drawing any CSD window, including menus, tooltips, and other popup windows as well as toplevel windows for applications such as Gedit.

The resizing issue is a side effect (in this PR) of advertising GTK_FRAME_EXTENTS support when window dimensions are not actually calculated with them taken into effect.

All "normal" windows like the toplevel windows MATE makes resize just fine with or without this PR as well. It's only CSD or client-side decorated, GNOME style windows (with header bars) that get a spacing gap when this PR is used, and this is not going to be an issue unless they are tiled. if maximized (.maximized style class thus used), it seems GTK by default allocates no extra space (the gap) for box shadows etc so no gap is seen. You won't see this problem on menu, tooltips, calendar popups etc because they are never tiled or moved.

The gap will only be seen on a CSD window, only with this PR applied, and mostly on dragging it to the top or on tiling it. Maximize works fine, and CSD windows can be dragged over oneanother so also can be dragged next to oneanother and in contact. Dragging to the sides works fine as with multiple desktops there is no constraint in that direction, and dragging to the bottom works fine. Dragging to the top you will see the gap, and you will see it all the way around a tiled window. The fixes in https://code.launchpad.net/%7Emuktupavels/compiz/gtk-frame-extents fix most but not all of this problem.

@zezic
Copy link
Contributor

zezic commented Oct 3, 2017

Can I ask if this PR plays correctly with Blur plugin? Or we get blurred image under CSD shadows-area as well as under main window area?

@lukefromdc
Copy link
Contributor

The Blur plugin seemed to work OK, though I've never tested it without this PR (plugins have to be rebuilt to switch it in or out) and its not one I've used before so not sure the expected behavior. I do have ALL of the plugins (including experimental) installed. Anyway, no artifacts or screwy behavior came up.

@lukefromdc
Copy link
Contributor

Did more testing with the blur plugin(which I do not normally use). Focus blur works fine and the area around a blurred window does not seem to blur the desktop beneath(assuming that's what you were asking). Alpha blur (the default) does NOT work on any CSD window (menus and dialogs included) , but no idea if this PR had anything to do with that. To test that I would have to rebuild every plugin as well as compiz without this PR, as plugins built with this PR in place are not compatable with compiz built without.

@lukefromdc
Copy link
Contributor

What I got with alpha blur on a CSD window was the alpha blur remaining in effect when the window was supposed to be focussed, thus making the window impossible to use. My mate-panel, all menus, and all dialogs were included in this.

@zezic
Copy link
Contributor

zezic commented Oct 17, 2017

In general I talk about alpha blur. Current behavior allows to blur image UNDER semi-transparent areas of window, such as semi-transparent terminals, docks or panels which is good and usable feature.

What I mean in my question is ability of blur plugin with this PR to blur image UNDER semi-transparent areas of GTK3 windows including CSD but WITHOUT bluring image under CSD-shadows area.

@lukefromdc
Copy link
Contributor

The entire Blur plugin is one I have never used as my workflow requires interaction with multiple windows, so I have literally never seen its intended behavior. I found alpha blur entirely unusable as the entire area of all CSD windows was alphaed out to a somewhat yellow transparency with little foreground detail showing.

No idea if this PR is responsible. Using AMD HD6750 with r600g graphics on Debian Unstable. I do occasionally get other graphical glitches involving a missing color channel, randomly on some comboboxes and that might be related. I have not run compiz-reloaded without this PR long enough to tell if that is related either, as my theme and whole setup absolutely requires this PR, compiz 0.9 with the comparable PR, or not using compiz.

At any rate, I have not seen the alpha blur function cause any unwanted effects in the shadow area surrounding CSD windows. All the problems are in the window itself.

I used https://code.launchpad.net/%7Emuktupavels/compiz/gtk-frame-extents from the time it came out until recently switching to compiz-reloaded with this PR applied. I found the behavior of GTK 3.16 and later in compiz without these to be totally intolerable-and otherwise my setup would require patching GTK itself to revert the offending commit, which is
https://git.gnome.org/browse/gtk+/commit/?id=5ced234144ce63decbf5afc8a3517290b9027018

@lukefromdc
Copy link
Contributor

Re: black borders:
You will not see these with all themes, but won't see rounded borders either in any theme without this PR.

The black border issue is theme-dependent, it is simply space in the window that nothing is drawn in until the theme tells it to, with the window extended outward by the greater of the margin or the longest box-shadow radius. Better written themes use the .solid class to set box shadows to "none" and the margin to just a couple px, plus draw a background color matching the decoration in that space so you never see a black border no matter what your WM.

What no theme can do with GTK considering the WM not to be a compositor is draw a rounded border on a window or a box-shadow, as these require the underlying window to be transparent and slightly larger than the visible application window. With https://git.gnome.org/browse/gtk+/commit/?id=5ced234144ce63decbf5afc8a3517290b9027018 in GTK, a window manager not supporting _GTK_FRAME_EXTENTS is treated as not being a compositing WM at all.

@soreau
Copy link
Member

soreau commented Oct 21, 2017

If I recall correctly, with this patch there is some problem with grid and other plugins regarding the shadow area of the window. The problem I have, is distinguishing this type of window from others so that they can be special cased in the relevant plugins. If needed we could add a variable to CompWindow and set it at window creation time, but what does the conditional code look like for this? What code would be needed to know if a window is gnome 3 CSD or should be treated as such?

lukefromdc added a commit to lukefromdc/compiz that referenced this pull request Jan 25, 2018
lukefromdc added a commit to lukefromdc/compiz that referenced this pull request Feb 8, 2018
@lukefromdc
Copy link
Contributor

Note that
https://github.com/lukefromdc/compiz/tree/gtk-frame-extents
is kept rebased and current with master. No PR as this one already exists, but if you simply want to build current compiz-reloaded with this PR applied my branch is the easiest way to do so. Just download a tarball and build, making sure to rebuild your plugins after installation as some plugins will break if you do not.

@soreau
Copy link
Member

soreau commented Feb 8, 2018

@lukefromdc Thanks for keeping up with this. I still do not have the problem locally and don't intend on looking into it further until it happens, or there's a specific test case to reproduce the problem. This PR doesn't address the associated caveats in various relevant plugins though it should, which is what primarily needs attention.

@lukefromdc
Copy link
Contributor

I use a theme that has a great deal of transparent effects and rounded corners, and that makes the issue this PR addresses really a problem. Thus my branch is required for my own use case, and offered to anyone else who can benefit from it. Even if this PR is never merged because of the effects on tiling (less important in my use case and with my GTK theme), I can still use it and so can other users of my theme.

If you want to see what I mean, here is my theme:
https://github.com/lukefromdc/gtk-theme-ubuntustudio-legacy

Look at CSD windows and their edges and corners, look at tooltips, and if you have gnome-panel installed its menus also might still come up with a solid black background and square corners protruding beyond the grey rounded border lines. In mate-panel and its applets I hardcoded the panel menu windows to check the visual rather than use GTK's judgement on whether the WM is a compositor or not.

Also keep in mind that if you are testing on Ubuntu, Ubuntu patches GTK itself or did so through 17.04. They may not need to anymore because they dumped Unity (and compiz with Unity) for GNOME. I hope the keep doing so because Ubuntu MATE with compiz 0.9 will still benefit from their GTK patch, as the equivalent patch for compiz 0.9 also has not been merged despite better behavior with tiled windows than here. I am using Debian Unstable and building vanilla GTK 3.22 locally.

I have yet to find a way to get all the move/resize/tile plugins to properly account for GtkFrameExtents , I don't know if that will ever be solved. This has been an issue all the way back to when the GNOME devs merged
https://git.gnome.org/browse/gtk+/commit/?id=5ced234144ce63decbf5afc8a3517290b9027018
the offending commit into GTK 3.15.

Reverting that commit is the other option for a local fix, but I would imagine the behavior would be exactly the same because that commit just causes the function to detect compositing to return false if GtkFrameExtents are not supported.
That commit, in turn, reverted a previous commit that removed the GtkFrameExtents requirement during the GTK 3.12 cycle, which had been needed to stop double header bar issues, but then gave a problem in QT/KDE apps (or their WM?) with shadows on CSD windows. Ubuntu did NOT simply re-revert this, they wrote another patch (actually a set of patches) for GTK.

@lukefromdc
Copy link
Contributor

lukefromdc commented Feb 8, 2018

Just found out why the .tiled style class never showed up in my tests: the X11 backend for GDK only sets .tiled if either _GTK_EDGE_CONSTRAINTS is supported by the WM and any edge is tiled, or if _GTK_EDGE_CONSTRAINTS is not supported, then only windows which are vertically maximized get the .tiled style class and get treated by GTK as tiled.

EDIT: This is why the tiled window gap issue has never been fixable in themes, thus cannot be hardcoded out with a cssprovider in compiz

Here is the comment in question above some complex code in gdkdisplay-x11.c in GTK3:


  /* If the WM doesn't support _GTK_EDGE_CONSTRAINTS, rely on the fallback
   * implementation. If it supports _GTK_EDGE_CONSTRAINTS, arrange for
   * GDK_WINDOW_STATE_TILED to be set if any edge is tiled, and cleared
   * if no edge is tiled.
   */
  if (!gdk_x11_screen_supports_net_wm_hint (screen,
                                            gdk_atom_intern_static_string ("_GTK_EDGE_CONSTRAINTS")))
    {
      /* FIXME: we rely on implementation details of mutter here:
       * mutter only tiles horizontally, and sets maxvert when it does
       * and if it tiles, it always affects all edges
       */

@lukefromdc
Copy link
Contributor

lukefromdc commented Feb 23, 2018

lukefromdc@ea65b7b
Window: Add clientFrame(), to get the eventually set CSD extents

And to make the grig plugin really work,
lukefromdc/compiz-plugins-extra@f51d181

I finally found the fix for getting the grid plugin to work right with CSD windows, will now work on move, resize etc all to consider GtkFrameExtents in dealing with CSD windows. It turned out this work had all been experimented with before back in early 2016, only to be reverted to stop a memory leak. I will be using this daily and will monitor memory use to see if that problem still exists. If it does it will need to be found.

Here's what I got combining this work with
https://github.com/lukefromdc/compiz/tree/dev-focus_WIP

windows_in_grid

@lukefromdc
Copy link
Contributor

lukefromdc@f20c043
Handles moving CSD windows, and although the position of the mouse on the top border can be a little touchy allows moving them to the top, same as SSD windows. Note that plugins such as grid providing "edge resistance" make it hard to move the exact top of any kind of window to the exact top of the screen.

@lukefromdc
Copy link
Contributor

lukefromdc commented Feb 23, 2018

So far the resize plugin has defeated me, with applying the "paint box" portion of
https://bazaar.launchpad.net/~muktupavels/compiz/gtk-frame-extents/revision/4113
simply not enough to allow resizing CSD apps to the screen edges.

This diff builds and runs but does not fix resizing:


--- /home/luke/Desktop/Development/Compiz-Reloaded/compiz_0.8.15+focus+extents20180223/plugins/resize.c	2018-01-24 20:56:46.919275000 -0500
+++ /home/luke/Desktop/COMPIZ_CHANGES/EXTENTS2/Plugins/resize/resize.c	2018-02-23 01:19:00.813150518 -0500
@@ -147,21 +147,21 @@ resizeGetPaintRectangle (CompDisplay *d,
 {
     RESIZE_DISPLAY (d);
 
-    pBox->x1 = rd->geometry.x - rd->w->input.left;
-    pBox->y1 = rd->geometry.y - rd->w->input.top;
+    pBox->x1 = (rd->geometry.x - rd->w->input.left) + rd->w->clientFrame.left;
+    pBox->y1 = (rd->geometry.y - rd->w->input.top) + rd->w->clientFrame.top;
     pBox->x2 = rd->geometry.x +
-	rd->geometry.width + rd->w->serverBorderWidth * 2 +
-	rd->w->input.right;
+	    (rd->geometry.width + rd->w->serverBorderWidth * 2 +
+	    rd->w->input.right) - rd->w->clientFrame.right;
 
     if (rd->w->shaded)
     {
-	pBox->y2 = rd->geometry.y + rd->w->height + rd->w->input.bottom;
+	pBox->y2 = (rd->geometry.y + rd->w->height + rd->w->input.bottom) - rd->w->clientFrame.bottom;
     }
     else
     {
-	pBox->y2 = rd->geometry.y +
+	pBox->y2 = (rd->geometry.y +
 	    rd->geometry.height + rd->w->serverBorderWidth * 2 +
-	    rd->w->input.bottom;
+	    rd->w->input.bottom) - rd->w->clientFrame.bottom;
     }
 }
 

@soreau
Copy link
Member

soreau commented Feb 23, 2018

@lukefromdc These patches look much better than previous ones. I believe the reason the resize patch doesn't work is because that code is only for drawing the rectangle in case of outline or rectangle modes. It looks like resizeHandleMotionEvent() is where the logic would need to go.

@lukefromdc
Copy link
Contributor

I'll play around with that some more tomorrow.

@lukefromdc
Copy link
Contributor

lukefromdc@a2332eb

Mostly fixes resizing, I ran into a bug where gnome-disks with this will quarter-tile too deep with the grid plugin while gedit tiles correctly, no idea what's different between those two GNOME apps. Manual resizing now can go to the edge or very close to it(in the edge-resistance case) for CSD apps the same as ssd apps. Had to include checks to prevent ssd apps from tiling or maximizing too large

@lukefromdc
Copy link
Contributor

lukefromdc@ef4bbfd
Cleans up the resize code, simpler and seems to work better. Grid still needs a little work (in compiz-plugins-extra) as in the half-maximized cases it does not compute the extents in the direction it is fully maximized and still leave a gap.

Also, the "tile" plugin in compiz-plugins-experimental has a major problem, unknown if it relates to this work, the focus work, or is present with compiz 0.8.15 from master. If "tile" is enabled, Firefox comes up in a window that is never shown but appears in the taskbar and cannot be unminimized, and mate-panel's window also tends to "get lost." turning "tile" off makes those windows appear, and always they have been resized rather narrow. Tile itself works with the showing windowsx, though my first attempt at fixing the extents gap has not yet produced results.

@lukefromdc
Copy link
Contributor

In my combined focus/extents branch I seem to have no memory leak(the reason _GTK_FRAME_EXTENTS support was originally reverted) at all, witha 3 1/2 hour video editing session with one monitor some of the time and two monitors some of the time keeping 59MB of RAM in use for compiz

@ethus3h
Copy link
Contributor

ethus3h commented Feb 25, 2018

@lukefromdc Just so you know, there's a handy tool called Valgrind that can spot memory leaks more reliably, I think, than just looking at an app's used memory. It's pretty straightforward, here's their quick-start guide http://valgrind.org/docs/manual/quick-start.html

(bear in mind that it might show already existing problems, so a leak it reports might not necessarily be introduced by the patch! ;)

@lukefromdc
Copy link
Contributor

I just traced the issue in the "tile" plugin to the "join windows" feature which is itself flagged "experimental." Plugin seems to work fine without it so I can proceed to work on _GTK_FRAME_EXTENT support for it

@soreau
Copy link
Member

soreau commented Feb 26, 2018

@lukefromdc Good work. I think now that you have the initial groundwork laid to get the correct size values in lukefromdc/compiz@ea65b7b it's a great start to the solution to this problem. There will be plenty of caveats for other plugins. I wouldn't worry too much with the experimental plugin set though it's nice to see you are looking for bugs to fix preemptively. I think the major concerns are making sure the window size and related behavior work as expected, including maximizing and resizing especially. Note that resize can happen by dragging on the edge of a window and also by keybinding, alt+middle click drag by default, so try to test both cases. If you can focus on a patch set for compiz that addresses adding the initial support and then addressing the plugins included in core, we could work to get that finalized for upstream inclusion and go from there to patch other plugins. The one thing I wonder about is whether this support should be optional but it seems it wouldn't hurt to include it unconditionally.

@lukefromdc
Copy link
Contributor

Thanks! I just fixed a bug in the "grid" plugin where my first work (tested with my regular quarter-tiled case) caused left-half or right-half windows to render oversize. Apparently GTK3 special cases that as Mutter supported it, so the correction for GtkFrameExtents had to be removed from that case, giving perfectly left and right tiled windows. Something else is still constraining top and bottom tiled windows, I am trying to find it.

lukefromdc/compiz-plugins-extra@e3034b7

When I get this done it will need more testers due to different use cases-and to see if issues crop up with different video drivers etc. We don't want to get back the old "compiz is buggy" reputation after all. The Grid plugin I am working on because it is one I use and need. Thanks for the tips about various ways of resizing windows, I will need to test as many of these cases as I can.

The issue with the grid and top or bottom tiled window seems to relate to some kind of inconsistancy that can also on rare occasions cause all windows not to maximize quite all the way until compiz is restarted. That can also be invoked some of the time but not all of the time by tiling windows left or right, then switching to quarter-tiling. My guess is something gets set but not always unset and I need to find it. Could be in grid itself or maybe in resize?

@lukefromdc
Copy link
Contributor

Found https://github.com/compiz-reloaded/compiz/issues/113 which also affects master while testing resize

@lukefromdc
Copy link
Contributor

lukefromdc@3350ba4
fixes the intermittant maximization problem, found it could be reliably triggered by resizing a CSD window beyond the screen (after moving it partially offscreen), then maximizing it. Turned out the function applying GtkFrameExtents was saving that to a permanent value elsewhere and thus could apply it more than once. By saving and restoring the original value I seem to have put an end to the worst bug in my branch.

@lukefromdc
Copy link
Contributor

lukefromdc commented Feb 27, 2018

lukefromdc/compiz-plugins-extra@4c198a4
for the "grid" plugin fixes left and right side gaps on top or bottom tiled windows. It is actually a workaround for horizontal maximization not working right with csd windows and GtkFrameExtents as or now. The code in window.c for handling horizontal maximization looks the same as the code for vertical maximization yet works differently. Both vertical and fullscreen maximization now work without issue. Are there any ways of triggering horizontal maximization other than the "grid" or "tile" plugins?

EDIT: I suspect the underlying issue is that GTK treats fullscreen and vertical (left or right) tiled windows differently, probably simply setting GtkFrameExtents to zero where the edges are reach the edge of the screen.
(see #8 (comment) above )
For WM's that do not support _GTK_EDGE_CONSTRAINTS, gtk uses a fallback implementation that does not consider any window tiled except a left side or right side tiled (vertically maximized) window. Thus, it is probably the behavior of GTK itself I had to work around, unless we want to also do the work to support _GTK_EDGE_CONSTRAINTS, which looks as complicated as this work and we should be able to avoid with this workaround.

@lukefromdc
Copy link
Contributor

lukefromdc commented Feb 27, 2018

lukefromdc/compiz-plugins-experimental@b867cf5
is an initial fix for the "tile" plugin, which works with most themes (maybe all as extent distance does not vary that much) EDIT: so long as "join windows" is not enabled. Odd behavior in it, it needed decimal correction factors for this. Still, at this point I have all the known ways I move and resize windows working with CSD windows and GtkFrameExtent support. What have I missed?

@lukefromdc
Copy link
Contributor

Running compiz under Valgrind after reinstalling compiz (just the core package with its plugins, not all packages) from make-install to include debugging information got this log, I could not find any references to my own work unless something in grid (not reinstalled with debugging info) came from my work:

Compiz_Valgrind_Log.gz

This looks like there are plenty of small memory leaks (33097 errors from 6 contexts) on a short run including moving, resizing, maximizing, and quarter-tiling a gnome-disks CSD window, but that I seem to have avoided introducing any new once traceable directly to the extents code. This is with the combined extents/focus branch. Compiz was initially very slow to respond under Valgrind to the point of unusability but then got going and was actually usable. Runtime for this was maybe ten minutes.

@lukefromdc
Copy link
Contributor

lukefromdc@2ae00b7
and companion commits to compiz-plugins-extra
lukefromdc/compiz-plugins-extra@80096c9
and
lukefromdc/compiz-plugins-extra@85569f4

give much better usability of the "grid" plugin with CSD windows, allowing the same behavior as with normal SSD windows. Note that GTK themes must not set different shadow dimensions for the .tiled style class with this. I had added that to my theme at least a year ago in a failed attempt to fix gaps in both versions of compiz and it never worked. I could not find any other theme doing that so I took it out of mine, and found that there was no way I could get the grid plugin to work the same both with and without it in the theme. Some way to catch this would be desirable, but for now IO documented it in the code as a FIXME

lukefromdc added a commit to lukefromdc/compiz that referenced this pull request Apr 3, 2018
lukefromdc added a commit to lukefromdc/compiz that referenced this pull request Apr 3, 2018
@lukefromdc
Copy link
Contributor

Just rebased everything after the latest commit to master-one that might also be a framework for saving geometry of windows being quarter-tiled at some point. Didn't get any nasty interactions from it, works, and something has fixed the issue I was having (with the focus work) of not all windows coming up when restarting compiz. Still have mate-panel not being rerendered (or maybe rendered underneath caja-desktop) on a reload from fusion-icon but that is shared with master. At this point, my combined branch is working very nicely for me.

lukefromdc added a commit to lukefromdc/compiz that referenced this pull request Jun 3, 2018
soreau pushed a commit that referenced this pull request Jun 16, 2018
* Advertise support for _GTK_FRAME_EXTENTS

Apply #8

* Window: Add clientFrame(), to get the eventually set CSD extents

Re-apply 8f8e6ec
"Window: Add clientFrame(), to get the eventually set CSD extents"

and 8f8e6ec
"Restore ABI compatibility broken by the previous commit"

This is necessary to allow plugins such as "move" and "grid" to accomodate CDS extents and shadow areas

* Move: account for GtkFrameExtents on CSD windows

Based on
https://bazaar.launchpad.net/~muktupavels/compiz/gtk-frame-extents/revision/4109
Note that moving CSD windows to the very top is still a little touchy about grab position
but now they can be moved to the top of the screen

* Resize: take GtkFrameExtents into account

* Resize: clean up and simplify GtkFrameExtent support

Also works better this way

* resize: Fix intermittant failures to properly maximize windows

Do not save the added GtkFrameExtent values to the stored workArea

* Window-adjust for GtkFrameExtents when maximized one direction only

Fixes issues with grid plugin generating gaps if vertical and horizontal maximization are used to permit storing previous geometry
@XRevan86 XRevan86 closed this Jun 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants