Bug 137731 - Use is<>() / downcast<>() for TransformOperation subclasses
Summary: Use is<>() / downcast<>() for TransformOperation subclasses
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on: 137056
Blocks:
  Show dependency treegraph
 
Reported: 2014-10-14 20:01 PDT by Chris Dumez
Modified: 2014-10-15 12:15 PDT (History)
13 users (show)

See Also:


Attachments
Patch (31.53 KB, patch)
2014-10-14 20:09 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (31.53 KB, patch)
2014-10-14 21:22 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (31.74 KB, patch)
2014-10-15 10:21 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2014-10-14 20:01:14 PDT
Use is<>() / downcast<>() for TransformOperation subclasses
Comment 1 Chris Dumez 2014-10-14 20:09:49 PDT
Created attachment 239842 [details]
Patch
Comment 2 Chris Dumez 2014-10-14 21:22:17 PDT
Created attachment 239847 [details]
Patch
Comment 3 Darin Adler 2014-10-15 09:21:35 PDT
Comment on attachment 239847 [details]
Patch

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.
Comment 4 Chris Dumez 2014-10-15 10:21:34 PDT
Created attachment 239876 [details]
Patch
Comment 5 Chris Dumez 2014-10-15 11:38:09 PDT
Comment on attachment 239876 [details]
Patch

All green \o/
Comment 6 WebKit Commit Bot 2014-10-15 12:15:22 PDT
Comment on attachment 239876 [details]
Patch

Clearing flags on attachment: 239876

Committed r174739: <http://trac.webkit.org/changeset/174739>
Comment 7 WebKit Commit Bot 2014-10-15 12:15:30 PDT
All reviewed patches have been landed.  Closing bug.