It is working OK for me, and nothing much has changed in the code for quite a while.

What version of GTK+ are you using? What distro?

Sorry for the delay, I've just checked this issue and I still got it.

I am using the jhbuild so most of the libs I got are pretty recent. The GTK+ version I'm using is 3.9.3.

What GTK+ are you using? Could you try with a recent version?

Thanks for checking this.

Created attachment 246615 [details]
Screenshot of the correct display

Yes, I can see this now. (I was accidentally linking against old stuff.)

It has messed up the canvas quite badly. Either GTK+ has broken backwards
compatibility or GooCanvas has been doing something wrong forever!

I'll move this to GTK+ to see if they know what's going on, and if it is
a GTK+ or GooCanvas issue.

not "forever", but definitely since GTK+ 3.0: widgets should only draw using the cairo_t provided in the GtkWidget::draw signal handler.

with GTK+ 3.10, all child widgets are now drawn using the parent's context - i.e. when drawing a child we set the offset in the cairo_t. this means that you have to take that offset into account when drawing, if you're not using the cairo context provided by a widget.

see also bug 702538 for another potential cause that deals with stomping on the cairo matrix stack without cairo_save()/cairo_restore().

Created attachment 249163 [details]
The GooCanvas draw code

This is the GooCanvas draw signal handler function.

We do use the cairo_t passed in, and call save/restore.

The GooCanvas code is complicated as it supports scrolling and scaling, and
draws onto a child window.

I've committed a fix for this, though I'm not sure it is 100% working.

The problem was that GooCanvas draws into a child window which scrolls.
The new drawing code in GTK+ 3.9 has changed the way we need to do this.

 o We need to use the translation from the cairo_t, instead of drawing
   from 0,0 into the child window.
 o We need to add our own translation to handle the scrolling, instead of
   relying on the child window's scrolled position.
 o We need to use the clip passed from the cairo_t, instead of relying on X
   to clip to the child window.


So maybe GTK+ 3.9 has broken backwards compatibility, at least for widgets
that have child windows which scroll!

The first 2 of the above issues could possibly be fixed in GTK+, if they
are considered to be bugs.

I can confirm this bug, I get exact same issue running http://github.com/drahnr/oregano (ignore the expand issue, I will file a second bug for that). This only appeared in 3.10 (3.6 was find, everything in between being untested)

I can confirm git 8f2c634d12d05ef2e7fdaf41f4e6bd7a28894e58 commit seems to work out.

I don't quite understand comment #7. We did change the way drawing is done in Gtk+ 3, and there are some fringe cases where this may break things, but it doesn't really seem like goocanvas should have needed to hit those.

Basically, it used to be the case the we sent a separate gdk expose event for each GdkWindow, creating a new cairo_t for each call and emitting the expose event from the mainloop. So, for a widget with multiple windows there would be multiple ::draw calls with different cairo_t:s. Each cairo_t would however be drawing (via an offsetted subsurface) on the same surface the double buffer surface for the entire window (or rather the minimal rect covering the exposed area on the toplevel).  If a container had non-windowed children it was responsible for forwarding the drawing of those in its draw signal handler.

In the new (3.10) world we instead only deliver draw events to the toplevel window (or native subwindows, but those are very rare), and all container widgets are propagating the drawing of all children into the same cairo_t (now using the offsets in the cairo_t rather than via a subsurface). The nice thing about this is that the entire drawing operation happens in a single call hierarchy with cairo knowing the entire tree. This lets us do things like cairo_push/pop() during the drawing for alpha translucency, and implement offscreen caches of subtrees.

Some things change in this model:

* The exact value of the cairo_t transform offset changes because its now essentially an offset from the toplevel rather from the subwindow that got the expose event

* The position of the clip of the cairo_t changes, for the same reason

* Sometimes the order of drawing of child GdkWindows inside a container changes because we have to draw all the GdkWindows that belong to a particular GtkWidget at the same time. So, if you have a setup where a container has two children, one with two GdkWindows ordered like this:

  ContainerWindow
     Child1Window1
     Child2Window
     Chile1Window2
     
