After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 650254 - Make windows overlays/borders in overview look like in mockup
Make windows overlays/borders in overview look like in mockup
Status: RESOLVED WONTFIX
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 645563 652938 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-05-15 20:00 UTC by Maxim Ermilov
Modified: 2013-08-15 02:03 UTC
See Also:
GNOME target: ---
GNOME version: 3.1/3.2


Attachments
[WIP] workspace: implement new windows overlays/border mockup (24.43 KB, patch)
2011-05-15 20:00 UTC, Maxim Ermilov
none Details | Review
[WIP] workspace: implement new windows overlays/border mockup (25.85 KB, patch)
2011-05-21 20:32 UTC, Maxim Ermilov
none Details | Review
workspace: implement new windows overlays/border mockup (29.40 KB, patch)
2011-06-01 01:19 UTC, Maxim Ermilov
none Details | Review
workspace: implement new windows overlays/border (30.85 KB, patch)
2011-06-04 14:34 UTC, Maxim Ermilov
none Details | Review
add meta_window_actor_get_content (4.39 KB, patch)
2011-06-14 21:43 UTC, Maxim Ermilov
needs-work Details | Review
workspace: implement new windows overlays/border (29.24 KB, patch)
2011-06-14 21:45 UTC, Maxim Ermilov
none Details | Review
add meta_window_actor_get_content (2.11 KB, patch)
2011-07-09 21:34 UTC, Maxim Ermilov
needs-work Details | Review
add meta_window_actor_get_content (4.21 KB, patch)
2011-07-10 13:05 UTC, Maxim Ermilov
reviewed Details | Review
Screenshot (206.77 KB, image/jpeg)
2011-07-12 22:10 UTC, Maxim Ermilov
  Details
add meta_window_actor_get_content (4.00 KB, patch)
2011-07-17 17:29 UTC, Maxim Ermilov
none Details | Review
workspace: implement new windows overlays/border (29.75 KB, patch)
2011-07-17 17:30 UTC, Maxim Ermilov
none Details | Review
add meta_window_actor_get_content (4.46 KB, patch)
2011-08-29 23:51 UTC, Maxim Ermilov
none Details | Review
workspace: implement new windows overlays/border (29.74 KB, patch)
2011-08-29 23:52 UTC, Maxim Ermilov
none Details | Review
workspace: get rid of windowOverlay (44.36 KB, patch)
2011-09-05 01:40 UTC, Maxim Ermilov
none Details | Review

Description Maxim Ermilov 2011-05-15 20:00:00 UTC
Created attachment 187866 [details] [review]
[WIP] workspace: implement new windows overlays/border mockup

mockup:
http://git.gnome.org/browse/gnome-shell-design/plain/mockups/static/overview-window-picker-6-workspaces.png
Comment 1 Maxim Ermilov 2011-05-21 20:32:24 UTC
Created attachment 188311 [details] [review]
[WIP] workspace: implement new windows overlays/border mockup

fully match latest mockup.
Comment 2 Dan Winship 2011-05-25 14:18:57 UTC
*** Bug 645563 has been marked as a duplicate of this bug. ***
Comment 3 Dan Winship 2011-05-25 14:21:03 UTC
Comment on attachment 188311 [details] [review]
[WIP] workspace: implement new windows overlays/border mockup

>+ClutterActor*
>+shell_global_get_actor (ShellGlobal *global, MetaWindow *win)
>+{
>+  Window xwin = meta_window_get_xwindow (win);
>+  ClutterActor *actor = clutter_x11_texture_pixmap_new_with_window (xwin);
>+
>+  clutter_x11_texture_pixmap_set_automatic (CLUTTER_X11_TEXTURE_PIXMAP (actor), TRUE);
>+  return actor;
>+}

would probably be better to add a MutterWindow method to return its own internal texturepixmap?
Comment 4 Maxim Ermilov 2011-06-01 01:19:54 UTC
Created attachment 188966 [details] [review]
workspace: implement new windows overlays/border mockup

(require patch from Bug 651012)
Comment 5 Maxim Ermilov 2011-06-04 14:34:41 UTC
Created attachment 189215 [details] [review]
workspace: implement new windows overlays/border

correct initial animation.

