1388985 - css clip-path with basic-shape rendering should respect box-decoration-break

Status: NEW

(Core :: Web Painting, enhancement, P2)

Description

[Louis Chang [:louis]]

Attachment: testcase: box-decoration-break-02.html

+++ This bug was initially created as a clone of Bug #1319406 +++

According to CSS Fragmentation Module Level 3:
https://drafts.csswg.org/css-break/#break-decoration

When a break (page/column/region/line) splits a box, the box-decoration-break property controls
* how the background positioning area [CSS3BG] (and mask positioning area [CSS-MASKING-1], shape reference box [CSS-SHAPES-1], etc.) is derived from or duplicated across the box fragments and how the element’s background is drawn within them. 

Depend on the value of box-decoration-break, we should generate the shape of clip-path differently. Currently, both slice and clone only clip the first continuation frame.

Comment 1

[u459114]

We only apply basic-shape onto the first continuation. Do you check the behavior of chrome or safari?

Comment 2

[Louis Chang [:louis]]

(In reply to C.J. Ku[:cjku](UTC+8) from comment #1)
> We only apply basic-shape onto the first continuation. Do you check the
> behavior of chrome or safari?

Chrome clip only first frame as well. Safari doesn't clip anything.

Comment 3

[Louis Chang [:louis]]

Attachment: Bug 1388985 Part 1 - Compute box for clip-path: basic shape based on first frame according to the value of box-decoration-break.

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

Comment 4

[Louis Chang [:louis]]

Attachment: Bug Part 1 - Add BBoxFlags::eIncludeOnlyCurrentFrameForNonSVGElement to determine whether include all continuations while computing bbox of a html frame.

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

Comment 5

[Louis Chang [:louis]]

Comment on attachment 8896098 [details]
Bug 1388985 Part 1 - Compute box for clip-path: basic shape based on first frame according to the value of box-decoration-break.

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

Comment 6

[Louis Chang [:louis]]

Comment on attachment 8896098 [details]
Bug 1388985 Part 1 - Compute box for clip-path: basic shape based on first frame according to the value of box-decoration-break.

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

Comment 7

[Louis Chang [:louis]]

Comment on attachment 8896098 [details]
Bug 1388985 Part 1 - Compute box for clip-path: basic shape based on first frame according to the value of box-decoration-break.

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

Comment 8

[u459114]

Comment on attachment 8896098 [details]
Bug 1388985 Part 1 - Compute box for clip-path: basic shape based on first frame according to the value of box-decoration-break.

https://reviewboard.mozilla.org/r/167380/#review172700

::: layout/svg/nsCSSClipPathInstance.cpp:74
(Diff revision 4)
>    return path->ContainsPoint(ToPoint(aPoint) * pixelRatio, Matrix());
>  }
>  
>  already_AddRefed<Path>
> -nsCSSClipPathInstance::CreateClipPath(DrawTarget* aDrawTarget)
> +nsCSSClipPathInstance::CreateClipPath(DrawTarget* aDrawTarget, nsIFrame* aFrame)
>  {

What's the difference between aFrame and nsCSSClipPathInstance::mFrame?

Comment 9

[u459114]

Comment on attachment 8896098 [details]
Bug 1388985 Part 1 - Compute box for clip-path: basic shape based on first frame according to the value of box-decoration-break.

https://reviewboard.mozilla.org/r/167380/#review172702

::: layout/svg/nsCSSClipPathInstance.cpp:80
(Diff revision 4)
> -  nsRect r =
> -    nsLayoutUtils::ComputeGeometryBox(mTargetFrame,
> -                                      mClipPathStyle.GetReferenceBox());
> +  uint32_t flags =
> +    (aFrame->StyleBorder()->mBoxDecorationBreak == StyleBoxDecorationBreak::Clone)
> +      ? nsSVGUtils::eBBoxIncludeFillGeometry |
> +        nsSVGUtils::eIncludeOnlyCurrentFrameForNonSVGElement
> +      : nsSVGUtils::eBBoxIncludeFillGeometry;
> +  gfxRect bbox = nsSVGUtils::GetBBox(aFrame, flags);

You should not use nsSVGUtils::GetBBox directly here since it does not handle clip-path: reference-box

Comment 10

[Louis Chang [:louis]]

Comment on attachment 8896098 [details]
Bug 1388985 Part 1 - Compute box for clip-path: basic shape based on first frame according to the value of box-decoration-break.

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

Comment 11

[u459114]

Comment on attachment 8896098 [details]
Bug 1388985 Part 1 - Compute box for clip-path: basic shape based on first frame according to the value of box-decoration-break.

https://reviewboard.mozilla.org/r/167380/#review174790

::: layout/svg/nsCSSClipPathInstance.cpp:83
(Diff revision 5)
>  
> +  // If mTargetFrame is a continuation frame, compute the box based on
> +  // first frame according to different values of box-decoration-break.
> +  nsIFrame* firstFrame =
> +    nsLayoutUtils::FirstContinuationOrIBSplitSibling(mTargetFrame);
> +  if (firstFrame != mTargetFrame) {

what if "firstFrame == mTargetFrame" and "mTargetFrame->StyleBorder()->mBoxDecorationBreak == slice"? We still need to union bbox of all continuations, right?

::: layout/svg/nsCSSClipPathInstance.cpp:93
(Diff revision 5)
> +      r.MoveTo(r.TopLeft() + offsetToFirstFrame);
> +    } else {
> +      nsRect firstComputedRect =
> +        nsLayoutUtils::ComputeGeometryBox(firstFrame,
> +                                          mClipPathStyle.GetReferenceBox());
> +      gfxRect bbox = nsSVGUtils::GetBBox(mTargetFrame);

What if you have tree, or above, continuations?

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:miko, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 13

[Miko Mynttinen]

Attachment: ss.png

The clipping is slightly different compared to Chrome.