We would now be drawing the two Child1 windows before the Child2 window, which is a break in the behaviour.

However, most things do *not* change:

The cairo_t that is passed to draw() still draws to the same *place* as before. I.e. if you draw at 0,0 in the cairo_t it will be placed at the top left corner of the GtkWidget allocation (*not* the gdkwindow the expose is in, that was never the case). And the clip region will be at the same place in the final rendering as before, i.e. when rendering a specific GdkWindow of a GtkWidget the cairo_t clip will be set to match the shape of that GdkWindow.

So, behaviour will only change if you either throw away the pre-existing clip region or transform. But technically you should not be doing that even in pre 3.10 Gtk+, as that breaks some features like gtk_widget_draw() which draws to any random cairo_t in the same way that 3.10 draws, and has existed since 3.0.

So, really as long as code "does it right" only the last case is a real break in behaviour. Obviously its not impossible for code to break (like GooCanvas did) or like gnumeric did (it relied on the clip origin), but the Gtk+ changes are important enough that I think having to fix a few places is worth it.

Now, looking at the goocanvas details of commit 8f2c634d12d05ef2e7fdaf41f4e6bd7a28894e58 I'm a bit confused:
paint_static_items() did:
- cairo_identity_matrix (cr);
- cairo_translate (cr, -priv->static_window_x, -priv->static_window_y);

But it seems to me that static_window_x/y is *always* the same as window_x/y except in a commented out region in redraw_static_items_at_position, so at every time during paint_static_items the above will just reset the cr offset to -window_x, -window_y, which is what Gtk+ would have set it to before.

So, this was only ever needed for:
      /* FIXME: This causes flicker. Do we need it? */
      /*gdk_window_process_updates (canvas->canvas_window, TRUE);*/

Which seems like a very weird thing. You never want to process updates sync in the middle of the code. We have a lot of advanced stuff like frame clocks synced with compositor framerates and whatnot that this will break.

goo_canvas_draw() did:
- cairo_identity_matrix (cr);
and then later:
- cairo_translate (cr, priv->window_x, priv->window_y);

This will clear the cairo_t matrix and then manually compute the same value for it on gtk < 3.10, except it doesn't work with gtk_widget_draw(). But why? Can't it just rely on the pre-existing correct offset?

then:
 /* Get rid of the current clip, as it uses the wrong coordinate space.
FIXME: Maybe we should always set a clip with the new GTK+ drawing code. */
- cairo_reset_clip (cr);

Whats wrong about the clip? It should be ok (i.e. clip to the GdkWindow visible parts).
Reseting it is wrong as you may then draw outside your GdkWindow. This is less likely with Gtk < 3.10 where the subsurface added some additional clipping outside the cairo_t clip, but could still happen if some other GdkWindow overlapped the area of the GdkWindow you were drawing to.

Also, in Gtk+ 3.10 we removed the X scrolling operation. I.e. gdk_window_move() of a subwindow will completely redraw the new and old position of the subwindow, whereas before it would XCopyArea the old region that would be visible in the new area and only expose the new area. This is slower (but may be ok for most uses). If that is a problem for GooCanvas though you may want to look at the new scrolling model that Gtk+ is using for its scrolling widgets in 3.10, where it uses offscreen buffering for very fast scrolling that matches more how modern GPU hardware works.

Hi Alex,

Thanks for the explanation of the new way drawing is done.

I've fixed the code now - GooCanvas should just have been relying on the cairo_t transform/clip from 3.0 as you say, but in porting it I just wiped those so I could use the 2.x code as much as possible.


The only problem I have left is that the "Large Items" demo page isn't working.
It uses a very large canvas - right up to the limits of a GdkWindow, a signed 32-bit integer. But now it will only work up to about a 23-bit int.

Has the limit on window sizes changed?

Damon: Ugh!
Maybe that is broken because cairo uses 24.8-bit fixed point?

I've added bug #710958 for the large canvas issue.
It seems to be a problem with gtk_cairo_should_draw_window().

I guess this can be closed now too. Thanks.