GNOME Bugzilla – Bug 642091
Support transparency in our search popup
Last modified: 2012-08-02 18:35:11 UTC
Right now our search popup is a little bit ugly. We should make it transparency using composition or emulate this by making a clip with the parent window. I've just found out how gnome-panel makes this so maybe we could have a look at http://git.gnome.org/browse/gnome-panel/tree/applets/notification_area/na-tray-child.c?id=6f9a60b4b810f224b31a5c0ffca4792a107a6d6d
Created attachment 185565 [details] [review] Implement compositing support for GeditOverlay
Note that there is one thing that I'm not so sure about if we should do it like this or somewhat different, which is that now the main widget is also embedded in a GeditOverlayChild. This is needed to properly do the compositing, because for example a scrolled window (which we have in gedit) does not have a GdkWindow of its own, meaning it will draw directly on the GeditOverlay in which case the compositing cannot work. Otherwise the patch seems to do the job.
I just noticed that with compositing enabled, for some of the themes the background in the text view does not render properly anymore. Presumably because the background color is not set explicitly or something.
Review of attachment 185565 [details] [review]: Some things, gedit-overlay-child are supposed to be generic widgets so either do the hack in the slider widget that inherits from gedit-overlay-child and gedit-animated-overlay that inherits from gedit-overlay. To me this is a bit hacky there should be an easier way to do this. See that the overlay is a normal container, if other containers implemented in a similar way like GtkBox etc are able to do compositing without this I don't know why we can't. Anyway I'm ok for adding the hack in the inherited classes for now. See that this will be added to gtk so I want to avoid this kind of things in the main widgets.
Created attachment 185577 [details] [review] Implement compositing support for GeditOverlay
I don't really understand what you mean... no container in gtk+ can do this. I don't think this is a 'hack', this is the way you are supposed to implement compositing in a container where your children can overlap.
Just to clarify, what gtk+ does by default (maybe what you refer to) is compositing of one child onto the parent. What is doesn't do is to composite multiple children together on the parent, which is why what want requires manual compositing. As far as I understand it anyway.
Review of attachment 185577 [details] [review]: Ok, apart from minor comments it looks good. You are right let's go for it. ::: gedit/gedit-overlay.c @@ +163,3 @@ + if (!GEDIT_IS_OVERLAY_CHILD (child)) + { + child = GTK_WIDGET (gedit_overlay_child_new (child)); I guess that you can do gtk_container_add to avoid duplicated code? ::: gedit/gedit-view-frame.c @@ +1477,3 @@ NULL); + gtk_widget_override_background_color (frame->priv->slider, shouldn't you check here that we supports composition before setting it transparent?
I want to comment on commit ab64e914fb7e9cd198c30437b030eef440fd774e, but since there's no bug for it in the message I'll write here. That patch changes, AFAICT, how the overlay works in that now the main widget property is a GeditOverlayChild (instead of the actual widget), which we don't even export. Among other things this makes epiphany instantaneously crash, since it's an incompatible change. Is this intentional? What is the rationale behind it? Also, if I workaround that my GtkFrame+GtkLabel packed in the overlay come up completely black.
Maybe we should just revert the composition patch?
The issue of this change is to support composition between all the widgets, but since it is imcompatible I guess we should revert and try to put something in for this for 3.2
Created attachment 186064 [details] [review] remove composition This patch removes the composition and leaves the refactorings made.
Created attachment 186066 [details] [review] overlay child Another refactoring in top of the remove composition one.
Review of attachment 186064 [details] [review]: Looks fine with minor comment. ::: gedit/gedit-overlay.c @@ +393,3 @@ + else + { + gtk_widget_destroy (original); I don't think you want to destroy when removing from the container...
Review of attachment 186066 [details] [review]: There seem to be some other changes here, not related to composition. It would be good to separate them cleanly.
(In reply to comment #14) > Review of attachment 186064 [details] [review]: > > Looks fine with minor comment. > > ::: gedit/gedit-overlay.c > @@ +393,3 @@ > + else > + { > + gtk_widget_destroy (original); > > I don't think you want to destroy when removing from the container... Ups right!
So the patches are pushed.
*** Bug 647802 has been marked as a duplicate of this bug. ***
What is the status here, of the report and the patches?
Actually I fixed this like a couple of months ago. Thanks for the reminder. http://git.gnome.org/browse/gedit/commit/?id=fe7c0fc6ffcd8a034c1805fa3a22e58025b51379