[webkit-reviews] review requested: [Bug 87686] [chromium] Accelerated animations should use WebTransformOperations : [Attachment 145914] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Jun 5 19:04:03 PDT 2012


vollick at chromium.org has asked	for review:
Bug 87686: [chromium] Accelerated animations should use WebTransformOperations
https://bugs.webkit.org/show_bug.cgi?id=87686

Attachment 145914: Patch
https://bugs.webkit.org/attachment.cgi?id=145914&action=review

------- Additional Comments from vollick at chromium.org
(In reply to comment #22)
> (From update of attachment 145901 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=145901&action=review
>
> > Source/WebCore/platform/graphics/chromium/AnimationTranslationUtil.h:2
> > + * Copyright (C) 2010 Google Inc. All rights reserved.
>
> for this and other new files - it's 2012 now, and we prefer 2-clause for new
files (although that's not a big deal)
Fixed.
>
> > Source/WebCore/platform/graphics/chromium/AnimationTranslationUtil.h:43
> > +PassOwnPtr<CCActiveAnimation> createActiveAnimation(const
KeyframeValueList&, const Animation*, size_t animationId, size_t groupId,
double timeOffset, const FloatSize& boxSize);
>
> Would you mind documenting the behavior of this function with non-supported
input (like non-invertable transforms or whatnot?)  Looks like it just returns
nullptr, but it'd be good to know that without having to read through the impl
Done.
>
> > Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:506
> > +	 // Bail early if we have a large rotation.
>
> Hm - we bail here for some things, but it looks like createActiveAnimation()
returns nullptr for other "bad" values.  What's the rationale for the split?
I do the big angle check in GraphicsLayerChromium, because I use the protected
GraphicsLayer::validateTransformOperations. I can do everything else in the
AnimationTranslationUtil.


More information about the webkit-reviews mailing list