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

Fixed edge mask decoding in the shader #2254

Merged
merged 2 commits into from Jan 7, 2018
Merged

Fixed edge mask decoding in the shader #2254

merged 2 commits into from Jan 7, 2018

Conversation

@kvark
Copy link
Member

kvark commented Jan 3, 2018

Related to #1797

Apparently, the old GLSL code that I had was all-or-nothing. For some reason, it didn't show the issue before, but it started showing it when the new brush splitting has arrived.


This change is Reviewable

@kvark kvark force-pushed the kvark:rotate branch from 0988f12 to 80a7a6f Jan 3, 2018
@kvark
Copy link
Member Author

kvark commented Jan 3, 2018

There was already a reftest for this (rotated-clip) but it was so small in size that it went under the segment split threshold and rendered in one go, hence it didn't reveal the problem. This PR includes an updated reftest.

@kvark kvark requested a review from glennw Jan 3, 2018
@glennw
glennw approved these changes Jan 3, 2018
Copy link
Member

glennw left a comment

The change looks good, thanks!

The reftest image is cropped through - I guess it should probably be enlarged to include the entire primitive? Would it also make sense to have the reftest include both a small and large primitive, to test both cases?

We should probably do a try run for this too, I guess.

@glennw
Copy link
Member

glennw commented Jan 3, 2018

Oh, CI is failing on TC too - might be a slight discrepancy in the reftest output.

@Gankra
Copy link
Contributor

Gankra commented Jan 4, 2018

Could this also be the cause/fix for #2252 ?

@kvark
Copy link
Member Author

kvark commented Jan 4, 2018

@glennw agreed, was also thinking about tests with different sizes.
@gankro the fix affects AA on edges of brush primitives, mostly segments. Not sure if it's related to #2252

@kvark kvark force-pushed the kvark:rotate branch from 80a7a6f to 34b3eb0 Jan 5, 2018
@kvark
Copy link
Member Author

kvark commented Jan 5, 2018

Try looks green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=68770f9175f0c8e963521ab0a775544564898f5c , if you don't mind 4 unexpected passes.

The new changes from border.rs are not new function but rather replicating the exact behavior of the old code for borders. Certainly something to put on the radar for future work on borders.

@bors-servo r=glennw

@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2018

📌 Commit 34b3eb0 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2018

Testing commit 34b3eb0 with merge 2b168e1...

bors-servo added a commit that referenced this pull request Jan 5, 2018
Fixed edge mask decoding in the shader

Related to #1797

Apparently, the old GLSL code that I had was all-or-nothing. For some reason, it didn't show the issue before, but it started showing it when the new brush splitting has arrived.

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

bors-servo commented Jan 5, 2018

💔 Test failed - status-travis

@kvark
Copy link
Member Author

kvark commented Jan 5, 2018

old good OSX timeout...
@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jan 5, 2018

Testing commit 34b3eb0 with merge 2bc88e4...

bors-servo added a commit that referenced this pull request Jan 5, 2018
Fixed edge mask decoding in the shader

Related to #1797

Apparently, the old GLSL code that I had was all-or-nothing. For some reason, it didn't show the issue before, but it started showing it when the new brush splitting has arrived.

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

bors-servo commented Jan 5, 2018

💔 Test failed - status-travis

@kvark
Copy link
Member Author

kvark commented Jan 5, 2018

ugh, all sorts of Travis failures... to check out the source, to wait for the completion, etc.

@kvark kvark mentioned this pull request Jan 5, 2018
0 of 3 tasks complete
@glennw
Copy link
Member

glennw commented Jan 7, 2018

@bors-servo retry

bors-servo added a commit that referenced this pull request Jan 7, 2018
Fixed edge mask decoding in the shader

Related to #1797

Apparently, the old GLSL code that I had was all-or-nothing. For some reason, it didn't show the issue before, but it started showing it when the new brush splitting has arrived.

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

bors-servo commented Jan 7, 2018

Testing commit 34b3eb0 with merge fa7d137...

@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: glennw
Pushing fa7d137 to master...

@bors-servo bors-servo merged commit 34b3eb0 into servo:master Jan 7, 2018
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@kvark kvark deleted the kvark:rotate branch Jan 8, 2018
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.

None yet

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