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

Backport invisible resize borders #503

Merged
merged 53 commits into from
Jun 5, 2019
Merged

Backport invisible resize borders #503

merged 53 commits into from
Jun 5, 2019

Conversation

vkareh
Copy link
Member

@vkareh vkareh commented May 23, 2019

So far I'm happy with the result of this. It's a sequential backport of Metacity commits to get invisible resize borders working. I've made sure to preserve Marco's own HiDPI logic this time.

Fixes mate-desktop/mate-themes/issues/249
Fixes #481
Fixes #248

vkareh added 21 commits May 24, 2019 09:34
There were actually *two* MetaFrameGeometry structs: one in theme-private.h,
one in frame.h. The latter public struct was populated by a mix of (void*)
casting and int pointers, usually pulling directly from the data in the private
struct.

Remove the public struct, replace it with MetaFrameBorders and scrap all
the pointer hacks to populate it, instead relying on both structs being used
in common code.

This commit should be relatively straightforward, and it should not do any
tricky logic at all, just a sophisticated find and replace.

https://bugzilla.gnome.org/show_bug.cgi?id=644930

upstream commit: https://gitlab.gnome.org/GNOME/metacity/commit/72224a165
NOTE: Patch copied from metacity and adapted for marco.
Just a quick little commit to help clean things up for when we add invisible
borders. Additionally, do a little housekeeping in preview-widget as well.

https://bugzilla.gnome.org/show_bug.cgi?id=644930

NOTE: Patch copied from metacity and adapted for marco.

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/7d519b3f
Since version 3.0, GTK+ has support for style variants. At the moment,
themes may provide a dark variant, which can be requested by
applications via GtkSettings. The requested variant is exported to
X11 via the _GTK_THEME_VARIANT property - support this property, in
order to pick up the correct style variant in the future.

https://bugzilla.gnome.org/show_bug.cgi?id=645355

NOTE: Patch is adapted for marco.

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/341d0945
To associate frames with the correct style variant, the UI will
need access to the window's theme variant property.

https://bugzilla.gnome.org/show_bug.cgi?id=645355

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/5c7403cc
This method allows forcing a style update of a particular frame
from the core, so that it can pick up style variants.

https://bugzilla.gnome.org/show_bug.cgi?id=645355

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/97554dc4
When the _GTK_THEME_VARIANT property changes, rather than just
updating the window's theme_variant property, update its frame
style as well, so that the window decoration reflects the requested
variant. As the initial properties of a window may be read before
its frame is created, there will be cases where the change is not
picked up initially.

https://bugzilla.gnome.org/show_bug.cgi?id=645355

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/8a779ab9
Like the setting of new frames' background is delayed until the
frame is associated with its window, delay attaching the initial
style, so that the correct style variant is picked.

https://bugzilla.gnome.org/show_bug.cgi?id=645355

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/ee056853
Rather than sharing a single style context between all frames, use
a default style and one style per encountered variant. The value of
the _GTK_THEME_VARIANT property should determine which style is
attached to a particular frame, though for the time being the default
style is used for every frame, as the window property cannot be
accessed at the time the style is attached. This will be fixed in
a later commit.

https://bugzilla.gnome.org/show_bug.cgi?id=645355

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/04d8135f
meta_frames_destroy() was not safe to be called multiple times, which
was causing a crash on exit due to something else changing somewhere
that makes it get called multiple times.

https://bugzilla.gnome.org/show_bug.cgi?id=654489

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/e35be641
We were relying on GTK+ emitting GtkWidget::style-updated during
widget initialization to create the GtkStyleContexts used for
window decorations. A recent GTK+ update broke this assumption,
so do the necessary initialization ourselves.

https://bugzilla.gnome.org/show_bug.cgi?id=671796

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/e4fde7b0
… widget

The style context of the widget is rarely what we want. We won't
fix this to be a MetaFrames style context yet; this just changes
the internal API.

https://bugzilla.gnome.org/show_bug.cgi?id=690317

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/76aa0704
Commit https://gitlab.gnome.org/GNOME/metacity/commit/c3a04bf
unintentionally broke XShape handling. By studying the code
extremely carefully, I found this inconsistency with the code that was
there before.

https://bugzilla.gnome.org/show_bug.cgi?id=635268

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/2b1c6443
An ARGB window with a frame is likely something like a transparent
terminal. It looks awful (and breaks transparency) to draw a big
opaque black shadow under the window, so clip out the region under
the terminal from the shadow we draw.

