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

Check preserve-3d when computing stacking-context backface-visibility #3040

Merged
merged 2 commits into from Sep 12, 2018

Conversation

Projects
None yet
3 participants
@Gankro
Contributor

Gankro commented Sep 10, 2018

this appears to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1489937

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=386d8adb469cb458af4b5359de63c9a067345e22

(that some other should-be-fine commits snuck onto)


This change is Reviewable

@Gankro

This comment has been minimized.

Show comment
Hide comment
@Gankro

Gankro Sep 11, 2018

Contributor

New impl, new try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=454e91fbffc1f642362eab3df8edc5136dd2cf61

Writing a test now (since nothing in gecko's suite is robust enough to catch this issue, sadly)

Contributor

Gankro commented Sep 11, 2018

New impl, new try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=454e91fbffc1f642362eab3df8edc5136dd2cf61

Writing a test now (since nothing in gecko's suite is robust enough to catch this issue, sadly)

Show outdated Hide outdated webrender/src/display_list_flattener.rs
Show outdated Hide outdated webrender/src/display_list_flattener.rs
// when doing backface-visibility checking, so we need to grab the backface-visibility
// of the lowest ancestor which *doesn't* preserve-3d, and AND it in with ours.
//
// No this isn't obvious or clear, it's just what we worked out over a day of testing.

This comment has been minimized.

@kvark

kvark Sep 11, 2018

Member

I'm sorry, I don't understand why we are looking for the nearest Flat ancestor. Is there an actual reason backing this logic, or just empirical evidence?

@kvark

kvark Sep 11, 2018

Member

I'm sorry, I don't understand why we are looking for the nearest Flat ancestor. Is there an actual reason backing this logic, or just empirical evidence?

Show outdated Hide outdated webrender/src/display_list_flattener.rs
@kvark

kvark approved these changes Sep 11, 2018

Show outdated Hide outdated webrender/src/display_list_flattener.rs
@kvark

kvark approved these changes Sep 12, 2018

@Gankro

This comment has been minimized.

Show comment
Hide comment
@Gankro

Gankro Sep 12, 2018

Contributor

@bors-servo r=kvark

Contributor

Gankro commented Sep 12, 2018

@bors-servo r=kvark

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Sep 12, 2018

Contributor

📌 Commit 0f59017 has been approved by kvark

Contributor

bors-servo commented Sep 12, 2018

📌 Commit 0f59017 has been approved by kvark

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Sep 12, 2018

Contributor

⌛️ Testing commit 0f59017 with merge 9aafedc...

Contributor

bors-servo commented Sep 12, 2018

⌛️ Testing commit 0f59017 with merge 9aafedc...

bors-servo added a commit that referenced this pull request Sep 12, 2018

Auto merge of #3040 - Gankro:backface, r=kvark
Check preserve-3d when computing stacking-context backface-visibility

this appears to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1489937

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=386d8adb469cb458af4b5359de63c9a067345e22

(that some other should-be-fine commits snuck onto)

<!-- 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/3040)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Sep 12, 2018

Contributor

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: kvark
Pushing 9aafedc to master...

Contributor

bors-servo commented Sep 12, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: kvark
Pushing 9aafedc to master...

@bors-servo bors-servo merged commit 0f59017 into servo:master Sep 12, 2018

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment