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

Jump to bottom

Use a more conservative approach for inner rect calculation #2205

Merged
merged 1 commit into from Dec 9, 2017
Merged

Use a more conservative approach for inner rect calculation #2205

merged 1 commit into from Dec 9, 2017

Conversation

mrobinson
Copy link
Member

@mrobinson mrobinson commented Dec 8, 2017 •
edited by larsbergstrom

This is a temporary workaround for a Gecko issue with the conservative
inner rect calculation for clips. In this case we just don't calculate
an inner rect when faced with a non-axis-aligned transformation.

This change is Reviewable

@mrobinson
Use a more conservative approach for inner rect calculation
d10bbdd 
This is a temporary workaround for a Gecko issue with the conservative
inner rect calculation for clips. In this case we just don't calculate
an inner rect when faced with a non-axis-aligned transformation.
@mrobinson mrobinson requested a review from kvark December 8, 2017 20:41
@mrobinson
Copy link
Member Author

Here's the Gecko try run for this change: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b370376404d4d6c001a5f92c9a227b8303ac213b

Once we confirm that the Talos test is passing, this should be okay to merge.

kvark
kvark approved these changes Dec 8, 2017
View reviewed changes
Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!
I assume this is exactly the code you used to run a try push with.

@mrobinson mrobinson mentioned this pull request Dec 8, 2017
Merged
@mrobinson
Copy link
Member Author

@kvark Unfortunately, it is not exactly the same, because Gecko is still using an older version of Euclid. This required me to use the original version of calculate_screen_bounding_rect for the try push. I can update this PR to include that as well.

@kvark
Copy link
Member

kvark commented Dec 8, 2017

@mrobinson nah, it's fine. We can wait for your new try push's Talos result.

@mrobinson
Copy link
Member Author

@kvark Okay. I'm fairly certain the changes you made to calculate_screen_bounding_rect did not change behavior.

@kvark
Copy link
Member

kvark commented Dec 8, 2017

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit d10bbdd has been approved by kvark

@bors-servo
Copy link
Contributor

⌛ Testing commit d10bbdd with merge 423432d...

bors-servo pushed a commit that referenced this pull request Dec 9, 2017
Auto merge of #2205 - mrobinson:inner-rect-more-conservative, r=kvark
423432d 
Use a more conservative approach for inner rect calculation

This is a temporary workaround for a Gecko issue with the conservative
inner rect calculation for clips. In this case we just don't calculate
an inner rect when faced with a non-axis-aligned transformation.

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

💔 Test failed - status-travis

@glennw
Copy link
Member

glennw commented Dec 9, 2017

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit d10bbdd with merge c7a6840...

bors-servo pushed a commit that referenced this pull request Dec 9, 2017
Auto merge of #2205 - mrobinson:inner-rect-more-conservative, r=kvark
c7a6840 
Use a more conservative approach for inner rect calculation

This is a temporary workaround for a Gecko issue with the conservative
inner rect calculation for clips. In this case we just don't calculate
an inner rect when faced with a non-axis-aligned transformation.

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

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

@bors-servo bors-servo merged commit d10bbdd into servo:master Dec 9, 2017
@mrobinson mrobinson deleted the inner-rect-more-conservative branch April 26, 2018 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvark kvark kvark approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mrobinson @kvark @bors-servo @glennw