Change 20100623-maxcarlson-M by maxcarl...@friendly on 2010-06-23 16:46:00 PDT
in /Users/maxcarlson/openlaszlo/trunk-clean
for http://svn.openlaszlo.org/openlaszlo/trunk
Summary: UPDATED AGAIN: Correct basecomponent tinting.
Bugs Fixed: LPP-9132 - Regression:
examples/animation/animation.lzx?lzr=swf8&debug=true failing to start up
Technical Reviewer: ptw
QA Reviewer: hminsky, mdemmon
Details: I'm checking this in ahead of review because it's been trhrough
several rounds, and it fixes a regression swatchview.
Updating to address Tucker's latest comments:
1) You still have the situation that set_tintcolor is constructing a
colortransform that maps the tint to offsets, yet set_colortransform updates
tintcolor by reading out the multipliers. This can't be right. set_tintcolor
is cheating because it calls set_colortransform and then smashes the tintcolor
that the transform created with the actual value. There should be an assertion
in set_tintcolor that after calling set_colortransform the value of tintcolor
=== the value you are trying to set.
Doe! Fixed. Nice catch!
2) If the point of tintcolor is to do the same thing as was done in
basecomponents, then maybe we should just copy the algorithm from there and
have basecomponents use tintcolor directly.
I'm not a color scientist, and I don't play one on TV, but from what I've read,
our use of `tint` in basecomponents (and in view now) is non-standard. It
seems basecomponents has a _very_ narrow application, which is a grayscale
asset that it is colorizing. It's more like the color is the base and the
grayscale is creating 'tints' of the base color. I think this is why it works
with the multiplier.
Tintcolor does something different - essentially, it emulates the old fgcolor
tinting behavior. For better or for worse, it's here to stay :)
Comment:
1) I just have one curiosity: Why do we encode alpha as a fraction? As you
say, this is fragile (it's effectively a feature of binary floating point
representaions of decimal fractions). Why not encode the alpha in the 'top
byte' like it is done in most hardwares? Seems like we are asking for trouble
here...
Yeah, I was having trouble reliably getting 24-bit ints across runtimes, at
least with the << operator. AS3 does it reliably with the uint type...
Filed here: http://jira.openlaszlo.org/jira/browse/LPP-9154
Updating to address Tucker's comments:
1) I'd like to see us use a more modern format like the as3 colorTransform
(i.e., name the fields {red,green,blue}{offset,multiplier} instead of
{r,g,b}{a,b}). Since you are adding a new API here (the colortransform
attribute), let's use the opportunity to make the type saner. The old,
deprecated API can still take the old format -- it should be simple to map to
the new format.
Done.
2) I think in rgbatoint you should clip each of the input values (& 0xff) just
for cleanliness.
Done.
3) findAlpha looks wrong. Elsewhere alpha is encoded by `fraction = alpha /
25500` but in findAlpha you are decoding with `alpha = fraction * 25600`.
This breaks the rounding, which is quite fragile. I added a comment about
needing to do this.
Otherwise:
Updating to address comments from Andre and Tucker. Since there is no good
replacement for LzView.setColorTransform(), I moved it to a setter. I also
fixed the tinting implementation to work consistently across swf8 and swf10.
LzUtils - Ensure LzColorUtils.fromrgb() returns integers when no alpha
component is supplied.
LzUtils - Update/refactor LzColorUtils
swf/LzSprite - setColorTransform() converts multipliers to percentage
swf9/LzSprite - setColorTransform() takes values literally
LaszloView - Update docs, add colortransform setter to be used in place of
setColorTransform(), update tintcolor when possible. Update tintcolor setter
to tint correctly in swf8 and swf10.
basecomponent - Use LzView.colortransform instead of setColorTransform().
swatchview - Override colortransform setter instead of setColorTransform().
Use LzUtils.applyTransform() to derive transformed background color.
Tests: See LPP-9132 for a testcase that passes in swf8 and swf10.
test/lfc/lzunit-lzutils.lzx passes across all runtimes.
Files:
M WEB-INF/lps/lfc/kernel/swf/LzSprite.as
M WEB-INF/lps/lfc/kernel/swf9/LzSprite.as
M WEB-INF/lps/lfc/services/LzUtils.lzs
M WEB-INF/lps/lfc/views/LaszloView.lzs
M lps/components/base/swatchview.lzx
M lps/components/base/basecomponent.lzx
Changeset:
http://svn.openlaszlo.org/openlaszlo/patches/20100623-maxcarlson-M.tar
