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

No perspective interpolation for blend brushes #2425

Merged
merged 1 commit into from Feb 15, 2018

Conversation

@kvark
Copy link
Member

kvark commented Feb 15, 2018

A follow-up to #2422


This change is Reviewable

@glennw glennw self-requested a review Feb 15, 2018
@glennw
glennw approved these changes Feb 15, 2018
@glennw
Copy link
Member

glennw commented Feb 15, 2018

Needs a gecko try, and possibly a reference test image update, since there might be a couple of subpixel differences. Otherwise, looks great!

@glennw
Copy link
Member

glennw commented Feb 15, 2018

Paraphrasing IRC conversation - for later reference:

Once we support rasterizing pictures in screen or local space, what we'll want to do is:

  • Remove the FORCE_NO_PERSPECTIVE define.
  • Have brush.glsl select whether to force w=1 based on the raster mode of the picture in question.
@glennw
Copy link
Member

glennw commented Feb 15, 2018

Yup, very slight difference in one of the reftests which will need an updated reference image.

@kvark kvark force-pushed the kvark:force-no-perspective branch from b234876 to 3a17a77 Feb 15, 2018
@kvark
Copy link
Member Author

kvark commented Feb 15, 2018

@kvark
Copy link
Member Author

kvark commented Feb 15, 2018

@staktrace

builds/worker/workspace/build/tests/reftest/tests/dom/html/reftests/bug917595-pixel-rotated.jpg

Looks like an intermittent bug that is also affected by the way we interpolate things.
Is this the case where Gecko updates a reftest, or the PR needs to be fixed (I don't particularly see anything wrong with it, for the record)?

@staktrace
Copy link
Contributor

staktrace commented Feb 15, 2018

@kvark you can ignore that one, it was introduced by #2408 and I already have a fuzzing patch in my queue for it.

@kvark
Copy link
Member Author

kvark commented Feb 15, 2018

Thanks!
@bors-servo r=glennw

@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2018

📌 Commit 3a17a77 has been approved by glennw

@bors-servo
Copy link
Contributor

bors-servo commented Feb 15, 2018

Testing commit 3a17a77 with merge 114cd5e...

bors-servo added a commit that referenced this pull request Feb 15, 2018
No perspective interpolation for blend brushes

A follow-up to #2422

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

bors-servo commented Feb 15, 2018

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

@bors-servo bors-servo merged commit 3a17a77 into servo:master Feb 15, 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:force-no-perspective branch Feb 15, 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.