GNOME Bugzilla – Bug 608847
st_widget_recompute_style triggers reallocation on map
Last modified: 2010-02-03 22:35:34 UTC
Seen when debugging: ====
+ Trace 220388
This can cause effects like adding N items to a box triggering N complete reallocations of the stage. Calling clutter_actor_get_allocation_box() looks innocuous but ends up triggering some bad stuff. What I'd like to see is the gradient be demands-redrawn just before painting, though this is a bit hard because ClutterCairoTexture queues a redraw every time it is drawn on. What I'd probably suggest is skipping the ClutterCairoTexture and drawing directly to an cairo_image_surface and creating the CoglTexture from that. This interacts with some of the stuff discussed in bug 607500 about moving painting of the background and border into StThemeNode.
Created attachment 152956 [details] [review] Improve handling of gradients By calling clutter_actor_get_allocation() in st_widget_recompute_style() to determine whether to redraw gradients, we triggered a complete reallocation of the stage for each gradient. As gradients are processed in st_widget_real_style_changed() anyway, the additional checks in st_widget_recompute_style() are redundant and can be removed altogether. Also, in preparation to moving the actual drawing code to StThemeNode, replace ClutterCairoTexture with plain cairo for the gradient drawing.
(In reply to comment #1) > Created an attachment (id=152956) [details] [review] > Improve handling of gradients The patch leaves the gradient drawing in st_widget_allocate() instead of moving it to st_widget_paint(), but otherwise follows the suggestions above. As the changes to st_widget_recompute_style() should be enough for this bug, it might be a good idea to split the patch and attach the ClutterCairoTexture stuff to bug 607500 though ...
Review of attachment 152956 [details] [review]: Hmm, I'd really like to see this with only the fix, since there is a simple fix, and not pull in the conversion too. One side-note about the conversion below for reference. Also, my intent wasn't to move from ClutterCairoTexture to ClutterTexture, but to CoglTexture - if we manage a CoglTexture ourselves, we can just use it in paint() and not have to worry about allocating extra actors. ::: src/st/st-widget.c @@ +734,3 @@ + /* cogl doesn't seem to support the conversion, do it manually */ + /* borrowed from clutter-cairo, conversion from ARGB pre-multiplied + * to RGBA */ This is supported in COGL now @@ +779,3 @@ + texture = cogl_texture_new_from_data (width, height, + COGL_TEXTURE_NONE, + COGL_PIXEL_FORMAT_RGBA_8888, You'd just use COGL_PIXEL_FORMAT_ARGB_8888_PRE
Created attachment 152966 [details] [review] Improve handling of gradients Stripped the conversion code.
Review of attachment 152966 [details] [review]: Looks simple and correct. But do you have any plans to update the gradient code to use a CoglHandle? If so we should coordinate since I'd like to pick back up my borders work and there is a lot of overlap there.
Attachment 152966 [details] pushed as fd1ce40 - Improve handling of gradients (In reply to comment #5) > But do you have any plans to update the gradient code to use a CoglHandle? If > so we should coordinate since I'd like to pick back up my borders work and > there is a lot of overlap there. Well, yes ... how about me updating the patch and attaching it to the other bug so you can pick it up there? Shouldn't take too much time ...