GNOME Bugzilla – Bug 698766
Implicit transitions queued on invisible actors should be ignored
Last modified: 2014-03-19 13:52:29 UTC
I put git master here, but it's actually on the 1.16 branch. The following piece of JS fails with a type error : ----------------------------------------------------------- const Lang = imports.lang; const Mainloop = imports.mainloop; const Gettext = imports.gettext; const _ = imports.gettext.gettext; const Gio = imports.gi.Gio; const GLib = imports.gi.GLib; const Clutter = imports.gi.Clutter; Clutter.init(null, null); let stage = new Clutter.Stage({ width: 800, height: 600, }); let rect = new Clutter.Actor({ width: 100, height:100, background_color: Clutter.Color.new(0xff, 0, 0, 0xff), opacity: 0, }); stage.add_child(rect); stage.show(); rect.save_easing_state(); rect.set_easing_duration(6000); rect.set_easing_mode(Clutter.AnimationMode.LINEAR); rect.opacity = 0xff; rect.restore_easing_state(); let anim = rect.get_transition('opacity'); anim.set_repeat_count(-1); Clutter.main(); ----------------------------------------------------------- But works if I revert 8f032d595263a2f4fdb057ae5d954ee8236e96cf "actor: Skip transitions on invisible actors".
strictly speaking, a dupe of 695870, but since it was a specific bug, we should keep this open instead. temporary solution: if you're queueing a transition on an invisible actor, make it explicit.
queuing transitions on invisible/unpainted actors is pointless work that should just be skipped. in this case, opacity = 0 leading to not being painted is an internal optimization, so we should probably add a special case in _clutter_actor_create_transition() and ignore "opacity", with a pretty big comment on top.
Created attachment 242359 [details] [review] actor: Ignore :opacity when skipping transitions on invisible actors The internal optimization that we employ in clutter_actor_paint() and skips painting fully transparent actors is exactly that: an internal optimization. Caller code should not be aware of this change, and it should not influence code outside of ClutterActor itself. If we add the simple visibility check to ignore implicit transitions on invisible actors, then we need to check for all the escape hatches we have in place in ClutterActor: clones, cloned branches, and opacity bail outs.
Created attachment 243402 [details] [review] actor: Improve conditions for skipping implicit transitions The "should this implicit transition be skipped" check should live into its own function, where we can actually explain what it does and which conditions should be respected. Instead of just blindly skipping actors that are unmapped, or haven't been painted yet, we should add a couple of escape hatches. First of all, we don't want :allocation to be implicitly animated until we have been painted (thus allocated) once; this avoids actors "flying in" into their allocation. We also want to allow implicit transitions on the opacity even if we haven't been painted yet; the internal optimization that we employ in clutter_actor_paint() and skips painting fully transparent actors is exactly that: an internal optimization. Caller code should not be aware of this change, and it should not influence code outside of ClutterActor itself. The rest of the conditions are the same: if the easing state's duration is zero, or if the actor is both unmapped and not in a cloned branch of the scene graph, then implicit transitions are pointless, as they won't be painted.
Attachment 243402 [details] pushed as 9424e99 - actor: Improve conditions for skipping implicit transitions
Commit 18f7a4aa12a0563283b3788d95ed837540da330f reintroduced that bug by removing the check on pspec in should_skip_implicit_transition()
this is actually a bit odd: I don't use the was_painted flag any more, so only the :allocation property is handled as a special case. it may be the case that the actor is not being transitioned because it does not have a valid allocation? I can re-add the escape hatch, but I'd rather understand the issue first.
Indeed, I came up with a slightly different test case which is basically like the one in my very first comment. Except I moved the stage.show(); call just before the Clutter.main();
does this still happen?
It's working properly on the 1.18. Thanks!