Skip to content

/ webrender

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

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 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
Loading status checks…
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 Dec 8, 2017
@mrobinson
Copy link
Member Author

mrobinson commented Dec 8, 2017

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 changes
Copy link
Member

kvark left a comment

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

mrobinson commented Dec 8, 2017

@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

mrobinson commented Dec 8, 2017

@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

bors-servo commented Dec 8, 2017

📌 Commit d10bbdd has been approved by kvark
@bors-servo
Copy link
Contributor

bors-servo commented Dec 9, 2017

⌛ Testing commit d10bbdd with merge 423432d...
bors-servo added a commit that referenced this pull request Dec 9, 2017
@bors-servo
Auto merge of #2205 - mrobinson:inner-rect-more-conservative, r=kvark
Loading status checks…
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

bors-servo commented Dec 9, 2017

💔 Test failed - status-travis
@glennw
Copy link
Member

glennw commented Dec 9, 2017

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Dec 9, 2017

⌛ Testing commit d10bbdd with merge c7a6840...
bors-servo added a commit that referenced this pull request Dec 9, 2017
@bors-servo
Auto merge of #2205 - mrobinson:inner-rect-more-conservative, r=kvark
Loading status checks…
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

bors-servo commented Dec 9, 2017

☀️ 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
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@mrobinson mrobinson deleted the mrobinson:inner-rect-more-conservative branch Apr 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvark kvark

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@mrobinson @kvark @bors-servo @glennw
You can’t perform that action at this time.