GNOME Bugzilla – Bug 677412
StLabel text-shadow does not follow actor opacity
Last modified: 2015-02-21 02:03:45 UTC
Simple text case to explain my problem: new St.Label({ text: 'Foo', style: 'text-shadow: red 10px 10px 3px 0px; font-size: 48pt;', opacity: 0 }); Main.uiGroup.add_actor(r(0)) Tweener.addTween(r(0), { opacity: 255, time: 3, transition: 'easeOutQuad' }) Expected result: a red shadow is faded in with the label Actual result: no shadow, until you force a .style_changed() on the label The cause: StLabel creates a shadow texture the first time you paint after a style_changed and caches it. This texture is created by painting the ClutterTexture to FBO and the blurring pixels on the CPU. The problem is, the first time you paint the StLabel, opacity is 0, so painting the ClutterText draws nothing, resulting in a fully transparent shadow. Because opacity is not a style property, this fully transparent shadow is kept until something else invalidates it. I don't know how to best solve this problem. Probably we don't want to recreate the shadow texture every time opacity changes, as that can be quite expensive. Google tells me we could use a GLSL shader to draw it.
One simple solution I found is to paint the ClutterText at fully opacity. Patches follow.
Created attachment 215590 [details] [review] ClutterActor: expose setter for the opacity override Toolkits may need to paint actors internally outside the normal tree (for example to create a shadow shape), in which case they need to control the opacity directly.
Created attachment 215591 [details] [review] St: draw the actor at full opacity when creating shadow material When creating the shadow, we should ignore the opacity set on the actor and its parent, as it will be applied again at a later stage.
(In reply to comment #2) > Created an attachment (id=215590) [details] [review] > ClutterActor: expose setter for the opacity override ebassi isn't going to like this... though I don't know another way to force a paint at full opacity. I don't think the opacity override has any other purpose than that, so maybe we should just have a clutter_actor_paint_with_full_opacity. I wonder how the retained paint-node system s going to work with an opacity tree.
I *definitely* don't like exposing hacks. the opacity override was done to implement ClutterClone, and overloaded when we implemented OffscreenEffect. it's effectively a way to punch a hole in the Actor abstraction - and exposing it outside of Clutter is way above my comfort level. this alone almost begets a separation of APIs between app developers and toolkit developers.
Do you have a better suggestion for this, then? I don't.
StLabel is not a ClutterText, but has-a ClutterText. opacity is proxied from StWidget to ClutterText, and ClutterText is painted at the correct opacity. this means that StLabel controls the ClutterText opacity - so I guess you can paint the ClutterText at full opacity into the FBO without requiring punching a hole in the Actor abstraction through the paint opacity setter, and then painting it at the correct opacity. set_opacity() causes a queue_redraw(), but we can either: - make it context-dependent, and just set the opacity without queuing a redraw if the parent of the actor is inside the paint() implementation; - add a set_child_opacity() that works like set_opacity() but doesn't queue a redraw. I'd rather go with the first, since it's the same thing I want to do for setting the visibility of children actors during allocate(), so that StGenericContainer can remove the 'no paint list' API.
set_opacity doesn't work, because opacity is multiplied in clutter_actor_get_paint_opacity_internal. The problem is: ClutterText:opacity is 255, and nobody touches that, but StLabel:opacity animates. Even if you could temporarily override StLabel:opacity, what would happen if I put it inside an another actor? We really need the opacity override here.
Created attachment 219120 [details] [review] ClutterActor: expose setter for the opacity override Toolkits may need to paint actors internally outside the normal tree (for example to create a shadow shape), in which case they need to control the opacity directly.
Created attachment 219125 [details] [review] St: redirect labels with shadows offscreen To handle opacity on the label correctly, we need to redirect the label to a FBO, paint it there at full opacity, and then paint the results, otherwise the opacity is applied too early when building the shadow material and thus cannot be animated properly. A different way to tackle the problem.
Review of attachment 219125 [details] [review]: Have you measured how this affects performance?
Is this still needed? For what?
Yes, it's still needed, for the unlock dialog if bug 681576 lands making the dialog fade in, and possibly other places too.
Comment on attachment 219120 [details] [review] ClutterActor: expose setter for the opacity override Attachment 219120 [details] pushed as 10cce00 - ClutterActor: expose setter for the opacity override I still think this is a massive layering violation, and that St should just bite the bullet and use whatever GTK+ uses to render text shadows instead of using an FBO, but I don't want to block since I'm not going to write code to deal with this any longer.
the only difference between the patch that Giovanni submitted and the one that got committed is that the API is marked as experimental; this means it's only available in C, not via introspection. if you need it from introspection (and I'd rather you didn't) let me know.
https://git.gnome.org/browse/gnome-shell/commit/?id=07664e7d2f5bcc97fd137fd4a228be54046fe5a9