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 627085 - CSS transitions + fades == slow
CSS transitions + fades == slow
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-08-16 20:35 UTC by Owen Taylor
Modified: 2010-08-23 16:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[transitions] Do not recreate FBOs on opacity changes (4.23 KB, patch)
2010-08-18 15:02 UTC, Florian Müllner
none Details | Review
[transitions] Do not recreate FBOs on opacity changes (4.59 KB, patch)
2010-08-19 08:00 UTC, Florian Müllner
committed Details | Review

Description Owen Taylor 2010-08-16 20:35:29 UTC
If we have a CSS transition at the same time the element is fading in (this happens for any CSS transitions in the dash when going to the overview, since that is being faded in), then that's really slow because of the opacity check in:

  if (!clutter_actor_box_equal (allocation, &priv->last_allocation) ||
      paint_opacity != priv->last_opacity)
    priv->needs_setup = TRUE;

I think we can:

  st_theme_node_paint (priv->[old_]theme_node,
                       allocation,
-                       paint_opacity);
+                       255); // or is it 1.0?

and then add the fading as a third layer the material with no texture and the blend mode MODULATE (PREVIOUS, PRIMARY). If that - a layer with no texture - doesn't work, we should be able to get the Cogl maintainers to make it work.

BTW, isn't the presence of paint_opacity in:

  cogl_color_set_from_4ub (&constant, 0, 0, 0,
                           clutter_alpha_get_alpha (priv->alpha) * paint_opacity);

wrong?
Comment 1 Florian Müllner 2010-08-18 15:02:19 UTC
Created attachment 168206 [details] [review]
[transitions] Do not recreate FBOs on opacity changes

Creating an FBO may be expensive, so we should avoid the operation
if possible. When transitioning between theme nodes, the widget's
opacity is used to paint to the offscreen textures which are blended
together - this means that the textures have to be recreated each time
the widget's opacity changes. It is much more effective to paint the
textures at full opacity and respect the widget's paint opacity when
blending the textures together.

(In reply to comment #0)
> then add the fading as a third layer the material with no texture and the
> blend mode MODULATE (PREVIOUS, PRIMARY). If that - a layer with no texture -
> doesn't work, we should be able to get the Cogl maintainers to make it work.

As far as I can tell, that's not possible at the moment - cogl_material_set_layer() used to have a comment which suggested an addition like that, but the comment was changed in current master, proposing to deprecate the method altogether in favour of cogl_material_set_layer_texture().

But I think that we can get away with setting the opacity on the first layer and then using it when interpolating the second layer.


> BTW, isn't the presence of paint_opacity in:
> 
>   cogl_color_set_from_4ub (&constant, 0, 0, 0,
>                            clutter_alpha_get_alpha (priv->alpha) *
> paint_opacity);
> 
> wrong?

Yes - the opacity is already taken into account when painting to the FBO, so it is wrong here. But it should be right now, as we paint the FBOs with full opacity :-)
Comment 2 Owen Taylor 2010-08-18 18:15:07 UTC
Review of attachment 168206 [details] [review]:

Does the math work out? Say the colors of the old and new are CO and CN, the paint opacity is a, and the interpolation factor is x.

Layer1:

 color = CO * a

Layer2:

 color = (a * x) * (CN * a) + (1 - (a * x)) * (CO * a)

While we want:

 color = x * (CN * a) + (1 - x) * (CO * a)

Or in words, we don't want to interpolate by a smaller amount depending on the opacity.
Comment 3 Owen Taylor 2010-08-18 18:22:23 UTC
> (In reply to comment #0)
> > then add the fading as a third layer the material with no texture and the
> > blend mode MODULATE (PREVIOUS, PRIMARY). If that - a layer with no texture -
> > doesn't work, we should be able to get the Cogl maintainers to make it work.
> 
> As far as I can tell, that's not possible at the moment -
> cogl_material_set_layer() used to have a comment which suggested an addition
> like that, but the comment was changed in current master, proposing to
> deprecate the method altogether in favour of cogl_material_set_layer_texture().

Talked to Robert Bragg on IRC, and this is supposed to work fine - you can set_layer_texture(..., COGL_INVALID_HANDLE), but more naturally you can just not set a layer texture at all - set_layer_combine() will itself cause the layer to be created.
Comment 4 Florian Müllner 2010-08-19 08:00:08 UTC
Created attachment 168269 [details] [review]
[transitions] Do not recreate FBOs on opacity changes

(In reply to comment #3)
> Talked to Robert Bragg on IRC, and this is supposed to work fine - you can
> set_layer_texture(..., COGL_INVALID_HANDLE), but more naturally you can just
> not set a layer texture at all - set_layer_combine() will itself cause the
> layer to be created.

Indeed.
Comment 5 Owen Taylor 2010-08-19 16:02:19 UTC
Review of attachment 168269 [details] [review]:

So, with this change we have:

0: RGBA = MODULATE (PREVIOUS, TEXTURE)
1: RGBA = INTERPOLATE (PREVIOUS, TEXTURE, CONSTANT[A])
2: RGBA = MODULATE (PREVIOUS, CONSTANT[A])

with a primary color fixed to 1,1,1,1. It would make a little more sense to me to have:

0: RGBA = REPLACE (TEXTURE)
1: RGBA = INTERPOLATE (PREVIOUS, TEXTURE, CONSTANT[A])
2: RGBA = MODULATE (PREVIOUS, PRIMARY)

So PRIMARY would have the same meaning it does in most cogl code - a color that modulates the result.

Practically speaking it doesn't make a difference - right now as soon as you use one combine constant and you change it, Cogl seems to be recompiling the shader. And if that was fixed, it would be cheap whether there was one layer constant or two.

Fine to commit either way.
Comment 6 Florian Müllner 2010-08-20 09:46:59 UTC
(In reply to comment #5)
> with a primary color fixed to 1,1,1,1. It would make a little more sense to me
> to have:
> 
> 0: RGBA = REPLACE (TEXTURE)
> 1: RGBA = INTERPOLATE (PREVIOUS, TEXTURE, CONSTANT[A])
> 2: RGBA = MODULATE (PREVIOUS, PRIMARY)
> 
> So PRIMARY would have the same meaning it does in most cogl code - a color that
> modulates the result.

But blending with full white? Should be (paint_opacity, paint_opacity, paint_opacity, paint_opacity), right?
Comment 7 Owen Taylor 2010-08-23 16:31:26 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > with a primary color fixed to 1,1,1,1. It would make a little more sense to me
> > to have:
> > 
> > 0: RGBA = REPLACE (TEXTURE)
> > 1: RGBA = INTERPOLATE (PREVIOUS, TEXTURE, CONSTANT[A])
> > 2: RGBA = MODULATE (PREVIOUS, PRIMARY)
> > 
> > So PRIMARY would have the same meaning it does in most cogl code - a color that
> > modulates the result.
> 
> But blending with full white? Should be (paint_opacity, paint_opacity,
> paint_opacity, paint_opacity), right?

Yes. You could also use 

 <whatever>,<whatever>,<whatever>,paint_opacity

and MODULE (PREVIOUS, PRIMARY[A]). That works but less like the COGL default MODULATE (PREVIOUS, TEXTURE)  blending and a little less natural to me.
Comment 8 Florian Müllner 2010-08-23 16:42:56 UTC
(In reply to comment #7)
> Yes. You could also use 
> 
>  <whatever>,<whatever>,<whatever>,paint_opacity
> 
> and MODULE (PREVIOUS, PRIMARY[A]). That works but less like the COGL default
> MODULATE (PREVIOUS, TEXTURE)  blending and a little less natural to me.

Yeah, that's why I used CONSTANT instead - I updated the patch to use PRIMARY instead before pushing.


Attachment 168269 [details] pushed as 5bd977d - [transitions] Do not recreate FBOs on opacity changes