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

Push an empty hit testing clip node for empty ClipNodes #2899

Merged
merged 1 commit into from Jul 16, 2018

Conversation

@mrobinson
Copy link
Member

mrobinson commented Jul 16, 2018

Some Gecko display lists produce out-of-sequence ClipIds for ClipNodes
(#2898) which leads to empty ClipNodes. Previously the HitTester was
just discarding those nodes. This is an obvious problem because it means
that subsequent ClipNodeIndices are out-of-sync with the HitTester's
list of clipping nodes. This change fixes this by also adding empty
clipping nodes in the HitTester.

Unfortunately, we can't make a test for this until we figure out how and
why Gecko is producing display lists with out-of-sequence ClipIds.
Hopefully once we do that, we will no longer need to create empty
HitTester nodes at all.


This change is Reviewable

Some Gecko display lists produce out-of-sequence ClipIds for ClipNodes
(#2898) which leads to empty ClipNodes. Previously the HitTester was
just discarding those nodes. This is an obvious problem because it means
that subsequent ClipNodeIndices are out-of-sync with the HitTester's
list of clipping nodes. This change fixes this by also adding empty
clipping nodes in the HitTester.

Unfortunately, we can't make a test for this until we figure out how and
why Gecko is producing display lists with out-of-sequence ClipIds.
Hopefully once we do that, we will no longer need to create empty
HitTester nodes at all.
@kvark
Copy link
Member

kvark commented Jul 16, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 16, 2018

📌 Commit d1f031a has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Jul 16, 2018

Testing commit d1f031a with merge 526be26...

bors-servo added a commit that referenced this pull request Jul 16, 2018
…vark

Push an empty hit testing clip node for empty ClipNodes

Some Gecko display lists produce out-of-sequence ClipIds for ClipNodes
(#2898) which leads to empty ClipNodes. Previously the HitTester was
just discarding those nodes. This is an obvious problem because it means
that subsequent ClipNodeIndices are out-of-sync with the HitTester's
list of clipping nodes. This change fixes this by also adding empty
clipping nodes in the HitTester.

Unfortunately, we can't make a test for this until we figure out how and
why Gecko is producing display lists with out-of-sequence ClipIds.
Hopefully once we do that, we will no longer need to create empty
HitTester nodes at all.

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

bors-servo commented Jul 16, 2018

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

@bors-servo bors-servo merged commit d1f031a into servo:master Jul 16, 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
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

3 participants
You can’t perform that action at this time.