Add meta_window_get_frame_bounds() to get a cairo region for the
outer bounds of the frame of a window, and modify the frame handling
code to notice changes to the frame shape and discard a cached
region. meta_frames_apply_shapes() is refactored so we can extract
meta_frames_get_frame_bounds() from it.

https://bugzilla.gnome.org/show_bug.cgi?id=635268

NOTE: Applied only partially, compositor part is still missing...

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/0f2e32d1
Since window->frame_bounds is used as a cache we need to invalidate it when
destroying the frame.

https://bugzilla.gnome.org/show_bug.cgi?id=660773

upstream commit:
https://gitlab.gnome.org/GNOME/metacity/commit/330ff9b5
@raveit65
Copy link
Member

raveit65 commented Jun 4, 2019

I called already @XRevan86 @soreau from compiz-reloaded in a previous post.

@vkareh
Copy link
Member Author

vkareh commented Jun 4, 2019

@flexiondotorg, thanks for testing. I see that black artifact on master as well, it just looks a bit more pronounced on this branch, probably because the window frame now larger (since it includes the extra invisible space). I'm using intel drivers.

I will address as a separate pull request.

@flexiondotorg
Copy link
Member

Been using this PR all day. Working great!

Copy link
Member

@raveit65 raveit65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine and i didn't notice any more issues during testing the last days.
Ready to go from my point of view.
Sadly, nobody other from team has an interest to test such a massive change :/

@selectiveduplicate
Copy link
Member

Sorry I've been busy for a while.
Any way to test this on Ubuntu 18.04?

@flexiondotorg
Copy link
Member

I am 👍 on merging.

@vkareh vkareh merged commit 40301a7 into master Jun 5, 2019
@raveit65
Copy link
Member

raveit65 commented Jun 5, 2019

I filled out a ticket for compiz-reloaded because compiz doesn't start any more. https://gitlab.com/compiz/compiz-core/issues/148

@raveit65 raveit65 deleted the backport-borders branch June 5, 2019 16:13
@raveit65
Copy link
Member

raveit65 commented Jun 5, 2019

@vkareh @lukefromdc
Do you know exactly which commit breaks gtk-decorator?
This is the stacktrace https://www.dropbox.com/s/bisqevb687h3ice/compiz-backtrace-with-marco-invisible-border?dl=0

@vkareh
Copy link
Member Author

vkareh commented Jun 5, 2019

@raveit65 there are several. I will take a look, but it's not a trivial fix for compiz, it seems

@lukefromdc
Copy link
Member

lukefromdc commented Jun 5, 2019 via email

@lukefromdc
Copy link
Member

lukefromdc commented Jun 5, 2019 via email

@vkareh
Copy link
Member Author

vkareh commented Jun 5, 2019

@lukefromdc - I started last week, but gave up, since I wasn't sure whether it was working (because I've never used/compiled compiz-reloaded before, so I didn't know how to test it).

I suggest starting here:

$ git remote -v | grep launchpad
launchpad	https://git.launchpad.net/compiz (fetch)
launchpad	https://git.launchpad.net/compiz (push)

$ git log launchpad/master --grep=metacity --author=Muktupāvels --oneline --no-merges 
d6044e8aa Require libmetacity 3.22.0 or newer.
6c6efcb2c Ensure that surfaces used by libmetacity have device scale set to 1.
86fdc645d Require libmetacity 3.20+
651649413 Replace gwd_theme_metacity_is_valid with gwd_theme_metacity_new.
5a67946b0 Disconnect from update-metacity-button-layout signal.
33e626504 Update metacity theme loading.
6a2ac2b36 Make gwd_theme_metacity_get_decoration_geometry function static.
a4c12b42f Metacity theme settings now is under org.gnome.metacity.theme schema.
04cb44ca2 libmetacity-private has been fully replaced by libmetacity
496aabc9c Use libmetacity to parse button-layout string.
6eb42b9c2 Require new libmetacity library and in same time update for changes in git master.
9f204aa7c don't break build with older metacity version.
be4c3b032 Update metacity version in debian/control file.
0f893d953 Support only metacity 3.16.0 or newer.
d3c6b246a Use released metacity version.
7fce9e3b4 Use correct schema for theme name with metacity 3.16.
a9cd18e1f Build with metacity 3.16.
10a093d74 Start adding support for metacity 3.16.
03b5a22fa Reformat metacity.c.
1168d812f Add support for libmetacity-private 3.14
c2d9f05a7 metacity.c: stop using gtk_widget_get_style
d8880a7c9 Do not check for libmetacity-private.
d7aa74ac2 Update metacity.c for GTK+ 3. Decorations does not work.
23d99945a Build with metacity if 3.12 or newer is found
cac138e96 Don't try to build with metacity
95f41f536 Fix building. Disable metacity theme support if libmetacity-private 3.12.0 or newer found.
f45ed2004 Revert 'Don't try to build with libmetacity-dev 3.12.0. It won't work without porting gtk-window-decorator to GTK+ 3.'
95e31bd9b Don't try to build with libmetacity-dev 3.12.0. It won't work without porting gtk-window-decorator to GTK+ 3.
f8da8fac4 Make sure we can build without metacity theme support.

And backport those one by one starting from the bottom 😞

The main issue I had was that Compiz has all the components in separate files, whereas compiz-reloaded are all in the same file, so it was really hard to backport and know whether I was doing it right...

@lukefromdc
Copy link
Member

lukefromdc commented Jun 6, 2019

I've got an existing build of compiz-reloaded running right now with this installed and Emerald replacing gtk-window-decorator until we get this fixed over at the compiz-reloaded project. The first commit of the compiz-0.9 series is a do-over as compiz 0.9 uses cmake and 0.8 uses autotools.

Also, you can build compiz-reloaded against this right now without marco support (gtk-window-decorator does not use or follow metacity themes) by passing --disable-marco . You can also disable the entire gtk window decorator by passing --disable-gtk

@vkareh
Copy link
Member Author

vkareh commented Jun 6, 2019

That first one at the bottom is probably not necessary...

@lukefromdc
Copy link
Member

Turns out we already have the --disable-marco switch in configuration and it works: building with this switch set gives a version of gtk-window-decorator that ignores Metacity themes but works. Running it now for test purposes. Does not match my theme of course, but I have an emerald theme that is an almost exact match using the pixbuf engine and segments of screenshots of marco with my normal theme.

You can also disable the entire gtk window decorator by passing --disable-gtk and that should also work. In that case of course emerald is required.

@vkareh
Copy link
Member Author

vkareh commented Jun 6, 2019

@raveit65 - could you test/confirm what @lukefromdc is doing ☝️ ? If so, would it then make sense to merge #518 and update your spec files for compiz-reloaded?

@raveit65
Copy link
Member

raveit65 commented Jun 6, 2019

@vkareh
Yes, i know this build options --disable-marco, but this poor vanila gtk-decorator (pure cairo decoration) is not something what you want to use on a modern linux box.
And yes, compiz has a second decorator (emerald) which can be used.
But my problem is that if i want to update f29/30 with a new 1.22 release which use invisble-borders, than we break the compiz expierience for users.
Same for OpenSuse which have also compiz-reloaded in official repos.
And a new 1.22 release should be use for all distros.
I know we want to bring it in ubuntu-19.10. But i think we have a bit time for fixing compiz-reloaded, or not?
You can also test running emerald decorator in your f30 installation.
Use fusion-icon -n to start a tray applet where you can start compiz with emerald as decorator.

@XRevan86
I hope you read the notifications?

@lukefromdc
Copy link
Member

lukefromdc commented Jun 6, 2019 via email

@raveit65
Copy link
Member

raveit65 commented Jun 6, 2019

@vkareh @lukefromdc
Yes, i think we should bump soname version to mark this API breakage.
Minimal requirement is that compiz-reloaded doesn't crash any more and building is fixed, for me...
Adding invisible-borders to compiz is a nice-to-have but can be done later, ihmo.
At least c-r devs are responsible for that.

@flexiondotorg
Copy link
Member

I'd be quite happy with we released a 1.23 snapshot including this and other recent improvements and ship that in 19.10.

@vkareh
Copy link
Member Author

vkareh commented Jun 7, 2019

Should we close #518 then?

@raveit65
Copy link
Member

raveit65 commented Jun 7, 2019

No, let it open.
I would prefer to have a 1.22 release for fedora after compiz issue is fixed

@lukefromdc
Copy link
Member

https://gitlab.com/compiz/compiz-core/merge_requests/140 for compiz-reloaded brings the ability to resize from the shadow area to compiz, but I have yet to figure out how to compensate for the shadow area for tiled windows or windows being moved/resized to the top of the screen. Any help appreciated with this.

@raveit65
Copy link
Member

@lukefromdc
I did a MR with updated version_checks for marco.
https://gitlab.com/compiz/compiz-core/merge_requests/141

@lukefromdc
Copy link
Member

lukefromdc commented Jun 17, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants