Bug 92927 – Switch mapLocalToContainer to use a flag instead of boolean parameters

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Levi Weintraub]

I intend on adding another mode of operation for bug switching between pixel snapping which will further complicate the current contract.

[Levi Weintraub]

Created attachment 156114 [details]
Patch

[Build Bot]

Comment on attachment 156114 [details]
Patch

Attachment 156114 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13425302

[Dirk Schulze]

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.

[Levi Weintraub]

(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 :)

[Levi Weintraub]

Created attachment 156161 [details]
Patch

[Dirk Schulze]

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.

[Levi Weintraub]

Created attachment 156410 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 156410 [details]
Patch for landing

Clearing flags on attachment: 156410

Committed r124662: <http://trac.webkit.org/changeset/124662>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.