*** Bug 607007 has been marked as a duplicate of this bug. ***

Comment on attachment 154379 [details] [review]
fix deprecation

Citing Colin in bug 607007:
This patch needs to be part of a larger "Switch to clutter 1.2" patch, at a
minimum patching the configure.ac and jhbuild moduleset.

Well, at least it compiles on rawhide now...

we don't support building it other than via jhbuild though, so...

(In reply to comment #3)
> Well, at least it compiles on rawhide now...

True, but as the recommended way to compile gnome-shell is using our jhbuild moduleset, we should not break that - as long as we officially depend on clutter-1.0 we cannot use cogl_clip_push_rectangle().

I'd expect the switch to happen rather soon though ...

Created attachment 154528 [details] [review]
Support and require Clutter 1.1

- Specify a minimum version of clutter-1.1.10
 - Switch clutter branch in the moduleset to master
 - Replace deprecated cogl_texture/material_unref() with
   cogl_handle_unref()
 - Use cogl_clip_push_rectangle() rather than cogl_clip_push()
 - Replace cogl_check_extension() with strstr - should be
   accurate enough.

[ One blocking factor in landing this is some COGL bugs with
  GL_TEXTURE_RECTANGLE_ARB - I hope to get those fixed over
  the next few days.

  Beyond, that there are also a bunch of warnings of:

(mutter:13295): Clutter-WARNING **: The actor 'StDrawingArea' is currently inside an allocation cycle; calling clutter_actor_queue_relayout() is not recommended

  These are "harmless" but make development pretty much impossible
  since you an't see the log output. StDrawingArea probably needs
  to be rewritten not to use ClutterCairoTexture, since it's sizing
  is interacting badly with CLutterTexture's sizing - does the
  texture drive the preferred size, or is the texture sized according
  to the size? And there are apparently also problems with
  StScrollbar/StScrollview. There are workarounds for that in
  MX, apparently. ]

Review of attachment 154528 [details] [review]:

::: src/st/st-box-layout.c
@@ +1124,3 @@
+                              (int)content_box.y1,
+                              (int)content_box.x2 - (int)content_box.x1,
+                              (int)content_box.y2 - (int)content_box.y1);

cogl_clip_push() and cogl_clip_push_rectangle() differ in parameters: arguments 3 and 4 should be content_box.x2 and content_box.y2

(In reply to comment #6)

> (mutter:13295): Clutter-WARNING **: The actor 'StDrawingArea' is currently
> inside an allocation cycle; calling clutter_actor_queue_relayout() is not
> recommended

I assume those are coming from the arrows in altTab ... queue_relayout() is only called to force a redraw of the arrow (to change the "highlight" state) ... we could just emit a redraw signal but this is kinda ugly.

(I have not actually tested it yet).

(In reply to comment #8)
> (In reply to comment #6)
> 
> > (mutter:13295): Clutter-WARNING **: The actor 'StDrawingArea' is currently
> > inside an allocation cycle; calling clutter_actor_queue_relayout() is not
> > recommended
> 
> I assume those are coming from the arrows in altTab ... queue_relayout() is
> only called to force a redraw of the arrow (to change the "highlight" state)
> ... we could just emit a redraw signal but this is kinda ugly.

Actually, they are fundamentally coming from all uses of StDrawingArea. The basic problem is that when StDrawingArea.allocate() is called, it resizes the ClutterCairoTexture so the texture size matches the allocated size. This queues a relayout sinside ClutterTexture since ClutterTexture requests a size that matches the size of the texture.

(In reply to comment #7)
> Review of attachment 154528 [details] [review]:
> 
> ::: src/st/st-box-layout.c
> @@ +1124,3 @@
> +                              (int)content_box.y1,
> +                              (int)content_box.x2 - (int)content_box.x1,
> +                              (int)content_box.y2 - (int)content_box.y1);
> 
> cogl_clip_push() and cogl_clip_push_rectangle() differ in parameters: arguments
> 3 and 4 should be content_box.x2 and content_box.y2

Good catch. Fixed locally.

(In reply to comment #10)
> (In reply to comment #7)
> > Review of attachment 154528 [details] [review] [details]:
> > 
> > ::: src/st/st-box-layout.c
> > @@ +1124,3 @@
> > +                              (int)content_box.y1,
> > +                              (int)content_box.x2 - (int)content_box.x1,
> > +                              (int)content_box.y2 - (int)content_box.y1);
> > 
> > cogl_clip_push() and cogl_clip_push_rectangle() differ in parameters: arguments
> > 3 and 4 should be content_box.x2 and content_box.y2
> 
> Good catch. Fixed locally.

Just like to point out that those changes were already in my patch...

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #7)
> > > Review of attachment 154528 [details] [review] [details] [details]:
> > > 
> > > ::: src/st/st-box-layout.c
> > > @@ +1124,3 @@
> > > +                              (int)content_box.y1,
> > > +                              (int)content_box.x2 - (int)content_box.x1,
> > > +                              (int)content_box.y2 - (int)content_box.y1);
> > > 
> > > cogl_clip_push() and cogl_clip_push_rectangle() differ in parameters: arguments
> > > 3 and 4 should be content_box.x2 and content_box.y2
> > 
> > Good catch. Fixed locally.
> 
> Just like to point out that those changes were already in my patch...

Yeah, well, I already had Luis's patch from bug 607007 in my clutter-1.1 branch, so I started from that...

*** Bug 611725 has been marked as a duplicate of this bug. ***

Review of attachment 154528 [details] [review]:

No comments other than Florian's; marking reviewed.