Bug 92927 - Switch mapLocalToContainer to use a flag instead of boolean parameters
Summary: Switch mapLocalToContainer to use a flag instead of boolean parameters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Levi Weintraub
URL:
Keywords:
Depends on:
Blocks: 89238
  Show dependency treegraph
 
Reported: 2012-08-01 17:49 PDT by Levi Weintraub
Modified: 2012-08-03 15:24 PDT (History)
3 users (show)

See Also:


Attachments
Patch (31.12 KB, patch)
2012-08-02 10:12 PDT, Levi Weintraub
no flags Details | Formatted Diff | Diff
Patch (33.09 KB, patch)
2012-08-02 14:06 PDT, Levi Weintraub
no flags Details | Formatted Diff | Diff
Patch for landing (32.97 KB, patch)
2012-08-03 10:27 PDT, Levi Weintraub
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Levi Weintraub 2012-08-01 17:49:53 PDT
I intend on adding another mode of operation for bug switching between pixel snapping which will further complicate the current contract.
Comment 1 Levi Weintraub 2012-08-02 10:12:17 PDT
Created attachment 156114 [details]
Patch
Comment 2 Build Bot 2012-08-02 10:34:07 PDT
Comment on attachment 156114 [details]
Patch

Attachment 156114 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13425302
Comment 3 Dirk Schulze 2012-08-02 10:41:07 PDT
Comment on attachment 156114 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156114&action=review

> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:193
> +void RenderSVGForeignObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const

You need to comment it out, or flag it unused.

> Source/WebCore/rendering/svg/RenderSVGInline.cpp:82
> +void RenderSVGInline::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const

Ditto.

> Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:58
> +void RenderSVGModelObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const

Ditto.
Comment 4 Levi Weintraub 2012-08-02 11:26:48 PDT
(In reply to comment #3)
> (From update of attachment 156114 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156114&action=review
> 
> > Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:193
> > +void RenderSVGForeignObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const
> 
> You need to comment it out, or flag it unused.
> 
> > Source/WebCore/rendering/svg/RenderSVGInline.cpp:82
> > +void RenderSVGInline::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const
> 
> Ditto.
> 
> > Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:58
> > +void RenderSVGModelObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const
> 
> Ditto.

Of course, thank you :)
Comment 5 Levi Weintraub 2012-08-02 14:06:15 PDT
Created attachment 156161 [details]
Patch
Comment 6 Dirk Schulze 2012-08-02 20:21:52 PDT
Comment on attachment 156161 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=156161&action=review

LGTM. Just some snippets, r=me.

> Source/WebCore/rendering/svg/RenderSVGForeignObject.cpp:195
> +void RenderSVGForeignObject::mapLocalToContainer(RenderBoxModelObject* repaintContainer, TransformState& transformState, MapLocalToContainerFlags mode, bool* wasFixed) const
>  {
> +    UNUSED_PARAM(mode);

Sorry for not mention it before. But isn't it easier to just not use 'mode'? I think MapLocalToContainerFlags is self descriptive.

> Source/WebCore/rendering/svg/RenderSVGInline.cpp:84
> +    UNUSED_PARAM(mode);

Ditto.

> Source/WebCore/rendering/svg/RenderSVGModelObject.cpp:60
> +    UNUSED_PARAM(mode);

Ditto.

> Source/WebCore/rendering/svg/RenderSVGText.cpp:116
> +    UNUSED_PARAM(mode);

Ditto.
Comment 7 Levi Weintraub 2012-08-03 10:27:08 PDT
Created attachment 156410 [details]
Patch for landing
Comment 8 WebKit Review Bot 2012-08-03 15:23:58 PDT
Comment on attachment 156410 [details]
Patch for landing

Clearing flags on attachment: 156410

Committed r124662: <http://trac.webkit.org/changeset/124662>
Comment 9 WebKit Review Bot 2012-08-03 15:24:02 PDT
All reviewed patches have been landed.  Closing bug.