See http://lists.w3.org/Archives/Public/www-style/2011May/0090.html
David, Do you have opinions on this? We think it is a good idea but I don't want to implement it if Mozilla don't agree.
Sounds fine to me.
Created attachment 125534 [details] Patch Proposed patch.
Comment on attachment 125534 [details] Patch Attachment 125534 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11428136
Comment on attachment 125534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125534&action=review Some nitpicks to the TextureMapper part. > Source/WebCore/platform/graphics/texmap/TextureMapperAnimation.cpp:29 > +static double normalizedAnimationValue(double runningTime, double duration, unsigned direction) Why not just pass the AnimationDirection enum? > Source/WebCore/platform/graphics/texmap/TextureMapperAnimation.cpp:42 > + if ((direction == Animation::AnimationDirectionAlternate) && (loopCount & 1) > + || ((direction == Animation::AnimationDirectionAlternateReverse) && !(loopCount & 1)) > + || direction == Animation::AnimationDirectionReverse) > + normalized = 1 - normalized; this part is a bit verbose... can be better in a static function that gets AnimationDirection and loopCount and returns if reverse or not.
Comment on attachment 125534 [details] Patch Attachment 125534 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11428139
Comment on attachment 125534 [details] Patch r- based on Noam's comments.
Created attachment 125673 [details] Patch Proposed patch v2.
The last patch addresses Noam suggestions. (In reply to comment #8) > Created an attachment (id=125673) [details] > Patch > > Proposed patch v2.
Comment on attachment 125673 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125673&action=review Apart from the naming nitpick, this is an r+ for me on the TextureMapper part. smfr/others, are you ok with the common part? > Source/WebCore/platform/graphics/texmap/TextureMapperAnimation.cpp:30 > +static bool animationValueIsReverse(Animation::AnimationDirection direction, int loopCount) <nitpicks/> name -> shouldReverseAnimationValue
Created attachment 125880 [details] Patch Fix nitpick.
Comment on attachment 125880 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=125880&action=review I think we need a test for "reverse" as well. I could only see one for "alternate-reverse". r- for the missing test, but otherwise looks good. NOTE: Apple will need to raise a followup bug because our hardware animation won't support this directly. > LayoutTests/animations/animation-direction-alternate-reverse.html:53 > +<!-- Test animation-direction: alternate --> Nit: it's now alternate-reverse
While I think of it, it would be nice to add tests of this in combination with fill-mode too. To make sure things extend in the right way.
<rdar://problem/10822885>
Created attachment 126129 [details] Patch Updated patch. Add new tests.
Comment on attachment 126129 [details] Patch great!
Comment on attachment 126129 [details] Patch Rejecting attachment 126129 [details] from commit-queue. New failing tests: platform/chromium/compositing/layout-width-change.html Full output: http://queues.webkit.org/results/11460584
Comment on attachment 126129 [details] Patch Trying again, this failing test does not looks related with this patch
Comment on attachment 126129 [details] Patch Clearing flags on attachment: 126129 Committed r107162: <http://trac.webkit.org/changeset/107162>
All reviewed patches have been landed. Closing bug.