CSSPlugin.js does not take all css styles into account

[geo-at-github]

The "CSSplugin.js" uses element.style instead of getComputedStyle(). This prevents it from handling predefined css attributes correctly (e.g. predefined values from a css file are ignored). Also float values are not converted correctly because of parseInt().

Here is a quick fix:

/**
 * @method init
 * @protected
 * @static
 **/
CSSPlugin.init = function(tween, prop, value) {
    var sfx0,sfx1,map = CSSPlugin.cssSuffixMap;
    var style = CSSPlugin.getComputedStyle(tween.target, prop);
    if ((sfx0 = map[prop]) == null || !style) { return value; }
    var str = style;
    if (!str) { return 0; } // no style set.
    var i = str.length-sfx0.length;
    if ((sfx1 = str.substr(i)) != sfx0) {
        throw("CSSPlugin Error: Suffixes do not match. ("+sfx0+":"+sfx1+")");
    } else {
        if( str.indexOf('.') < 0 )
            return parseInt(str.substr(0,i));
        else
            return parseFloat(str.substr(0,i));
    }
}

/**
 * @method getComputedStyle
 * @protected
 * @static
 **/
CSSPlugin.getComputedStyle = function(target, prop) {
    if(!window.getComputedStyle){
        if(document.defaultView && document.defaultView.getComputedStyle){
            return document.defaultView.getComputedStyle.getPropertyValue(prop);
        }
        else{
            var camelCasedCssProperty = prop.replace(/-([a-z])/g, function (g) { return g.charAt(1).toUpperCase() });
            if(target.currentStyle){
                return target.currentStyle(camelCasedCssProperty);
            }
            else{
                return target.style[camelCasedCssProperty];
            }
        }
    }
    else{
        return window.getComputedStyle(target).getPropertyValue(prop);
    }
}

[lannymcnie]

CSSPlugin is just a sample to show you what is possible with plugins. I will leave this open as a reminder to eventually address it.

Thanks for the heads up!

[gskinner]

We've done some updates to the CSSPlugin, but are not using getComputedStyle yet.

My main concerns are that getComputedStyle is a fairly expensive operation, and we ideally should look it up for every single new .to that includes new properties. We could possibly cheat, and store the value off in the tween, but that could potentially get memory intensive, and lead to odd results in some edge cases.

I'll do some focused performance testing and see if this is as much of a concern as I'm making it out to be.

[gskinner]

I did a few really naive tests, culminating in this: https://jsfiddle.net/z0uo36q9/

It looks promising. The first getComputedStyle call is quite expensive (~0.1ms), but after that they are relatively cheap (<0.001ms each). Again this is using a really simple hierarchy and stylesheet, so I imagine it would be more expensive with more complex scenarios.

[gskinner]

Ok. Did some slightly more real-world testing, and it's not so good. I implemented getComputedStyle into CSSPlugin (literally a single line change), and the time to create 1000 tweens went from ~50ms to ~380ms. Moving some styles into a class increased that further to ~400ms, so complexity seems to factor in.

Another, perhaps more major concern is that getComputedStyle() normalizes units. Right now I can set style.top = "20%", and tween to 50%. With getComputedStyle that gets converted to px which makes things a lot less responsive.

Maybe we could add an option to enable this?

CSSPlugin.computedDefault = false;
Tween.get(o, {pluginData:{CSS_computed:true}}).to( ... );

Feedback welcome.

[gskinner]

I have added this as CSSPlugin.compute and pluginData.CSS_compute. Testing and feedback are appreciated.