Bug 80413 – Add SVGRenderingContext and move there the context related code from SVGRenderSupport

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Zoltan Herczeg]

First step of refactoring this part.

[Zoltan Herczeg]

Created attachment 130368 [details]
patch

[Nikolas Zimmermann]

Comment on attachment 130368 [details]
patch

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

Looks great! r=me, assuming no regressions.

> Source/WebCore/rendering/svg/SVGRenderingContext.h:38
> +#if ENABLE(FILTERS)

No need to guard class forwards IIRC.

> Source/WebCore/rendering/svg/SVGRenderingContext.h:75
> +    bool isRenderingPrepared() { return m_renderingFlags & RenderingPrepared; }

Could be const.

> Source/WebCore/rendering/svg/SVGRenderingContext.h:79
> +    // we record all successful steps.

No need to wrap lines here, eh?

> Source/WebCore/rendering/svg/SVGRenderingContext.h:91
> +        | EndOpacityLayer | EndShadowLayer | EndFilterLayer;

Ditto.

[WebKit Review Bot]

Comment on attachment 130368 [details]
patch

Attachment 130368 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11839392

New failing tests:
svg/filters/subRegion-in-userSpace.svg
svg/dynamic-updates/SVGFEColorMatrixElement-svgdom-values-prop.html
svg/W3C-SVG-1.1/filters-tile-01-b.svg
svg/dynamic-updates/SVGFEBlendElement-dom-mode-attr.html
svg/dynamic-updates/SVGFEColorMatrixElement-dom-type-attr.html
svg/dynamic-updates/SVGFEBlendElement-svgdom-mode-prop.html

[Zoltan Herczeg]

I will check the failing tests tomorrow.

[Zoltan Herczeg]

Created attachment 130816 [details]
fixing EWS failures

Let's wait for the EWS.

[WebKit Review Bot]

Comment on attachment 130816 [details]
fixing EWS failures

Attachment 130816 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11911471

New failing tests:
svg/dynamic-updates/SVGFECompositeElement-svgdom-k2-prop.html
svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-bias-attr.html
svg/dynamic-updates/SVGFECompositeElement-svgdom-k1-prop.html
svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-divisor-attr.html
svg/dynamic-updates/SVGFEColorMatrixElement-dom-values-attr.html
http/tests/inspector/inspect-element.html
svg/dynamic-updates/SVGFECompositeElement-dom-k3-attr.html
accessibility/aria-disabled.html
fast/loader/text-document-wrapping.html
svg/dynamic-updates/SVGFECompositeElement-svgdom-operator-prop.html
svg/dynamic-updates/SVGFECompositeElement-dom-operator-attr.html
svg/dynamic-updates/SVGFECompositeElement-dom-k1-attr.html
svg/dynamic-updates/SVGFEBlendElement-dom-mode-attr.html
compositing/reflections/reflection-ordering.html
svg/dynamic-updates/SVGFEConvolveMatrixElement-dom-edgeMode-attr.html
svg/dynamic-updates/SVGFECompositeElement-svgdom-k3-prop.html
svg/dynamic-updates/SVGFEColorMatrixElement-dom-type-attr.html
svg/dynamic-updates/SVGFECompositeElement-dom-k2-attr.html
http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect.html
svg/dynamic-updates/SVGFEColorMatrixElement-svgdom-values-prop.html
svg/dynamic-updates/SVGFEColorMatrixElement-svgdom-type-prop.html
compositing/masks/direct-image-mask.html
svg/dynamic-updates/SVGFECompositeElement-dom-k4-attr.html
css3/filters/effect-contrast-hw.html
svg/dynamic-updates/SVGFECompositeElement-svgdom-k4-prop.html
svg/dynamic-updates/SVGFEBlendElement-svgdom-mode-prop.html

[Nikolas Zimmermann]

(In reply to comment #6)
Your patch looks nice, can you confirm the EWS results are correct, and stuff got broken?

[Zoltan Herczeg]

Created attachment 131020 [details]
another attempt

[Zoltan Herczeg]

> Your patch looks nice, can you confirm the EWS results are correct, and stuff got broken?

Yeah. Still the return value of filter initialization. I misunderstood it. False means you don't need to draw anyithing, not initialization failed. Rather confusing :(

[Nikolas Zimmermann]

Comment on attachment 131020 [details]
another attempt

The cr-linux failures now look unrelated, r=me. The previously failing tests are all gone, so we think this is safe.

[Zoltan Herczeg]

Landed as http://trac.webkit.org/changeset/110285

[Patrick R. Gansterer]

Landed build fix in http://trac.webkit.org/changeset/110319