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

Use primitive clip masks only, instead of sharing clip chain masks. #1960

Merged
merged 1 commit into from Oct 30, 2017

Conversation

@glennw
Copy link
Member

glennw commented Oct 30, 2017

We're not getting any noticeable benefit from trying to share clip
masks. In fact, we are often seeing very significant slowdowns
since we over-estimate the size needed for a shared clip mask.

This change drops the GPU time in #1817 from 40ms to 7ms on
my machine - this is an extreme case, but it's faster in every
site that I tested on locally.

This is also a good simplification for upcoming changes, where
we will be more aggressively removing clip masks when they
are redundant, and drawing them in small segments where possible.


This change is Reviewable

We're not getting any noticeable benefit from trying to share clip
masks. In fact, we are often seeing very significant slowdowns
since we over-estimate the size needed for a shared clip mask.

This change drops the GPU time in #1817 from 40ms to 7ms on
my machine - this is an extreme case, but it's faster in every
site that I tested on locally.

This is also a good simplification for upcoming changes, where
we will be more aggressively removing clip masks when they
are redundant, and drawing them in small segments where possible.
@glennw
Copy link
Member Author

glennw commented Oct 30, 2017

r? @kvark or @nical

@glennw
Copy link
Member Author

glennw commented Oct 30, 2017

@nical
Copy link
Collaborator

nical commented Oct 30, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2017

📌 Commit 9d6e728 has been approved by nical

@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2017

Testing commit 9d6e728 with merge e0f90d2...

bors-servo added a commit that referenced this pull request Oct 30, 2017
Use primitive clip masks only, instead of sharing clip chain masks.

We're not getting any noticeable benefit from trying to share clip
masks. In fact, we are often seeing very significant slowdowns
since we over-estimate the size needed for a shared clip mask.

This change drops the GPU time in #1817 from 40ms to 7ms on
my machine - this is an extreme case, but it's faster in every
site that I tested on locally.

This is also a good simplification for upcoming changes, where
we will be more aggressively removing clip masks when they
are redundant, and drawing them in small segments where possible.

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

bors-servo commented Oct 30, 2017

💔 Test failed - status-travis

@jrmuizel
Copy link
Contributor

jrmuizel commented Oct 30, 2017

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Oct 30, 2017

Testing commit 9d6e728 with merge ebe7046...

bors-servo added a commit that referenced this pull request Oct 30, 2017
Use primitive clip masks only, instead of sharing clip chain masks.

We're not getting any noticeable benefit from trying to share clip
masks. In fact, we are often seeing very significant slowdowns
since we over-estimate the size needed for a shared clip mask.

This change drops the GPU time in #1817 from 40ms to 7ms on
my machine - this is an extreme case, but it's faster in every
site that I tested on locally.

This is also a good simplification for upcoming changes, where
we will be more aggressively removing clip masks when they
are redundant, and drawing them in small segments where possible.

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

bors-servo commented Oct 30, 2017

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

@bors-servo bors-servo merged commit 9d6e728 into servo:master Oct 30, 2017
4 checks passed
4 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
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

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