GNOME Bugzilla – Bug 611750
Rework StDrawingArea not to use ClutterCairoTexture
Last modified: 2010-03-11 20:09:32 UTC
Having StDrawingArea use ClutterCairoTexture causes circularity problems with sizing - StDrawingArea wants to use its allocation for the size of the texture, but ClutterTexture wants to use the size of the texture to determine the requited size. Avoid this by making StDrawingArea directly use Cairo and CoglTexture; while doing this, the API is changed a bit for simplicity and to match our use case: - Instead of clutter_cairo_texture_create(), we have st_drawing_area_get_context() to retrieve an already created context. This can only be called in the ::repaint signal. - The ::redraw signal is changed to ::repaint so we can have st_drawing_area_queue_repaint() that doesn't collide with clutter_actor_queue_redraw() - ::repaint is now emitted lazily when painting the actor rather than synchronously at various different points.
Created attachment 155175 [details] [review] Rework StDrawingArea not to use ClutterCairoTexture
Review of attachment 155175 [details] [review]: One small comment, otherwise this looks nice to me. ::: src/st/st-drawing-area.c @@ +228,3 @@ +st_drawing_area_get_context (StDrawingArea *area) + */ + * Return Value: (transfer none): the Cairo context for the paint operation This would look cleaner to me if priv was assigned at top and the assertions were together. @@ +253,3 @@ + + + StDrawingAreaPrivate *priv; Ditto.
Review of attachment 155175 [details] [review]: ::: src/st/st-drawing-area.c @@ +228,3 @@ + + priv = area->priv; + g_return_val_if_fail (priv->in_repaint, NULL); If you assign priv before you check SET_IS_DRAWING_AREA you segfault on NULL area rather than producing the desired warning. What we could do is just not use priv in the return if fails: StDrawingAreaPrivate *priv; g_return_val_if_fail (ST_IS_DRAWING_AREA (area), NULL); g_return_val_if_fail (area->priv->in_repaint, NULL); priv = area->priv; return priv->context; OK, in that case you'd probably just eleminate priv entirely, but if you were doing more complex stuff you might still want to assign priv after not using it in the g_return_val_if_fail(). Does that seem like a better approach?
Review of attachment 155175 [details] [review]: Yeah, that approach looks nice to me.
Attachment 155175 [details] pushed as 33dca51 - Rework StDrawingArea not to use ClutterCairoTexture