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 633340 - Reduce use of one-shot materials in StThemeNode
Reduce use of one-shot materials in StThemeNode
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-10-28 10:57 UTC by Neil Roberts
Modified: 2010-10-28 20:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
st-theme-node-drawing: Don't create lots of one-shot materials (12.91 KB, patch)
2010-10-28 10:58 UTC, Neil Roberts
reviewed Details | Review

Description Neil Roberts 2010-10-28 10:57:16 UTC
Currently in Cogl if you create a single material, paint with it and then discard it Cogl will end up compiling a program for it every time. The recommended usage is to retain materials between frames so that Cogl can reuse the program attached to it. Also it is best if similar programs are derived from a common material using cogl_material_copy. For example, if a similar set of materials are used that only differ by the texture handle then Cogl can attach the ARBfp program to the common ancestor and share the program between all of the derived materials.

StThemeNode has a few cases where it creates one-shot materials. During the animation to switch to the workspace view (I'm not sure if that's the right name) I was seeing around 30 new ARBfp programs being compiled every frame.

Cogl also had some problems when using cogl_set_source_texture and cogl_set_source_color. These would also end up recompiling a program every time an application alternated between the two functions because they both edited the same global material. I've now fixed this in Cogl with commit 65d7a113eeb0

With these two patches the ARBfp compilation goes down to approximately zero per frame during the animation although I still don't see very smooth performance so there may be more issues at play.

Also we are planning to improve Cogl so that if it needs to compile a program for a new material it will look in a hash table of the state of the material to see if it can reuse a shared program. That would make this patch less relevant because it would be able to avoid continuously compiling programs even for one-shot materials. However it is still better in general to avoid them.
Comment 1 Neil Roberts 2010-10-28 10:58:31 UTC
Created attachment 173388 [details] [review]
st-theme-node-drawing: Don't create lots of one-shot materials

A few places in st-theme-node-drawing create one-shot material, paint
with it and then free it. This is suboptimal with current Cogl because
it will end up compiling an ARBfp program just for that single paint
and then it will throw it away when the material is destroyed.

There is a new function in st-private.c called
_st_create_texture_material. This creates a simple material for a
texture based on a common parent material that points to a dummy
texture. Any materials created with this function are likely to be
able to share the same program unless the material is further modified
to contain a different number of layers. It would be possible to use
cogl_set_source_texture for this instead except that it's not possible
to modify the material's color in that case so we couldn't render the
texture with opacity.

The corner textures are now stored as a handle to a material that
references the texture rather than storing the texure directly. There
is also a separate border_material member which always points to
border_texture as the only layer.
Comment 2 Owen Taylor 2010-10-28 16:04:04 UTC
Review of attachment 173388 [details] [review]:

Looks great, just one tiny comment. Do you have a GNOME git account? If yes, you can just fix the below and push, otherwise, I can fix and push - not really a need to attach a new version.

::: src/st/st-theme-node-drawing.c
@@ +588,3 @@
 
+  if (node->border_material)
+    cogl_handle_unref (node->border_material);

_st_theme_node_free_drawing_state() already cleared it
Comment 3 Neil Roberts 2010-10-28 20:05:31 UTC
Thanks. I've made that change and pushed the patch as d66e7dd49edc58.