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

Add a workaround for inner rects on transformed clips. #1820

Merged
merged 1 commit into from Oct 6, 2017

Conversation

@glennw
Copy link
Member

glennw commented Oct 5, 2017

Fixes #1814.


This change is Reviewable

@glennw
Copy link
Member Author

glennw commented Oct 5, 2017

(Still needs a try run before merge)

r? @kvark

@glennw
Copy link
Member Author

glennw commented Oct 6, 2017

Servo / wrench tests pass.

Gecko try looks ok, I think (two unexpected passes from the recent transform change, and two jobs that didn't start correct - re-running those now).

https://hg.mozilla.org/try/rev/0a457dc9cac0a58e0cf922b8a61e5849b4b67066
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a457dc9cac0a58e0cf922b8a61e5849b4b67066

@kvark
kvark approved these changes Oct 6, 2017
// Inner rects aren't valid if the item is not axis-aligned, which can
// be determined by the apply_rectangles field. This is mostly a band-aid
// until we have better handling of inner rectangles for transformed clips.
Some(ref inner) if !work_item.apply_rectangles && !inner.device_rect.is_empty() => {

This comment has been minimized.

@kvark

kvark Oct 6, 2017

Member

weird, I believe they should be valid

@kvark
Copy link
Member

kvark commented Oct 6, 2017

I'm worried that band-aiding and working around the malfunctioning parts of WR in order to suffice the reftests would make us forget to actually make it work properly. We need to either create new bugs to keep track of it, leave TODO comments (as a bare minimum), or not close WR-side bugs until proper solutions are in place (as opposed to workarounds), while still closing the corresponding Gecko issues.

@kvark
Copy link
Member

kvark commented Oct 6, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Oct 6, 2017

📌 Commit 75cd1f6 has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Oct 6, 2017

Testing commit 75cd1f6 with merge 4052fe4...

bors-servo added a commit that referenced this pull request Oct 6, 2017
Add a workaround for inner rects on transformed clips.

Fixes #1814.

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

bors-servo commented Oct 6, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: kvark
Pushing 4052fe4 to master...

@bors-servo bors-servo merged commit 75cd1f6 into servo:master Oct 6, 2017
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@mrobinson mrobinson mentioned this pull request Oct 11, 2017
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.