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

regression: Specific wikipedia page: Semi-donut chart shows lines on the bottom half where there shouldn't be #2294

Closed
staktrace opened this issue Jan 12, 2018 · 3 comments
Assignees

Comments

@staktrace
Copy link
Contributor

@staktrace staktrace commented Jan 12, 2018

Filing for the "first regression" reported at https://bugzilla.mozilla.org/show_bug.cgi?id=1430063 where the chart goes from looking correct to having streaky lines in the bottom half.

Narrowed down the WR regression range to this:

* c16b37dd Auto merge of #2228 - glennw:overlap-border, r=kvark
* e3d7353a Fix overlapping border radii handling.

So definitely from #2228.

@glennw
Copy link
Member

@glennw glennw commented Jan 18, 2018

I did some investigation of this - that bisect is not quite correct. It occurred in #2242 which merged just after that.

I can see what the problem is, but I'm not sure of the correct fix. @mrobinson could you take a look at this?

A reduced test case:

---
root:
  items:
      -
        type: "stacking-context"
        items:
          -
            bounds: [1488, 111, 356, 197]
            "clip-rect": [1488, 111, 356, 197]
            type: clip
            id: 5
            "content-size": [356, 197]
          -
            "clip-and-scroll": 5
            type: "stacking-context"
            transform: [0.98883086, 0.14904226, 0, 0, -0.14904226, 0.98883086, 0, 0, 0, 0, 1, 0, 1533.3134, 109.21605, 0, 1]
            items:
              -
                "clip-and-scroll": 5
                type: "stacking-context"
                transform: [0.7818315, 0.6234898, 0, 0, -0.6234898, 0.7818315, 0, 0, 0, 0, 1, 0, 132.98201, -64.04077, 0, 1]
                items:
                  -
                    "clip-and-scroll": 5
                    type: "stacking-context"
                    transform: [0.93087375, 0.36534107, 0, 0, -0.36534107, 0.93087375, 0, 0, 0, 0, 1, 0, 68.64584, -46.80194, 0, 1]
                    items:
                      -
                        "clip-and-scroll": 5
                        type: "stacking-context"
                        transform: [0.8262389, 0.56332004, 0, 0, -0.56332004, 0.8262389, 0, 0, 0, 0, 1, 0, 116.458824, -61.550323, 0, 1]
                        items:
                          -
                            "clip-and-scroll": 5
                            type: "stacking-context"
                            transform: [0.90096885, 0.43388373, 0, 0, -0.43388373, 0.90096885, 0, 0, 0, 0, 1, 0, 84.200554, -52.906708, 0, 1]
                            items:
                              -
                                "clip-and-scroll": 5
                                type: "stacking-context"
                                transform: [0.98883086, 0.14904226, 0, 0, -0.14904226, 0.98883086, 0, 0, 0, 0, 1, 0, 25.3134, -21.78395, 0, 1]
                                items:
                                  -
                                    "clip-and-scroll": 5
                                    type: "stacking-context"
                                    transform: [0.73305184, 0.68017274, 0, 0, -0.68017274, 0.73305184, 0, 0, 0, 0, 1, 0, 149.64511, -65.28949, 0, 1]
                                    items:
                                      -
                                        bounds: [0, 0, 316, 158]
                                        "clip-rect": [0, 0, 316, 158]
                                        "clip-and-scroll": 5
                                        type: rect
                                        color: red

After that PR, this displays as a fairly large rectangle. Before that PR, this will correctly have a clip mask applied and show as a very thin rectangle.

With current master it doesn't get any clip mask applied at all. What happens is that the code here

if node.screen_inner_rect.contains_rect(&combined_outer_rect) {
drops the clip source in question. It says that this clip source is irrelevant, because the clip chain outer result intersected with the prim rect is completely inside the inner rect of that clip.

But since they are incompatible coordinate systems, there's no local clip rect that applies in the vertex shader, and since this clip gets filtered out, there is no clip mask generated either.

Thoughts @mrobinson ?

mrobinson added a commit to mrobinson/webrender that referenced this issue Jan 18, 2018
When all clips are optimized away,  we may still need a clip mask when
some of those clips were in a different coordinate system. This change
ensures that this information is used to properly apply global clip
masks for segmented primitives with this kind of clipping situation.

Fixes servo#2294.
@staktrace
Copy link
Contributor Author

@staktrace staktrace commented Jan 18, 2018

I did some investigation of this - that bisect is not quite correct. It occurred in #2242 which
merged just after that.

It depends on the STR you use - there are two changes in behavior reported in bug 1430063 comment 1. I filed this issue and #2293 for them. #2242 is in the regression range for #2293, so that might be what you're seeing. That being said maybe I shouldn't have filed two issues since that's probably just confusing the matter further.

@glennw
Copy link
Member

@glennw glennw commented Jan 19, 2018

Fixed by #2324.

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
4 participants
You can’t perform that action at this time.