[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