Change 20100526-maxcarlson-9 by maxcarl...@friendly on 2010-05-26 16:35:11 PDT
in /Users/maxcarlson/openlaszlo/trunk-clean
for http://svn.openlaszlo.org/openlaszlo/trunk
Summary: Clean up LzAnimator/LzNode interaction, part 2
Bugs Fixed: LPP-9020 - Support CSS3 transitions (partial)
Technical Reviewer: [email protected]
QA Reviewer: ptw
Details: Updated to address Andre and Tucker's comments:
> + this.__animExpected = {};
> + this.__animCounter = {};
Only a few nodes will be animated in an application, so both hashes should be
built lazily. Otherwise every node construction will cost +2 Objects.
Fixed.
> [...]
> + var __animExpected = {};
> [...]
> + var __animCounter = {};
Every node construction will cost +4 Objects for swf9/10
(http://jira.openlaszlo.org/jira/browse/LPP-5232).
Fixed.
> @@ -218,23 +227,32 @@
> var attr:String = this.attribute;
>
> var from:* = this.from;
> + var expected = targ.__animExpected;
> + var expectedvalue = (expected[attr] != null ? expected[attr] :
> targ[attr]); // inlined targ.getExpectedAttribute(attr);
> if (from != null) {
> targ.setAttribute(attr, from);
> - var d:Number = from - targ.getExpectedAttribute(attr);
> - targ.addToExpectedAttribute(attr, d);
> + var d:Number = from - expectedvalue;
> + // inlined targ.addToExpectedAttribute(attr, d);
> + var current = expected[attr];
> + expected[attr] = (current != null ? current : targ[attr]) + d;
> }
>
> if (! this.relative) {
> - this.to = this.origto - targ.getExpectedAttribute(attr);
> + this.to = this.origto - expectedvalue;
This seems to be wrong:
expected[attr] is updated (inlined call to addToExpectedAttribute), but later
"expectedvalue" is used again.
Fixed.
On 5/25/2010 4:54 PM, P T Withington wrote:
> Comments:
>
> 1) I'd just remove all the animator functions you commented out of LzNode.
> They are clutter and will (already for one of them) rot. svn can find them
> if we really need them back.
>
You mean to rot like setExpectedAttribute() which wasn't updated to use
__animExpected ;-)
Fixed.
> 2) Rather than creating multiple object hashes for the various animation
> attributes, why not define a class (data structure) that holds all the
> animation properties of an attribute? Thus, when animating an attribute, you
> will just say something like:
>
> var aas = this.__animatedAttributes;
> if (! aas[name]) { aas[name] = new AnimationAttribute(start, end, ...);
>
> ?
Fixed.
Also in stop()
> var chash = targ.__animCounter;
> var counter = chash[attr];
> if (counter == null || counter - 1<= 0) {
> chash[attr] = 0;
> targ.__animExpected[attr] = null;
> } else {
> chash[attr] = counter - 1;
> targ.__animExpected[attr] = this.to - this.currentValue;
> }
>
> /*
> var e_prop:String = "e_" + (this.attribute cast String);
> if (! targ[e_prop].c) {
> targ[e_prop].c = 0;
> }
> targ[e_prop].c -= 1; //decrement animation counter for prop
> if (targ[e_prop].c<= 0) {
> targ[e_prop].c = 0;
> targ[e_prop].v = null;
> } else {
> targ[e_prop].v -= this.to - this.currentValue;
> }
> */
>
1) the complete commented out block seems to be unnecessary
Fixed.
2)
targ[e_prop].v -= this.to - this.currentValue;
is changed to
targ.__animExpected[attr] = this.to - this.currentValue;
Shouldn't that be -= ?
Good catch. Fixed.
And I still think you cannot reuse "expectedvalue" here:
> if (! this.relative) {
> - this.to = this.origto - targ.getExpectedAttribute(attr);
> + this.to = this.origto - expectedvalue;
>
Fixed.
LzNode - Use a single property to track animator values.
LzAnimatorGroup - Update comments, remove cruft. Update __LZhalt so it can be
called from a delegate.
LaszloAnimation - Use attributename + '__lzcounter' to track animation counter
values. Shorten lookups. Fix bugs in reviewer comments.
Tests: Visual inspection, animators work as before across all runtimes.
Files:
M WEB-INF/lps/lfc/core/LzNode.lzs
M WEB-INF/lps/lfc/controllers/LzAnimatorGroup.lzs
M WEB-INF/lps/lfc/controllers/LaszloAnimation.lzs
Changeset:
http://svn.openlaszlo.org/openlaszlo/patches/20100526-maxcarlson-9.tar
