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

Implement nine patch support for border gradients #2848

Merged
merged 1 commit into from Jun 27, 2018

Conversation

@mrobinson
Copy link
Member

mrobinson commented Jun 26, 2018

This will allow border-gradients to support the full richness of the CSS
API. There are some areas where the implementation doesn't match what
CSS expects, but this should move us a good deal closer. We can
gradually fix those issues and turn this on in Gecko.

Fixes #1875.


This change is Reviewable

@mrobinson
Copy link
Member Author

mrobinson commented Jun 26, 2018

r? @gw3583

@mrobinson
Copy link
Member Author

mrobinson commented Jun 26, 2018

This doesn't allow us to turn on WebRender border-image in Gecko yet, but gets us a little closer. We would need to investigate a bit more into why so many reference tests still fail.

@gw3583
Copy link
Collaborator

gw3583 commented Jun 26, 2018

Reviewed 11 of 11 files at r1.
Review status: all files reviewed, 1 unresolved discussion (waiting on @mrobinson)


webrender_api/src/display_item.rs, line 276 at r1 (raw file):

#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
/// TODO(mrobinson): Currently only images are supported, but we will

We can remove this comment now.


Comments from Reviewable

@gw3583
Copy link
Collaborator

gw3583 commented Jun 26, 2018

One minor nit above, r=me otherwise. It also looks like there is no edge AA between gradient stops in the radial gradient image, but I think that's just a pre-existing issue with the radial gradient shader.

This will allow border-gradients to support the full richness of the CSS
API. There are some areas where the implementation doesn't match what
CSS expects, but this should move us a good deal closer. We can
gradually fix those issues and turn this on in Gecko.

Fixes #1875.
@mrobinson mrobinson force-pushed the mrobinson:nine-patch-gradient branch from 0e73f3f to 54e8c80 Jun 27, 2018
@mrobinson
Copy link
Member Author

mrobinson commented Jun 27, 2018

@bors-servo r=gw3583

@gw3583 Thanks for the review!

@bors-servo
Copy link
Contributor

bors-servo commented Jun 27, 2018

📌 Commit 54e8c80 has been approved by gw3583

@bors-servo
Copy link
Contributor

bors-servo commented Jun 27, 2018

Testing commit 54e8c80 with merge a185cfb...

bors-servo added a commit that referenced this pull request Jun 27, 2018
Implement nine patch support for border gradients

This will allow border-gradients to support the full richness of the CSS
API. There are some areas where the implementation doesn't match what
CSS expects, but this should move us a good deal closer. We can
gradually fix those issues and turn this on in Gecko.

Fixes #1875.

<!-- 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/2848)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jun 27, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: gw3583
Pushing a185cfb to master...

@bors-servo bors-servo merged commit 54e8c80 into servo:master Jun 27, 2018
3 of 4 checks passed
3 of 4 checks passed
code-review/reviewable 1 file, 1 discussion left (gw3583, mrobinson)
Details
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@mrobinson mrobinson deleted the mrobinson:nine-patch-gradient branch Jun 27, 2018
@mrobinson
Copy link
Member Author

mrobinson commented Jun 27, 2018

For the purposes of not breaking the build, here's a Gecko try job (should not change behavior at all) for this change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=76b9810cacc13df3dc227b73026f49775815317e&selectedJob=185117848

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

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