Merge lp://qastaging/~muktupavels/compiz/gtk-window-decorator-4 into lp://qastaging/compiz/0.9.12

Proposed by Alberts Muktupāvels
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: 3915
Merged at revision: 3886
Proposed branch: lp://qastaging/~muktupavels/compiz/gtk-window-decorator-4
Merge into: lp://qastaging/compiz/0.9.12
Prerequisite: lp://qastaging/~muktupavels/compiz/gtk-window-decorator-3
Diff against target: 374 lines (+60/-169)
3 files modified
debian/control (+1/-1)
gtk/CMakeLists.txt (+4/-1)
gtk/window-decorator/metacity.c (+55/-167)
To merge this branch: bzr merge lp://qastaging/~muktupavels/compiz/gtk-window-decorator-4
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
Review via email: mp+224338@code.qastaging.launchpad.net

Commit message

Re-enable metacity theme support

Description of the change

Restore metacity theme support.

To post a comment you must log in.
Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

I think it is ready for some review. Decorations works, but only with "hack" (added in 3912. revision).

Without it decorations are invisible/transparent. Do you have any idea why?

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> I think it is ready for some review. Decorations works, but only with "hack"
> (added in 3912. revision).
>
> Without it decorations are invisible/transparent. Do you have any idea why?

Mh, I've been trying to debug this a little, but I can't either find where the issue is.
It might be related to what meta theme does with the cairo surface, although this seems quite weird (it's like it can't draw in a fully transparent area?).

Anyway, the workaround is not "too ugly" for inclusion, and considering I think we should get the theming back to utopic, I think is fine to accept that.

The only thing I ask you is to document these three lines better, in order to make easier to understand what's going on there.

review: Approve
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Err, I also forgot one thing:

You need to update debian/control:
 - http://pastebin.ubuntu.com/8149755/

review: Needs Fixing
3914. By Alberts Muktupāvels

update build-depends

3915. By Alberts Muktupāvels

Add comment to workaround to fix invisible decorations.

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

Added comment and fixed build-depends. Will it be ok now?

I will try to find if invisible decorations is bug with metacity.

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

> I will try to find if invisible decorations is bug with metacity.

Does not look like metacity bug. After meta_theme_draw_frame I added following lines:

cairo_status_t status = cairo_surface_write_to_png (cairo_get_target (cr), "gwd.png");
if (status) {
 g_warning ("Failed to write \"%s\": %s\n", "gwd.png", cairo_status_to_string (status));
}

I don't see decorations on windows, but they are saved in ~/gwd.png file.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Ok, that's fine for now.

I was curious to try to do the same you did, but it seems then that the drawing happens correctly.

I'm not sure if that might be caused the decor plugin, expecting something else to be in the texture, but that would be strange, as it should not care about the texture content.

Anyway, the hack is not the nicest thing to see, but it causes no troubles to have it in. So I approve this.

review: Approve
Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

Maybe decor_update_meta_window_property function needs some adjustments? If XChangePropery is not called then decorations does not disappear.

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

Something is wrong with d->surface. Added cairo_surface_write_to_png (d->surface, "test.png"); in decor_update_meta_window_property.

With hack decorations are visible in test.png, but without hack it is empty/transparent.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Something is wrong with d->surface. Added cairo_surface_write_to_png
> (d->surface, "test.png"); in decor_update_meta_window_property.
>
> With hack decorations are visible in test.png, but without hack it is
> empty/transparent.

Yeah, that's strange. I've also tried to do something with this, but I wasn't able to find where the problem happens.
It's like that a fully transparent surface create troubles. While not calling the XChangeProperty function seems the decorator not to redraw the surface properly.

