Created attachment 161378 [details] [review]
[StWidget] Clean up private member variables

When moving the drawing code into StThemeNode, most private members
of StWidget became obsolete, so remove them.

Created attachment 161379 [details] [review]
[StWidget] Support style transitions

It is sometimes desirable to fade smoothly between two styles
instead of changing it abruptly. Add basic support for this, but
do not expose it yet.

Created attachment 161380 [details] [review]
[StWidget] Add hover-transition property

Add a property to specify a duration for the hover transition. When
non-zero, the new style is faded in if StWidget's generic hover
support is used.

(In reply to comment #1)
> Created an attachment (id=161378) [details] [review]
> [StWidget] Clean up private member variables
> 
> When moving the drawing code into StThemeNode, most private members
> of StWidget became obsolete, so remove them.

This is of course pretty unrelated to this bug, I just noticed it when working on this, so it got mixed up ...

Review of attachment 161378 [details] [review]:

Looks good

Review of attachment 161380 [details] [review]:

Even if we start with something that's simple and specialized to a specific case like this, should it be driven out of the CSS file? So, say, before changing the hover property StWidget refs the old style node, it changes the pseudo-class, it gets the new style node, and if the new style node as '-hover-transition-duration: <..>;' with a non-zero value it sets up the animation.

(Reading the CSS3 transitions specification, I can't actually figure out if the transition properties that affect whether a transition happen are the properties *before* the change or *after* the change - my best guess is after)

I'm curious if the effect described in http://www.w3.org/TR/css3-transitions/#reversing is noticeable. It seems like you would see something like that - if you went over a bunch of hover elements in a row each element would just fade in a short time before cancelling the fade in, then fade out slowly back to the original value.

::: src/st/st-widget.c
@@ +164,3 @@
       break;
 
+    case PROP_HOVER_TRANSITION:

If we go this way I think the property (and functions) here should be hover-transition-duration rather than just hover-transition

@@ +804,3 @@
+  pspec = g_param_spec_uint ("hover-transition",
+                             "Hover transition",
+                             "Duration of transition between hover states",

should include the "in milliseconds" here too

@@ +1609,3 @@
       priv->hover = hover;
+
+      if (priv->hover_transition)

I'd use an explicit != 0 here since you are testing for a non-zero numeric value

Review of attachment 161379 [details] [review]:

I think it's very cool that you are digging into doing this. 

I think there are some basic questions that we need to answer here (and experimentation is a good way to start answering them)

 - Do we want something that is hover specific, or do we want some more generic way of annotating CSS property changes?
 - How best to implement transitions?

I assume you've looked at the CSS3 transitions specification (http://www.w3.org/TR/css3-transitions/#reversing)- I definitely don't think we want to implement that literally for a various reasons, including:

 - Animating just some properties seems like a lot of complication
 - The easing functions aren't the clutter/tweener easing functions

But it's a good reference for how someone else has approached combining CSS with transitions.

::: src/st/st-widget.c
@@ +378,3 @@
+                           &allocation,
+                           MAX (1.0 - alpha, 0) * opacity);
+      opacity *= alpha;

If you are worried about out-of-[0,1] values above, you probably should worry about them here too. (I think some of the clutter alpha functions do go outside of [0,1])

@@ +381,3 @@
+    }
+
+  st_theme_node_paint (theme_node, &allocation, opacity);

There's a problem here with the approach of linearly interpolating opacity and drawing at an opacity of x and 1-x. Consider the simple case of animating an opaque white background to an opaque white background. At the midpoint:

 (50% white OVER black) = 128,128,128
 (50% white OVER 128,128,128) = 196,196,196

So, the white => white transition has gray in the middle. But the other obvious technique of painting the old node at 100%, then fading the new node in from 0 to 100% doesn't work any better - if, say, the new node is entirely transparent then we'll just see the old node sit there and then blink out at the end.

One correct way to do it is to render the old node and the new node each to a separate fbo then use clutter multi-texturing to blend between them. Pretty expensive though to allocate new FBO's each frame.... you could cache the FBO's (both the allocated FBO's and the rendering on them) as long as the allocation is constant.

Other than that, the only real approach I can think of is to do something more like CSS transitions where instead of interpolating the rendering, you interpolate the property values. I could imagine that during the transition you compute a new theme node as an interpolation of the old and new theme nodes (of the computed values), then use that interpolated node for painting.

In some cases - perhaps most cases - it will be less efficient than the FBO approach, but it will be likely be more efficient for something like a rectangular background that can just be drawn with a COGL rectangle.

And would naturally extend to animating things that affect allocation/size, to the extent we care.

@@ +900,3 @@
+      timeline = clutter_animation_get_timeline (priv->style_transition);
+      elapsed_ms = clutter_timeline_get_elapsed_time (timeline);
+      advance_ms = MAX (duration - elapsed_ms, 0);

Hmm, this may answer my question on the other patch about 'reversed', but *definitely* needs a comment. And this way of doing it is very much specific to the bistate hover - you seem to basically be assuming here that if we override we are cancelling back to the original state, and using the same duration. (Maybe I'm misunderstanding the code here though)

@@ +905,3 @@
+    }
+
+  priv->style_transition = clutter_animation_new ();

For explicit usages of ClutterAnimation it's best to do shell_global_begin_work/end_work() manually - unfortunately, we don't have any way of hooking into ClutterTimeline uses that aren't through Tweener.js.

I'd call the member variable something like transition_animation

@@ +907,3 @@
+  priv->style_transition = clutter_animation_new ();
+  g_object_add_weak_pointer (G_OBJECT (priv->style_transition),
+                             (gpointer *)&priv->style_transition);

Looks like you have both a weak reference and a strong reference. The weak reference is unnecessary since you have the strong reference - you know when you unref the animation

@@ +917,3 @@
+
+  g_signal_connect (priv->style_transition, "completed",
+                    G_CALLBACK (st_widget_style_transition_completed), widget);

If you connect to an object, you should do one or the other of:

 - Disconnect when you connected
 - Call g_object_run_dispose() which disconnects all signals

It's unsound to count on an object being finalized at a particular time - you are counting here on the style transition being finalized before the widget since you don't do anything to explicitly disconnect.

As well as the theoretical consideration, you have a less theoretical problem if a style transition is running when the widget is destroyed - I don't see anything that removes the animation in that case.

Comment on attachment 161378 [details] [review]
[StWidget] Clean up private member variables

Attachment 161378 [details] pushed as d05cf24 - [StWidget] Clean up private member variables

Created attachment 162376 [details] [review]
[StThemeNode] Add a comparison function

Add st_theme_node_equal() - two nodes are considered equal iff they
refer to identical elements, so e.g. .example and .example:hover are
not equal, even if no .example:hover rule exists in the CSS.

Created attachment 162379 [details] [review]
[St] Add StThemeNodeTransition

Add a new ClutterAnimation subclass to allow fading between styles.

(In reply to comment #7)
> One correct way to do it is to render the old node and the new node each to a
> separate fbo then use clutter multi-texturing to blend between them. Pretty
> expensive though to allocate new FBO's each frame.... you could cache the FBO's
> (both the allocated FBO's and the rendering on them) as long as the allocation
> is constant.

This is the approach taken here - the other approach of calculating an in-between theme node sounds both a lot fancier (border-radius transitions!) and a lot more complicated ...

Two problems with this patch:
 - it doesn't work properly with shadows (the paint allocation is used to
   determine the size of the FBOs, but the shadows are painted outside the
   allocation) - could be fixed by adding some API to StThemeNode
   (e.g. st_theme_node_adjust_paint_allocation)
 - quick style changes may lead to "flashes" (not sure here what's the cause,
   in general reversing the timeline works quite well)
  

> Other than that, the only real approach I can think of is to do something more
> like CSS transitions where instead of interpolating the rendering, you
> interpolate the property values. I could imagine that during the transition you
> compute a new theme node as an interpolation of the old and new theme nodes (of
> the computed values), then use that interpolated node for painting.


This sounds really cool - rounded corners would animate correctly, shadows would grow/shrink instead of fading ... basically anything involving allocation changes will look much more correct.

On the other hand, other properties would require quite some work to implement, e.g. color changes, changes between non-gradient and gradient ...
It is also tied to the internals of StThemeNode, so changes to StThemeNode will likely require changes in the calculation of the intermediate node.


> In some cases - perhaps most cases - it will be less efficient than the FBO
> approach, but it will be likely be more efficient for something like a
> rectangular background that can just be drawn with a COGL rectangle.

The most likely candidates for transitions are text entries and buttons - all of those have rounded corners in the current theme (hardly accidentally when looking at Clearlooks), so this looks like an odd optimization to me. If we decide to use this approach, it will probably for correctness and fanciness ;)

Created attachment 162380 [details] [review]
[StThemeNode] Add transition-duration CSS property

Add a CSS property to control style transitions.

Created attachment 162382 [details] [review]
[StWidget] Support style transitions

It is sometimes desirable to fade smoothly between two styles
instead of changing it abruptly.
Add transitions controlled by the transition-duration CSS property.

(In reply to comment #6)
> (Reading the CSS3 transitions specification, I can't actually figure out if the
> transition properties that affect whether a transition happen are the
> properties *before* the change or *after* the change - my best guess is after)

Yeah, that's mostly why the first patch series didn't use CSS (beside being ignorant of the w3c proposal). Thinking about it, neither before nor after make really sense to me though - I assume that in general we want transitions to happen both ways (not necessarily using the same duration though). If this requires adding a transition duration to more than one class, we'll probably run into problems:

.example {
  transition-duration: 100;
}

.example:hover {
  transition-duration: 100;
}

.example:pressed {
  /* no transition */
}

Unfortunately, :pressed will inherit the transition-duration property from .example, which does not sound like something we'd want.

I'm not really happy with the interpretation used here either - there may still be unwanted transitions (example above: between :hover and :pressed), and we now can have two conflicting property values.

So I consider this patch (series) as just another experiment to figure out how to add transitions to our API, rather than the solution we'll want to include finally.

(In reply to comment #12)

> Yeah, that's mostly why the first patch series didn't use CSS (beside being
> ignorant of the w3c proposal). Thinking about it, neither before nor after make
> really sense to me though - I assume that in general we want transitions to
> happen both ways (not necessarily using the same duration though). If this
> requires adding a transition duration to more than one class, we'll probably
> run into problems:
> 
> .example {
>   transition-duration: 100;
> }
> 
> .example:hover {
>   transition-duration: 100;
> }
> 
> .example:pressed {
>   /* no transition */
> }
> 
> Unfortunately, :pressed will inherit the transition-duration property from
> .example, which does not sound like something we'd want.

I just checked webkit, and it takes the 'after' interpretation. So, if you implemented the 'no transition' above with:

 .example:pressed {
   transition-duration: 0s;
 }

You'd had have transitions for normal => hover, hover => normal, pressed => hover, and pressed => normal, but not normal => pressed, hover => pressed.

Which actually I think would look fine,  
 
> I'm not really happy with the interpretation used here either - there may still
> be unwanted transitions (example above: between :hover and :pressed), and we
> now can have two conflicting property values.

Yeah, the MAX(before,after) approach gets rid of the :pressed => normal transition, but it introduces the :hover => :pressed transition which is visually more problematical.

It's hard to imagine how to reasonably encode a more general state description in CSS notation (especially without libcroco changes). Maybe you could do something like:

 .example {
    transitions: inactive prelight 1s, prelight inactive 1s;
    transition-state: inactive;
 }

 .example:hover {
    transition-state: prelight;
 }

But I don't really think it's worth the complexity. My instinct is to go with the after approach and see how it works out - can we use it to easily create things that look good in practice?

Review of attachment 162376 [details] [review]:

Looks like it should work.

::: src/st/st-theme-node.c
@@ +254,3 @@
+ *     <para>their id, class, pseudo-class and inline-style match</para>
+ *   </listitem>
+ * </itemizedlist>

Might be good to add something like:

 If two nodes compare equal, they will match the same CSS rules and have the same style properties. However, two nodes that select the same style properties do not necessarily compare equal.

Review of attachment 162380 [details] [review]:

::: src/st/st-theme-node.c
@@ +1548,3 @@
 
+int
+st_theme_node_get_transition_duration (StThemeNode *node)

A doc comment here is probably a good place to document non-obvious things like the fact it is in milliseconds. And also the "after" interpretation, if we go that way

@@ +1555,3 @@
+
+  if (node->transition_duration > -1)
+    return node->transition_duration;

Ineffective cache since you don't assign to it.

Review of attachment 162379 [details] [review]:

clutter_actor_animate() is trying to "decorate" an arbitrary ClutterActor without any changes to the ClutterActor code - but we *do* have changes to the StWidget code; and once StWidget has to be changed at all, I think things would simplify a whole lot if you moved the memory management into StWidget and made StThemeNodeTransition a self contained object. That is, st_widget_recompute_style() would look along the lines of:

 if (priv->transition)
   st_theme_node_transition_update (priv->transition, new_style);
 else
   priv->transition = st_theme_node_transition_new (...);

And st_widget_dispose() would g_object_run_dispose() and unref() the theme node transition. It would be much more obviously correct that the

::: src/Makefile-st.am
@@ +94,3 @@
     st/st-theme-context.h			\
     st/st-theme-node.h				\
+    st/st-theme-node-transition.h		\

Probably should be with the private headers; it doesn't immediately appear to me something that is useful for people using ST. (Though the public/private line is a little bit blurred)

::: src/st/st-theme-node-transition.c
@@ +2,3 @@
+/* Theme node transitions for StWidget.
+
+   Copyright (C) 2010 Florian Müllner <fmuellner@src.gnome.org>

Copyright headers typically have a solid row of * down the right side. Should update this copyright to your @gnome.org address.

@@ +136,3 @@
+  timeline = clutter_animation_get_timeline (CLUTTER_ANIMATION (transition));
+
+  if (st_theme_node_equal (new_node, priv->old_theme_node))

Needs a comment about the reversing behavior.

@@ +156,3 @@
+      duration = clutter_timeline_get_duration (timeline);
+      elapsed_ms = clutter_timeline_get_elapsed_time (timeline);
+      advance_ms = duration - elapsed_ms;

So the idea here is to advance to the end immediately, and we get a ::complete before drawing the next frame? (or maybe we're 0.00001 from the end due to floating point arithmetic and we get the ::complete after drawing the next frame)

And the idea of that is that we can't do a new transition because it looks bad to do:

 background fading from red to blue, gets to a slightly purplish red
 <something changes
 background fades from blue to green

So we want to just skip the second transition? Comments definitely needed.

Maybe, rather than advancing the timeline, it would be better if the "st_theme_node_transition_update()" function I proposed returned a boolean, and if it returned FALSE, that meant to cancel the animation?

@@ +180,3 @@
+  ClutterActorBox offscreen_allocation = { 0.0, 0.0,
+                                           allocation->x2 - allocation->x1,
+                                           allocation->y2 - allocation->y1 };

non-constant initializer for structure member is a C99 addition

@@ +183,3 @@
+  ClutterAlpha *alpha;
+  CoglColor constant = { 0, 0, 0, 0 };
+  gchar *combine_string;

This temporary doesn't add much compared to just inlining the combine string - and the existence of the variable gives the impression it might vary. (I think it might also make this function easier to read if the "setup" code was moved into a separate function so we didn't have a block of variables that apply partly to the setup code path and partly to the paint code path)

@@ +201,3 @@
+  if (priv->allocation == NULL)
+    {
+      priv->allocation = clutter_actor_box_copy (allocation);

While practically speaking it doesn't matter, I don't really like using a secondary allocation for a simple structure like this - I'd use an inline structure, and a has_allocation flag bit.

@@ +236,3 @@
+
+      cogl_push_framebuffer (priv->old_offscreen);
+      cogl_clear (&constant, COGL_BUFFER_BIT_COLOR);

Don't double-use this variable for two different purposes, just create one that is transparent, and one which has the alpha channel you want.

@@ +258,3 @@
+  alpha = clutter_animation_get_alpha (CLUTTER_ANIMATION (transition));
+  constant.alpha = clutter_alpha_get_alpha (alpha) * paint_opacity;
+  cogl_color_premultiply (&constant);

You don't need the premultiply here since you are only using the A channel anyways - what premultiply does is multiply the color channels (0's) with the alpha channel.

Created attachment 162701 [details] [review]
[StThemeNode] Add a comparison function

Updated comment.

Created attachment 162702 [details] [review]
[St] Add StThemeNodeTransition

Introduce a small helper class in order to add transitions between
theme nodes to StWidget.

Much simpler than the previous version. Still occasional "flashes"
when updating the transition though ...

Created attachment 162704 [details] [review]
[StThemeNode] Add transition-duration CSS property

(In reply to comment #13)
> It's hard to imagine how to reasonably encode a more general state description
> in CSS notation (especially without libcroco changes). Maybe you could do
> something like:
> 
>  .example {
>     transitions: inactive prelight 1s, prelight inactive 1s;
>     transition-state: inactive;
>  }
> 
>  .example:hover {
>     transition-state: prelight;
>  }

Mmmh, I'd prefer something along the lines of:

  .example {
    transitions: prelight 1s;
    transition-state: inactive;
  }

  .example:hover {
    transitions: inactive 1s;
    transition-state: prelight;
  }


> But I don't really think it's worth the complexity. My instinct is to go with
> the after approach and see how it works out - can we use it to easily create
> things that look good in practice?

Hopefully we can. So yeah, better avoid complexity unless we run into problems.

Created attachment 162705 [details] [review]
[StWidget] Support style transitions

It is sometimes desirable to fade smoothly between two styles
instead of changing it abruptly.
Add transitions controlled by the transition-duration CSS property.

Review of attachment 162701 [details] [review]:

::: src/st/st-theme-node.c
@@ +241,3 @@
+ * Compare two #StThemeNodes. Two nodes which compare equal will match
+ * the same CSS rules and have the same style properties. However, two
+ * nodes that select the same style properties do not necessarily compare

To edit myself, it would be clearer to say "two nodes that have end up with identical style properties do not necessarily compare equal"

Review of attachment 162704 [details] [review]:

Looks good

Review of attachment 162705 [details] [review]:

Just a few comments on this

::: src/st/st-widget.c
@@ +1214,3 @@
+                         StWidget              *widget)
+{
+  g_object_unref (widget->priv->transition_animation);

I would consider it good form to g_object_run_dispose() the transition object here too, and not count on the unref causing the object to be finalized right here and now. (Just good form, no practical bug since our only signal connection is a run-once thing and has already been called here.)

@@ +1217,3 @@
+  widget->priv->transition_animation = NULL;
+
+  clutter_actor_queue_relayout (CLUTTER_ACTOR (widget));

Why is this needed?

(In reply to comment #23)
> @@ +1217,3 @@
> +  widget->priv->transition_animation = NULL;
> +
> +  clutter_actor_queue_relayout (CLUTTER_ACTOR (widget));
> 
> Why is this needed?

It isn't. I was assuming that the last "new-frame" signal could be emitted before the timeline completes, but apparently it is emitted right before the "completed" signal is emitted.

Created attachment 162774 [details] [review]
[StThemeNode] Add a comparison function

Reattaching to maintain patch order.

Created attachment 162775 [details] [review]
[StThemeNode] Add helper method to get the paint allocation

StThemeNodes may have properties - namely shadows - which paint
outside an actor's allocation. This is not a problem unless drawing
is redirected to an offscreen buffer, in which case the actually
painted size is needed in advance when setting up the buffer.

Add a convenience method to calculate an allocation large enough to
paint the node.

Created attachment 162776 [details] [review]
[St] Add StThemeNodeTransition

Updated patch to enable transitions between theme nodes with different paint allocations.

Created attachment 162777 [details] [review]
[StThemeNode] Add transition-duration CSS property

Reattaching to maintain patch order.

Created attachment 162778 [details] [review]
[StWidget] Support style transitions

Updated according to Owen's comment.

Review of attachment 162778 [details] [review]:

This patch looks good now

Review of attachment 162776 [details] [review]:

Mostly looks very good to me. A few comments marked below, but I didn't immediately see anything that would cause the flash you described. My best guess would be that the style of the actor is getting updated more times than you think - that instead of going from:

 in: (A => B) out: (B => A)

that you have:

 in: (A => B) out: (B => B' => A)

If that is going on, first step in fixing is figuring what the mysterious B' state is.

::: src/st/st-theme-node-transition.c
@@ +100,3 @@
+
+  clutter_alpha_set_mode (transition->priv->alpha, CLUTTER_EASE_IN_OUT_QUAD);
+  clutter_alpha_set_timeline (transition->priv->alpha, timeline);

Looks like you leak the timeline- set_timeline() takes another reference, and you never unref. In my opinion it would be clearest if you had priv->timeline, just made it another object that we own, and didn't use clutter_alpha_get_timeline()

@@ +150,3 @@
+{
+  StThemeNodeTransitionPrivate *priv = transition->priv;
+  ClutterActorBox old_allocation, new_allocation;

I don't like calling these variables allocations, they aren't allocations. An allocation is something that an actor has.

@@ +244,3 @@
+  StThemeNodeTransitionPrivate *priv = transition->priv;
+
+  static ClutterActorBox offscreen_allocation;

Again, this isn't an allocation

@@ +246,3 @@
+  static ClutterActorBox offscreen_allocation;
+  static ClutterActorBox cached_allocation;
+  static guint8 cached_opacity = 0;

I'm not sure if this was a misinterpretation of a comment I made - I commented that the I thought you should have, in the private structure:

 ClutterActorBox last_allocation;

Not:

 ClutterActorBox *last_allocation;

But I didn't mean to put it in the function here - we should be able to have multiple transitions going at the same time.

@@ +260,3 @@
+  if (!clutter_actor_box_equal (allocation, &cached_allocation) ||
+      paint_opacity != cached_opacity)
+    priv->needs_allocation = TRUE;

The fact that opacity gotten taken into account here indicates that the 'needs_allocation' name is wrong - maybe needs_setup?

Created attachment 162973 [details] [review]
[StThemeNode] Add a comparison function

Reattaching to maintain patch order

Created attachment 162974 [details] [review]
[StThemeNode] Add helper method to get the paint allocation

Reattaching to maintain patch order.

Created attachment 162978 [details] [review]
[St] Add StThemeNodeTransition

Updated according to review - still occasional flashs, still looking into it.

Created attachment 162979 [details] [review]
[StThemeNode] Add transition-duration CSS property

Reattaching to maintain patch order

Created attachment 162980 [details] [review]
[StWidget] Support style transitions

Reattaching to maintain patch order

Created attachment 163050 [details] [review]
[St] Add StThemeNodeTransition

(In reply to comment #31)
> Mostly looks very good to me. A few comments marked below, but I didn't
> immediately see anything that would cause the flash you described. My best
> guess would be that the style of the actor is getting updated more times than
> you think - that instead of going from:
> 
>  in: (A => B) out: (B => A)
> 
> that you have:
> 
>  in: (A => B) out: (B => B' => A)
> 
> If that is going on, first step in fixing is figuring what the mysterious B'
> state is.

The bad news is: there is no mysterious B'.

Good news: interdiff to the previous patch version

--- a/src/st/st-theme-node-transition.c
+++ b/src/st/st-theme-node-transition.c
@@ -131,8 +131,13 @@ st_theme_node_transition_update (StThemeNodeTransition *transition,
    * Otherwise, we should initiate a new transition from the
    * current state to the new one; this is hard to do, so we
    * just cancel the ongoing transition in that case.
+   * Note that reversing a timeline before any time elapsed
+   * results in the timeline's time position being set to the
+   * full duration - this is not what we want, so we cancel the
+   * transition as well in that case.
    */
-  if (st_theme_node_equal (new_node, old_node))
+  if (st_theme_node_equal (new_node, old_node)
+      && clutter_timeline_get_elapsed_time (priv->timeline) > 0)
     {
       if (direction == CLUTTER_TIMELINE_FORWARD)
         clutter_timeline_set_direction (priv->timeline,
-- 
1.7.1

Review of attachment 162974 [details] [review]:

::: src/st/st-theme-node.c
@@ +2518,3 @@
+ * properties which paint outside the actor's assigned allocation
+ * (currently only st-shadow). This is a convenience function meant to be
+ * used when calling st_theme_node_paint() to draw to a framebuffer.

This doesn't actually correspond to my definition of a "convenience function", since it provides information about something the caller can't know  - "how the theme node is going to paint itself". You could say "When painting a theme node to an offscreen buffer, this function can be used to determine the necessary size of the buffer". Minor point

Review of attachment 163050 [details] [review]:

Good to go!

Attachment 162973 [details] pushed as 24a4ca0 - [StThemeNode] Add a comparison function
Attachment 162974 [details] pushed as af3ca02 - [StThemeNode] Add helper method to get the paint allocation
Attachment 162979 [details] pushed as d4a8c64 - [StThemeNode] Add transition-duration CSS property
Attachment 162980 [details] pushed as db45c09 - [StWidget] Support style transitions
Attachment 163050 [details] pushed as 0deffba - [St] Add StThemeNodeTransition