Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transformed complex clips should be nerfed #2700

Closed
Gankra opened this issue Apr 26, 2018 · 6 comments
Closed

transformed complex clips should be nerfed #2700

Gankra opened this issue Apr 26, 2018 · 6 comments

Comments

@Gankra
Copy link
Contributor

@Gankra Gankra commented Apr 26, 2018

The boundary of the clipped content can't be clipped by anything, escaping parent stacking contexts and even overlapping the chrome.

pls nerf

Here's a slightly scaled and rotated div with border-radius inside an overflow:hidden parent div:

clippy

As you can see, the edge of the inner div has escape the parent, clippable by no mortal.

I've reduced this to the following capture, but need to call it a day:

scene.yaml
(
    root_pipeline_id: Some((1, 12)),
    pipelines: {
        (1, 12): (
            pipeline_id: (1, 12),
            viewport_size: (3205, 1830),
            content_size: (3205, 1830),
            background_color: None,
            display_list: [
                (
                    item: PushStackingContext((
                        stacking_context: (
                            scroll_policy: Scrollable,
                            transform: None,
                            transform_style: Flat,
                            perspective: None,
                            mix_blend_mode: Normal,
                            reference_frame_id: None,
                            clip_node_id: None,
                            glyph_raster_space: Screen,
                        ),
                    ), [
                    ]),
                    clip_and_scroll: (
                        scroll_node_id: Clip(1, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (0, 0)),
                        clip_rect: ((0, 0), (0, 0)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [0]
                (
                    item: Rectangle((
                        color: (
                            r: 1,
                            g: 1,
                            b: 1,
                            a: 1,
                        ),
                    )),
                    clip_and_scroll: (
                        scroll_node_id: Clip(1, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (3205, 1830)),
                        clip_rect: ((0, 0), (3205, 1830)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [1]
                (
                    item: Rectangle((
                        color: (
                            r: 0,
                            g: 0,
                            b: 0,
                            a: 0,
                        ),
                    )),
                    clip_and_scroll: (
                        scroll_node_id: Clip(1, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (3205, 1830)),
                        clip_rect: ((0, 0), (3205, 1830)),
                        is_backface_visible: true,
                        tag: Some((0, 1)),
                    ),
                ),// [2]
                (
                    item: Clip((
                        id: Clip(2, (1, 12)),
                        image_mask: None,
                    ), [
                    ]),
                    clip_and_scroll: (
                        scroll_node_id: Clip(1, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (3205, 1830)),
                        clip_rect: ((0, 0), (3205, 1830)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [3]
                (
                    item: ScrollFrame((
                        clip_id: Clip(3, (1, 12)),
                        scroll_frame_id: Clip(4, (1, 12)),
                        external_id: Some((2, (1, 12))),
                        image_mask: None,
                        scroll_sensitivity: Script,
                    ), [
                    ]),
                    clip_and_scroll: (
                        scroll_node_id: Clip(2, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (3205, 1830)),
                        clip_rect: ((0, 0), (3205, 1830)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [4]
                (
                    item: Rectangle((
                        color: (
                            r: 0,
                            g: 0,
                            b: 0,
                            a: 0,
                        ),
                    )),
                    clip_and_scroll: (
                        scroll_node_id: Clip(4, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (3205, 1830)),
                        clip_rect: ((0, 0), (3205, 1830)),
                        is_backface_visible: true,
                        tag: Some((2, 1)),
                    ),
                ),// [5]
                (
                    item: Clip((
                        id: Clip(5, (1, 12)),
                        image_mask: None,
                    ), [
                    ]),
                    clip_and_scroll: (
                        scroll_node_id: Clip(4, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (3205, 1830)),
                        clip_rect: ((0, 0), (3205, 1830)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [6]
                (
                    item: Rectangle((
                        color: (
                            r: 0,
                            g: 0,
                            b: 0,
                            a: 0,
                        ),
                    )),
                    clip_and_scroll: (
                        scroll_node_id: Clip(5, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (3205, 1830)),
                        clip_rect: ((0, 0), (3205, 1830)),
                        is_backface_visible: true,
                        tag: Some((2, 1)),
                    ),
                ),// [7]
                (
                    item: Rectangle((
                        color: (
                            r: 1,
                            g: 1,
                            b: 1,
                            a: 1,
                        ),
                    )),
                    clip_and_scroll: (
                        scroll_node_id: Clip(5, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (3205, 1830)),
                        clip_rect: ((0, 0), (3205, 1830)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [8]
                (
                    item: Clip((
                        id: Clip(6, (1, 12)),
                        image_mask: None,
                    ), [
                    ]),
                    clip_and_scroll: (
                        scroll_node_id: Clip(4, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((20, 20), (2400, 900)),
                        clip_rect: ((20, 20), (2400, 900)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [9]
                (
                    item: PushStackingContext((
                        stacking_context: (
                            scroll_policy: Scrollable,
                            transform: Some(Value((1.0249937772750854, 0.003577917581424117, 0, 0, -0.003577917581424117, 1.0249937772750854, 0, 0, 0, 0, 1, 0, -28.3824462890625, -15.540679931640625, 0, 1))),
                            transform_style: Flat,
                            perspective: None,
                            mix_blend_mode: Normal,
                            reference_frame_id: Some(Clip(7, (1, 12))),
                            clip_node_id: None,
                            glyph_raster_space: Screen,
                        ),
                    ), [
                    ]),
                    clip_and_scroll: (
                        scroll_node_id: Clip(6, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((20, 20), (0, 0)),
                        clip_rect: ((20, 20), (0, 0)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [10]
                (
                    item: Rectangle((
                        color: (
                            r: 0,
                            g: 0,
                            b: 0,
                            a: 0,
                        ),
                    )),
                    clip_and_scroll: (
                        scroll_node_id: Clip(6, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (2400, 900)),
                        clip_rect: ((0, 0), (2400, 900)),
                        is_backface_visible: true,
                        tag: Some((2, 1)),
                    ),
                ),// [11]
                (
                    item: Clip((
                        id: Clip(8, (1, 12)),
                        image_mask: None,
                    ), [
                        (
                            rect: ((0, 0), (2400, 900)),
                            radii: (
                                top_left: (5, 5),
                                top_right: (5, 5),
                                bottom_left: (5, 5),
                                bottom_right: (5, 5),
                            ),
                            mode: Clip,
                        ),// [0]
                    ]),
                    clip_and_scroll: (
                        scroll_node_id: Clip(6, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (2400, 900)),
                        clip_rect: ((0, 0), (2400, 900)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [12]
                (
                    item: Rectangle((
                        color: (
                            r: 0,
                            g: 0,
                            b: 0,
                            a: 1,
                        ),
                    )),
                    clip_and_scroll: (
                        scroll_node_id: Clip(8, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (2400, 900)),
                        clip_rect: ((0, 0), (2400, 900)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [13]
                (
                    item: PopStackingContext,
                    clip_and_scroll: (
                        scroll_node_id: Clip(6, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (0, 0)),
                        clip_rect: ((0, 0), (0, 0)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [14]
                (
                    item: PopStackingContext,
                    clip_and_scroll: (
                        scroll_node_id: Clip(1, (1, 12)),
                        clip_node_id: None,
                    ),
                    info: (
                        rect: ((0, 0), (0, 0)),
                        clip_rect: ((0, 0), (0, 0)),
                        is_backface_visible: true,
                        tag: None,
                    ),
                ),// [15]
            ],
        ),
    },
    pipeline_epochs: {
        (1, 12): (3),
    },
)
@Gankra
Copy link
Contributor Author

@Gankra Gankra commented Apr 26, 2018

bugzilled: https://bugzilla.mozilla.org/show_bug.cgi?id=1455839

(contains the html version of this testcase)

@glennw
Copy link
Member

@glennw glennw commented Apr 26, 2018

@mrobinson Have you got cycles to take a look at this one?

@mrobinson
Copy link
Member

@mrobinson mrobinson commented Apr 27, 2018

@glennw Sure. I'm happy to take a look at this one.

@gankro Can you comment on what I should expect from the output of this test case? Changing the size of the scroll frame seems to clip the interior black rectangle just fine, but perhaps I'm missing something...

@Gankra
Copy link
Contributor Author

@Gankra Gankra commented Apr 27, 2018

@Gankra
Copy link
Contributor Author

@Gankra Gankra commented Apr 27, 2018

tl;dr: you should just see a solid black rect.

@mrobinson
Copy link
Member

@mrobinson mrobinson commented Apr 30, 2018

Here's some wrench yaml showing a reduced test case

---
root:
  items:
    - type: clip
      bounds: [100, 100, 100, 100]
      id: 2
    -
      type: stacking-context
      clip-and-scroll: 2
      transform: rotate(0.25)
      items:
       - type: clip
         bounds: [0, 0, 2400, 900]
         id: 3
         complex:
           - rect: [ 0, 0, 2400, 900 ]
             radius: 50
       - type: rect
         color: [0, 255, 0, 1]
         bounds: [0, 0, 2400, 900]
         clip-and-scroll: 3

There should be a single green 100x100 rectangle at 100x100. Instead an area the size of the radius of the interior clip shows up surrounding the 100x100 rectangle. This seems to be a bad interaction between the rotation and a clip with a radius. The rotation suggests that it's an issue that involves complex clips that span coordinate systems.

mrobinson added a commit to mrobinson/webrender that referenced this issue Apr 30, 2018
When clip segments were optimized away due to having no intersection
with the outer screen bounds, they were treated as opaque regions. This
is due to the fact that an Option is not expressive enough to represent
the three different kinds of distinct destinies of clip segments.

This change replaces the Option with an enum that is either a
RenderTaskId, Opaque, or Empty to represent a clip segment that should
not be rendered.

Fixes servo#2700.
mrobinson added a commit to mrobinson/webrender that referenced this issue Apr 30, 2018
When clip segments were optimized away due to having no intersection
with the outer screen bounds, they were treated as opaque regions. This
is due to the fact that an Option is not expressive enough to represent
the three different kinds of distinct destinies of clip segments.

This change replaces the Option with an enum that is either a
RenderTaskId, Opaque, or Empty to represent a clip segment that should
not be rendered.

Fixes servo#2700.
bors-servo added a commit that referenced this issue Apr 30, 2018
Fix issue with optimized away clip segments

When clip segments were optimized away due to having no intersection
with the outer screen bounds, they were treated as opaque regions. This
is due to the fact that an Option is not expressive enough to represent
the three different kinds of distinct destinies of clip segments.

This change replaces the Option with an enum that is either a
RenderTaskId, Opaque, or Empty to represent a clip segment that should
not be rendered.

Fixes #2700.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2707)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.