Not sure if there's instead a problem caused by the surface not being reinitialized correctly... By the way, thanks for your further researches and it would be nice if you could continue. Although, I'm more concerned with crashes you found (and I didn't) due to BadWindow arguments.

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

It might not be ready for merging. :(

Did you tried to resize windows with metacity theme? I am getting black decorations:
https://www.dropbox.com/s/on1hianmmphqsrd/black-decorations.png?dl=0

I am getting crashes too often. Does this crash file is useful for you:
https://www.dropbox.com/s/bklaya9phs6dl9y/_usr_bin_gtk-window-decorator.1002.crash?dl=0

Also I created one more merge proposal:
https://code.launchpad.net/~albertsmuktupavels/compiz/gtk-window-decorator-6/+merge/232477

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> It might not be ready for merging. :(
>
> Did you tried to resize windows with metacity theme? I am getting black
> decorations:
> https://www.dropbox.com/s/on1hianmmphqsrd/black-decorations.png?dl=0

Mh, no I'm not experiencing that issue here.

> I am getting crashes too often. Does this crash file is useful for you:
> https://www.dropbox.com/s/bklaya9phs6dl9y/_usr_bin_gtk-window-
> decorator.1002.crash?dl=0

Mh, that would be helpful but my binary doesn't match.
So in order to give me a proper crash file, copy to /usr/bin/gtk-window-decorator a binary with debugging symbols, then once you've the crash file send me both the binary and the core dump (crash file).

(otherwise you can examine it getting the core file with apport-extract and then open it with -core option of gdb and the binary file).
Thanks.

> Also I created one more merge proposal:
> https://code.launchpad.net/~albertsmuktupavels/compiz/gtk-window-
> decorator-6/+merge/232477

Fine, I'll check that later, once we've found these issues

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

So I have two problems that you don't have. Maybe I have problems because I am using nvidia proprietary drivers? Actualy I have third problem.

Videos:
https://www.dropbox.com/s/wjgmx2txye806th/VIDEO0005.mp4?dl=0
https://www.dropbox.com/s/ygmf6eoa1wyw7t1/VIDEO0006.mp4?dl=0
https://www.dropbox.com/s/xu118nuphe1dtto/VIDEO0007.mp4?dl=0

Is this bug with compiz or it is bug in nvidia driver? If it is nvidia than could it cause my crashes too?

I don't have above problems with metacity.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Mh, no I don't thin I've them.

As for the BadWindow errors, using GDK_SYNCRNIZE you should be able to get where the error is more easily. Otherwise try to cast all the XLib calls inside gtk-window-decorator inside error traps (or just do that one for once when starting).

The 2nd video is quite strange, and weird that it's caused by decorator.
As for the last video: do you get the crash when running the decorator in an unity session?

Anyway, indeed the nvidia drivers often cause weird issues, but not sure we can only stop on this assumption.

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

How about adding 'gdk_x11_display_error_trap_push (gdkdisplay);' in gtk-window-decorator.c? After short period of testing it seem that I am not getting crashes. I think it should be added in part 3 if you agree.

Is it possible to run decorator in unity? If I try to enable it it will disable unity.

So now I have only problem with black decorations, but that might be nvidia problem. Video with it:
https://www.dropbox.com/s/2ba27yvyfb8gjsi/VIDEO0009.mp4?dl=0

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> How about adding 'gdk_x11_display_error_trap_push (gdkdisplay);' in gtk-
> window-decorator.c? After short period of testing it seem that I am not
> getting crashes. I think it should be added in part 3 if you agree.

Ok, but and pop it also. At this point you should not get errors.
And you can also remove the other instances.

> Is it possible to run decorator in unity? If I try to enable it it will
> disable unity.

No currently it's not possible.

> So now I have only problem with black decorations, but that might be nvidia
> problem. Video with it:
> https://www.dropbox.com/s/2ba27yvyfb8gjsi/VIDEO0009.mp4?dl=0

Mh, no I've not found that issue definitely.

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

I decided to not use error trap for whole display...

Added XSynchronize (x, True) in gtk-window-decorator.c. After that I found that XFreePixmap sometimes generate BadPixmap error in pixmap_destroy_cb function you suggested to add in gdk.c. Also seems that it is causing black decorations for me. So now my question is - are you sure that it was needed?

This is what I get from gdb when decorator crashes with BadWindow:
http://pastebin.ubuntu.com/8188141/

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

I tested this in VirtualBox. It is not crashing there.

Also today Dmitry tested this branch. GWD was not crashing for him too. Radeon HD 6400M/7400M Series with the default open source driver.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Yeah, I'm using a similar configuration... And in fact it works well here.

Wondering why pixmap_destroy_cb function cause problems to you when using metacity, while it doesn't when using standard gtk decorations.

I'm wondering weather if the metacity code in some part tries to delete these textures in a different way (not checked the full code yet for that). But it sounds like it's that.

Anyway, disabling the pixmap_destroy_cb function fixes all the problems you have?

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

No. It only fix resizing problem.

Also it crashed for Dmitry too. He send me some stack traces. I have not looked at them yet. If you have time maybe you could look:
1) https://www.dropbox.com/s/utvqkcj7n5anhnq/trace1.txt?dl=0
2) https://www.dropbox.com/s/1zupkjtszsi44ym/trace2.txt?dl=0
3) https://www.dropbox.com/s/os5nin5jlis9yz9/trace3.txt?dl=0

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Was he launching gdk-window-decorator using GDK_SYNCHRONIZE or calling XSynchronize (x, True) somewhere?

Anyway, all these problems seems to be Bad{Window,Pixmap} issues, that - even if it would be better to avoid - it's not a big problem to ignore them using gdk traps (at the end these can be considered only warnings).

I'm more concerned about the black textures issue you get, and it would be nice to find why that happens if we clear them.
If I'm not wrong these aren't destroyed at decor plugin level, right?

3916. By Alberts Muktupāvels

Merge with gtk-window-decorator-3

Revision history for this message
Alberts Muktupāvels (muktupavels) wrote :

> Was he launching gdk-window-decorator using GDK_SYNCHRONIZE or calling
> XSynchronize (x, True) somewhere?

GDK_SYNCHRONIZE

> Anyway, all these problems seems to be Bad{Window,Pixmap} issues, that - even
> if it would be better to avoid - it's not a big problem to ignore them using
> gdk traps (at the end these can be considered only warnings).

I added one more error trap. See gtk-window-decorator-3 part.

> I'm more concerned about the black textures issue you get, and it would be
> nice to find why that happens if we clear them.
> If I'm not wrong these aren't destroyed at decor plugin level, right?

I am not good at reading stack traces and/or debuging, but it looks like crash is happending in cairo. Crash is happening when calling cairo_destroy (d->cr) in decorator.c (692 line). If I delete it then it is not crashing, also black decoration problem dissapears. Any ideas? This looks like last problem.

3917. By Alberts Muktupāvels

Merge with gtk-window-decorator-3

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches