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

Fix bug where brush mask kind changes during scrolling. #2973

Merged
merged 1 commit into from Aug 16, 2018

Conversation

@gw3583
Copy link
Collaborator

commented Aug 16, 2018

Calculate the brush clip mask kind during clip chain instance
building, instead of during segment building.

If this changes during a scroll of the same display list, detect
that and rerun the logic that assigns the clip mask kind to
each segment.

This doesn't actually re-segment, just updates the clip mask kind,
which is fine since the segments are based only on clips in the
same spatial node (so unable to change relative to each other
during scrolling).

This fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1483568.

Unfortunately we don't have enough scroll testing infrastructure
in wrench to add a regression test for this yet.


This change is Reviewable

Calculate the brush clip mask kind during clip chain instance
building, instead of during segment building.

If this changes during a scroll of the same display list, detect
that and rerun the logic that assigns the clip mask kind to
each segment.

This doesn't actually re-segment, just updates the clip mask kind,
which is fine since the segments are based only on clips in the
same spatial node (so unable to change relative to each other
during scrolling).

This fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1483568.

Unfortunately we don't have enough scroll testing infrastructure
in wrench to add a regression test for this yet.
@gw3583

This comment has been minimized.

@kvark
kvark approved these changes Aug 16, 2018
Copy link
Member

left a comment

:lgtm:

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@gw3583

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 16, 2018

Try run looks ok. 🚀

@bors-servo r=kvark

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2018

📌 Commit 081a4ef has been approved by kvark

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2018

⌛️ Testing commit 081a4ef with merge e33d0de...

bors-servo added a commit that referenced this pull request Aug 16, 2018
Fix bug where brush mask kind changes during scrolling.

Calculate the brush clip mask kind during clip chain instance
building, instead of during segment building.

If this changes during a scroll of the same display list, detect
that and rerun the logic that assigns the clip mask kind to
each segment.

This doesn't actually re-segment, just updates the clip mask kind,
which is fine since the segments are based only on clips in the
same spatial node (so unable to change relative to each other
during scrolling).

This fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1483568.

Unfortunately we don't have enough scroll testing infrastructure
in wrench to add a regression test for this yet.

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

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2018

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

@bors-servo bors-servo merged commit 081a4ef into servo:master Aug 16, 2018
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
code-review/reviewable 2 files reviewed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@gw3583 gw3583 deleted the gw3583:clip-fix branch Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.