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

Better ClipChain optimization #2104

Merged
merged 1 commit into from Nov 28, 2017
Merged

Conversation

mrobinson
Copy link
Member

@mrobinson mrobinson commented Nov 27, 2017

Better ClipChain optimization

There are a lot of situations where we can omit redundant clips or
combine a series of clips into a single empty mask (which has the
effect of clipping the primitive in screen space). Briefly, some of the
optimizations this change includes:

  1. When the inner rect of a clip completely encloses the outer rect
    of a parent, we can ignore it.
  2. When the outer rectangle of a clip is completely enclosed by the
    inner rectangle of a parent, we can start a new ClipChan and
    ignore the parents.
  3. If the combined inner rectangle of a ClipChain completely
    encloses the intersection of a primitive and the current clips,
    we can stop applying clips from that chain.
  4. If a clip's inner rect completely encloses the intersection
    of a primitive and the current clips, we can ignore it.

This change is Reviewable

There are a lot of situations where we can omit redundant clips or
combine a series of clips into a single empty mask (which has the
effect of clipping the primitive in screen space). Briefly, some of the
optimizations this change includes:

1. When the inner rect of a clip completely encloses the outer rect
   of a parent, we can ignore it.
2. When the outer rectangle of a clip is completely enclosed by the
   inner rectangle of a parent, we can start a new ClipChan and
   ignore the parents.
3. If the combined inner rectangle of a ClipChain completely
   encloses the intersection of a primitive and the current clips,
   we can stop applying clips from that chain.
4. If a clip's inner rect completely encloses the intersection
   of a primitive and the current clips, we can ignore it.
@mrobinson
Copy link
Member Author

This is one half of #2062.

@glennw
Copy link
Member

glennw commented Nov 27, 2017

Awesome! I'll look at this first thing in the morning if no-one else gets to it first. The sooner we can get this part merged the better :)

@glennw
Copy link
Member

glennw commented Nov 27, 2017

Reviewed 7 of 7 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@glennw
Copy link
Member

glennw commented Nov 27, 2017

Looks good to me!

I've done a try push that was green yesterday:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d3e515665df920695200feff390c9ec4a52984ac

I don't think this patch has changed since then.

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 71ffd50 has been approved by glennw

bors-servo pushed a commit that referenced this pull request Nov 27, 2017
Better ClipChain optimization

Better ClipChain optimization

There are a lot of situations where we can omit redundant clips or
combine a series of clips into a single empty mask (which has the
effect of clipping the primitive in screen space). Briefly, some of the
optimizations this change includes:

1. When the inner rect of a clip completely encloses the outer rect
   of a parent, we can ignore it.
2. When the outer rectangle of a clip is completely enclosed by the
   inner rectangle of a parent, we can start a new ClipChan and
   ignore the parents.
3. If the combined inner rectangle of a ClipChain completely
   encloses the intersection of a primitive and the current clips,
   we can stop applying clips from that chain.
4. If a clip's inner rect completely encloses the intersection
   of a primitive and the current clips, we can ignore it.

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

⌛ Testing commit 71ffd50 with merge f21cc30...

@glennw
Copy link
Member

glennw commented Nov 28, 2017

@kvark @jrmuizel @staktrace Once this lands, it's probably worth getting it into a Gecko update as soon as possible - it should improve the clip mask profiling on a lot of real world sites (and a few benchmarks too).

@bors-servo
Copy link
Contributor

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

@bors-servo bors-servo merged commit 71ffd50 into servo:master Nov 28, 2017
@glennw
Copy link
Member

glennw commented Nov 28, 2017

@kvark @jrmuizel @staktrace Hmm, actually I'm seeing some strange behavior with extra large clip masks that seem redundant - it's probably unrelated to this - but might want to hold off on updating Gecko until I investigate them further.

@glennw
Copy link
Member

glennw commented Nov 28, 2017

OK, there is a PR #2115 which fixes those issues (pending green try run of course).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants