[webkit-reviews] review granted: [Bug 137731] Use is<>() / downcast<>() for TransformOperation subclasses : [Attachment 239847] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Oct 15 09:21:40 PDT 2014


Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 137731: Use is<>() / downcast<>() for TransformOperation subclasses
https://bugs.webkit.org/show_bug.cgi?id=137731

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=239847&action=review


r=me as long as you fix the coordinated graphics build

> Source/WebCore/platform/graphics/transforms/Matrix3DTransformOperation.cpp:37

> -    if (!isSameType(o))
> +    if (!isSameType(other))
>	   return false;
> -    const Matrix3DTransformOperation& m = toMatrix3DTransformOperation(o);
> -    return m_matrix == m.m_matrix;
> +    return m_matrix == downcast<Matrix3DTransformOperation>(other).m_matrix;


Almost seems like we’d want to write this with && on a single line.a

>
Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp
:187
> -	       encoder << toScaleTransformOperation(operation)->x();
> -	       encoder << toScaleTransformOperation(operation)->y();
> -	       encoder << toScaleTransformOperation(operation)->z();
> +	       const auto& scaleOperation =
downcast<ScaleTransformOperation>(*operation);
> +	       encoder << scaleOperation.x();
> +	       encoder << scaleOperation.y();
> +	       encoder << scaleOperation.z();
>	       break;

I wouldn’t expect this to compile unless you add braces to scope the local
variable so that jumping to later cases doesn’t jump across it. I am guessing
that’s why EFL failed to build.

>
Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp
:193
> +	       const auto& translateOperation =
downcast<TranslateTransformOperation>(*operation);

Ditto.

>
Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp
:202
> +	       const auto& rotateOperation =
downcast<RotateTransformOperation>(*operation);

Ditto.

>
Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp
:211
> +	       const auto& skewOperation =
downcast<SkewTransformOperation>(*operation);

Ditto.


More information about the webkit-reviews mailing list