1305253 - In nsSVGIntegrationUtils::PaintMaskAndClipPath, manually pop up clip-path

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[u459114]

For now, we save the context before push clip-path, and restore that context after blending. 
Compare with gfxContext::Save/Restore, using gfxContext::PopClip to remove the pushed clip-paths is more efficient.

Comment 1

[u459114]

Attachment: Bug 1305253 - Part 2. Replace gfxContext::Save/Restore by gfxContext::PopClip.

Review commit: https://reviewboard.mozilla.org/r/89260/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89260/

Comment 2

[u459114]

Attachment: Bug 1305253 - Part 1. Use nsCSSClipPathInstance::ApplyBasicShapeClip when clip-path type is basic-shape or geometry-box.

shouldApplyBasicShape will be set as true when the url of a clip-path is not
valid, for example:
  clip-path: url("#non-exist-id");

So, we calls ApplyBasicShapeClip and early returns in that function even if
the clip-path's type is StyleShapeSourceType::URL.

Review commit: https://reviewboard.mozilla.org/r/89332/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89332/

Comment 3

[u459114]

Comment on attachment 8805509 [details]
Bug 1305253 - Part 2. Replace gfxContext::Save/Restore by gfxContext::PopClip.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89260/diff/1-2/

Comment 4

[u459114]

Comment on attachment 8805567 [details]
Bug 1305253 - Part 1. Use nsCSSClipPathInstance::ApplyBasicShapeClip when clip-path type is basic-shape or geometry-box.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89332/diff/1-2/

Comment 5

[u459114]

Comment on attachment 8805509 [details]
Bug 1305253 - Part 2. Replace gfxContext::Save/Restore by gfxContext::PopClip.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89260/diff/2-3/

Comment 6

[u459114]

Comment on attachment 8805567 [details]
Bug 1305253 - Part 1. Use nsCSSClipPathInstance::ApplyBasicShapeClip when clip-path type is basic-shape or geometry-box.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89332/diff/2-3/

Comment 7

[u459114]

Comment on attachment 8805509 [details]
Bug 1305253 - Part 2. Replace gfxContext::Save/Restore by gfxContext::PopClip.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89260/diff/3-4/

Comment 8

[Markus Stange [:mstange]]

Comment on attachment 8805567 [details]
Bug 1305253 - Part 1. Use nsCSSClipPathInstance::ApplyBasicShapeClip when clip-path type is basic-shape or geometry-box.

https://reviewboard.mozilla.org/r/89332/#review88974

::: layout/svg/nsSVGIntegrationUtils.cpp:750
(Diff revision 3)
> +    case  StyleShapeSourceType::Shape:
> +    case  StyleShapeSourceType::Box:
> +      shouldApplyBasicShape = true;
> +      break;
> +    case  StyleShapeSourceType::None:

double space after case

Comment 9

[Markus Stange [:mstange]]

Comment on attachment 8805567 [details]
Bug 1305253 - Part 1. Use nsCSSClipPathInstance::ApplyBasicShapeClip when clip-path type is basic-shape or geometry-box.

https://reviewboard.mozilla.org/r/89332/#review88982

Comment 10

[Markus Stange [:mstange]]

Comment on attachment 8805509 [details]
Bug 1305253 - Part 2. Replace gfxContext::Save/Restore by gfxContext::PopClip.

https://reviewboard.mozilla.org/r/89260/#review88984

Comment 11

[Markus Stange [:mstange]]

(In reply to C.J. Ku[:cjku](UTC+8) from comment #2)
> Created attachment 8805567 [details]
> Bug 1305253 - Part 1. Use nsCSSClipPathInstance::ApplyBasicShapeClip when
> clip-path type is basic-shape or geometry-box.
> 
> shouldApplyBasicShape will be set as true when the url of a clip-path is not
> valid, for example:
>   clip-path: url("#non-exist-id");
> 
> So, we calls ApplyBasicShapeClip and early returns in that function even if
> the clip-path's type is StyleShapeSourceType::URL.

Did this cause wrong rendering?

Comment 12

[u459114]

(In reply to Markus Stange [:mstange] from comment #11)
> (In reply to C.J. Ku[:cjku](UTC+8) from comment #2)
> > Created attachment 8805567 [details]
> > Bug 1305253 - Part 1. Use nsCSSClipPathInstance::ApplyBasicShapeClip when
> > clip-path type is basic-shape or geometry-box.
> > 
> > shouldApplyBasicShape will be set as true when the url of a clip-path is not
> > valid, for example:
> >   clip-path: url("#non-exist-id");
> > 
> > So, we calls ApplyBasicShapeClip and early returns in that function even if
> > the clip-path's type is StyleShapeSourceType::URL.
> 
> Did this cause wrong rendering?
Actually not. Originally, we do need this early return, since we call the function(ApplyBasicShapeClip) with wrong clip-path type(StyleShapeSourceType::URL).

Comment 13

[u459114]

https://hg.mozilla.org/integration/mozilla-inbound/rev/be245d1bb68a300ed82e42e2042b72f7aa11f014
Bug 1305253 - Part 1. Use nsCSSClipPathInstance::ApplyBasicShapeClip when clip-path type is basic-shape or geometry-box. r=mstange

https://hg.mozilla.org/integration/mozilla-inbound/rev/adc65475e590daeab1e208077be561fc7a431e13
Bug 1305253 - Part 2. Replace gfxContext::Save/Restore by gfxContext::PopClip. r=mstange

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/be245d1bb68a
https://hg.mozilla.org/mozilla-central/rev/adc65475e590