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

Box shadows with blur and spread increase the corner radius too much #2287

Closed
mstange opened this issue Jan 11, 2018 · 4 comments
Closed

Box shadows with blur and spread increase the corner radius too much #2287

mstange opened this issue Jan 11, 2018 · 4 comments
Assignees

Comments

@mstange
Copy link
Contributor

@mstange mstange commented Jan 11, 2018

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

Testcase

This shadow has a spread radius.

When the shadow has no blur, then the shadow's corner radius is correct. But if it does have a blur, then the corner radius is too large.

Expected vs actual results:

screen shot 2018-01-11 at 4 19 10 pm

---
root:
  items:
    - type: stacking-context
      bounds: [0, 0, 1000, 1000]
      items:
        - type: box-shadow
          bounds: [ 0, 0, 200, 200 ]
          color: blue
          clip-mode: outset
          offset: 250 50
          blur-radius: 1
          spread-radius: 40
          border-radius:
            top-left: [0, 0]
            top-right: [120, 120]
            bottom-left: [120, 120]
            bottom-right: [0, 0]
        - type: box-shadow
          bounds: [ 0, 0, 200, 200 ]
          color: blue
          clip-mode: outset
          offset: 50 250
          blur-radius: 0
          spread-radius: 40
          border-radius:
            top-left: [0, 0]
            top-right: [120, 120]
            bottom-left: [120, 120]
            bottom-right: [0, 0]
@mstange
Copy link
Contributor Author

@mstange mstange commented Jan 11, 2018

I discovered this on the box-shadow playground.

@nical nical self-assigned this Jan 12, 2018
@nical
Copy link
Collaborator

@nical nical commented Jan 15, 2018

What seems to be happening is that per spec we are adjusting the border radii of the rounded rect we are using for the shadow, but with a wrong spread value.
If I divide the spread value we use in adjust_radius_for_box_shadow by two, the result looks correct.
Or the radii are adjusted twice maybe.

@mstange
Copy link
Contributor Author

@mstange mstange commented Jan 15, 2018

That sounds like we're doing the adjustment twice instead of just once.

@nical
Copy link
Collaborator

@nical nical commented Jan 15, 2018

Looks like it's a tad more complicated than my hunch above. One thing that strikes me as odd is that the rounded rect that we send to the shader has the correct adjusted border radii, but unadjusted rectangle dimensions. Yet the final blurred rectangle gets drawn at the correct size later and I haven't seen where the dimensions are adjusted. In any case the rectangle that is blurred out has a mismatch between the actual size and the radii and I am pretty sure the problem is there.
Edit: I kinda see how that could be made to work with the nine-patch path, but in the case I am looking at we are not using nine-patches.

bors-servo added a commit that referenced this issue Jan 16, 2018
Fix incorrect border radius with outset shadows.

Fixes #2287.

The problem was that if the blur radius/corners are big enough we blur the entire rounded rectangle. In this case we need to make sure that the blurred rectangle takes the spread amount into account (while this is not necessary when using the nine-patch code path where the rectangular patches can be stretched).
This PR adjusts the size of the rounded rectangle primitive accordingly when it makes sense.

<!-- 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/2308)
<!-- Reviewable:end -->
@nical nical closed this Jan 17, 2018
bors-servo added a commit that referenced this issue Jan 18, 2018
Fix incorrect border radius with outset shadows.

Fixes #2287.

The problem was that if the blur radius/corners are big enough we blur the entire rounded rectangle. In this case we need to make sure that the blurred rectangle takes the spread amount into account (while this is not necessary when using the nine-patch code path where the rectangular patches can be stretched).
This PR adjusts the size of the rounded rectangle primitive accordingly when it makes sense.

<!-- 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/2308)
<!-- 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.