> would probably be better to add a MutterWindow method to return its own
> internal texturepixmap?
It is more complex.
I think, it will not affect on performance.
Comment 6 drago01 2011-06-05 08:54:16 UTC
(In reply to comment #5)
> Created an attachment (id=189215) [details] [review]
> workspace: implement new windows overlays/border
> 
> correct initial animation.
> 
> > would probably be better to add a MutterWindow method to return its own
> > internal texturepixmap?
> It is more complex.
> I think, it will not affect on performance.

Well it breaks the mipmap emulation as you are bypassing mutter's texture tower.

Also while testing it I noticed that the window sizes are randomly wrong (i.e they end up being to narrow in the overview, going out and back fixes it again).
Comment 7 Maxim Ermilov 2011-06-14 21:43:51 UTC
Created attachment 189944 [details] [review]
add meta_window_actor_get_content

it gets the ClutterActor that is used to display the contents of the window without decoration
Comment 8 Maxim Ermilov 2011-06-14 21:45:08 UTC
Created attachment 189945 [details] [review]
workspace: implement new windows overlays/border
Comment 9 Florian Müllner 2011-06-19 13:54:11 UTC
*** Bug 652938 has been marked as a duplicate of this bug. ***
Comment 10 Dan Winship 2011-06-27 14:26:44 UTC
Comment on attachment 189944 [details] [review]
add meta_window_actor_get_content

oh. huh. I'd been thinking that the MetaWindowActor already had a texture that represented just the window contents, but I guess it only has the texture for contents+frame...

In this case, your original patch (or the equivalent inside mutter) may be better, if it would let us fix bug 602379 too...

(Although I don't know about the memory/GPU implications of the two approaches.)

>+  sub = cogl_texture_new_from_sub_texture (texture, x, y,
>+                                           inner_rect->width, inner_rect->height);
>+  clutter_texture_set_cogl_texture (CLUTTER_TEXTURE (data->clone), sub);

cogl_object_unref (sub); ?

>+meta_window_actor_get_content (MetaWindowActor *self)
>+{
>+  MetaWindowActorPrivate *priv = self->priv;
>+
>+  ClutterActor *actor = clutter_texture_new ();

Would it make sense to keep a pointer to this once it's been requested, rather than regenerating it every time?

(Even if you allowed it to be destroyed independently of the MetaWindowActor, you could still just keep the pointer inside the MetaWindowActor, rather than creating ContentPair.)

>+  ContentPair  *data = g_new (ContentPair, 1);

if you're keeping this, use g_slice_new()


It looks like the signal handlers only get removed if the MetaWindowActor gets destroyed, not if the content actor gets destroyed?
Comment 11 drago01 2011-06-27 14:35:02 UTC
(In reply to comment #10)
> (From update of attachment 189944 [details] [review])
> oh. huh. I'd been thinking that the MetaWindowActor already had a texture that
> represented just the window contents, but I guess it only has the texture for
> contents+frame...
> 
> In this case, your original patch (or the equivalent inside mutter) may be
> better, if it would let us fix bug 602379 too...

No, see comment 6 re. mipmaps (the scaled down versions look way worse without the mipmap emulation).
Comment 12 Maxim Ermilov 2011-07-09 21:34:14 UTC
Created attachment 191598 [details] [review]
add meta_window_actor_get_content

just create MetaShapedTexture from content
Comment 13 Jasper St. Pierre (not reading bugmail) 2011-07-09 21:38:42 UTC
Review of attachment 191598 [details] [review]:

You never add any rectangles (which will change to a cairo_region_t* in the future, see bug 644930), so shaped windows will look incorrect.

::: src/compositor/meta-window-actor.c
@@ +940,3 @@
+ * without decoration
+ *
+ * Return value: (transfer none): the ClutterActor for the contents

This is not (transfer none).

@@ +942,3 @@
+ * Return value: (transfer none): the ClutterActor for the contents
+ */
+ClutterActor*

style: space between type and *

@@ +949,3 @@
+  actor = meta_shaped_texture_new ();
+
+  clutter_x11_texture_pixmap_set_window (CLUTTER_X11_TEXTURE_PIXMAP (actor), meta_window_get_xwindow (self->priv->window), TRUE);

Split up your long lines here.
Comment 14 Maxim Ermilov 2011-07-10 13:05:12 UTC
Created attachment 191615 [details] [review]
add meta_window_actor_get_content

> You never add any rectangles
fixed
> This is not (transfer none).
Actually, It is)
actors should be managed manually.
Comment 15 Maxim Ermilov 2011-07-12 22:10:10 UTC
Created attachment 191851 [details]
Screenshot
Comment 16 drago01 2011-07-13 16:11:31 UTC
(In reply to comment #8)
> Created an attachment (id=189945) [details] [review]
> workspace: implement new windows overlays/border

I have tested this it is and noticed this bug:

1) go to overview
2) switch workspace using the thumbnails on the right
3) window clones are empty


The first time when I do that it is fine.
But all other attempts result into the above bug.

And sometimes it gets the sizes wrong. (Windows with wrong aspect ratio).
Comment 17 drago01 2011-07-13 16:12:35 UTC
Review of attachment 191615 [details] [review]:

::: src/compositor/meta-window-actor.c
@@ +934,3 @@
 
+static void
+remove_from_list (GObject *actor,

I'd use a less generic name here.

@@ +943,3 @@
+
+static void
+reshape_clones (MetaWindowActor *self)

Should be reshape_content_actors to match the rest of the naming.
Comment 18 Jasper St. Pierre (not reading bugmail) 2011-07-13 16:16:36 UTC
Review of attachment 191615 [details] [review]:

::: src/compositor/meta-window-actor.c
@@ -44,2 @@
   ClutterActor     *actor;
-

Nope.

@@ +942,3 @@
+}
+
+static void

You need to add an

#ifdef HAVE_SHAPE

around this method, otherwise there will be multiple compile errors (unused function, unused XShapeGetRectangles).
Comment 19 Maxim Ermilov 2011-07-17 17:29:17 UTC
Created attachment 192135 [details] [review]
add meta_window_actor_get_content
Comment 20 Maxim Ermilov 2011-07-17 17:30:58 UTC
Created attachment 192136 [details] [review]
workspace: implement new windows overlays/border

> I have tested this it is and noticed this bug.
> And sometimes it gets the sizes wrong.
fixed
Comment 21 Maxim Ermilov 2011-08-29 23:51:28 UTC
Created attachment 195139 [details] [review]
add meta_window_actor_get_content

rebased on TRUNK
Comment 22 Maxim Ermilov 2011-08-29 23:52:19 UTC
Created attachment 195140 [details] [review]
workspace: implement new windows overlays/border

rebased on TRUNK
Comment 23 Maxim Ermilov 2011-09-05 01:40:40 UTC
Created attachment 195649 [details] [review]
workspace: get rid of windowOverlay

change window clone's allocation logic
Comment 24 drago01 2011-09-09 16:12:57 UTC
(In reply to comment #23)
> Created an attachment (id=195649) [details] [review]
> workspace: get rid of windowOverlay
> 
> change window clone's allocation logic

This fixes the annoying blinks when clicking on an empty space in the overview. But it does not look like it used to be the labels are bigger and there is no space between the labels and the windows. Is this intentional?
Comment 25 Maxim Ermilov 2011-09-14 19:34:03 UTC
(In reply to comment #24)
> But it does not look like it used to be the labels are bigger and there is no
> space between the labels and the windows. Is this intentional?
yes. But It can be easily changed via css)
Comment 26 rockonthemoonfm 2011-10-07 10:03:02 UTC
hi, what's the status of this implementation?
I really wanted to see it included in 3.2.
Few remarks on the last screenshot posted in comment 15:

1. titles should be centred like in window decorations and in windows overview.
2. a close button (an X) should be added at the right of the pretty decorations, like in mutter, no additional overlay buttons on highlight.

cheers
Comment 27 Jasper St. Pierre (not reading bugmail) 2011-10-08 02:44:55 UTC
Can you rebase these patches? They're out of date with the invisible border patches.
Comment 28 Jasper St. Pierre (not reading bugmail) 2013-08-15 02:03:03 UTC
The design of this has never been clear with regards to transitioning in and out of the overview, and with our new CSD-based app designs, it's (intentionally) getting harder and harder split between content and titlebar.

Designs have moved on, and I don't think we want this anymore.

Thanks for all the hard work, Maxim. :)