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

Simplify determining if a batch can be merged / needs scissor. #2974

Merged
merged 1 commit into from Aug 16, 2018

Conversation

Projects
None yet
3 participants
@gw3583
Contributor

gw3583 commented Aug 16, 2018

Previously, we checked if the bounding box of any child primitives
extended outside the rectangle of the allocated task rect. However,
there is a simpler way to calculate this.

If the allocated size of the render task is >= the unclipped size
of the picture bounding rect, no scissor is needed, since we know
that local clip rects will take care of ensuring nothing is
drawn outside the task boundary.

This is an optimization, but the main benefit is removing one more
piece of code that relies on knowledge of screen / device rects,
which simplifies the ongoing work to be able to rasterize in other
coordinate systems.


This change is Reviewable

@gw3583

This comment has been minimized.

Show comment
Hide comment
@gw3583

gw3583 Aug 16, 2018

Contributor

Also removed support for dynamic tasks with an unknown size, since that's no longer used.

r? @kvark

Try run here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=31d78e6ce4d91bfedf2716794b5e9656714f285d

Still has a few jobs to finish, but looks good so far:
R1 and R8 are new PASSes from a previous PR.
R5, Wd, wpt3, mda2 appear to be unrelated intermittents.

Contributor

gw3583 commented Aug 16, 2018

Also removed support for dynamic tasks with an unknown size, since that's no longer used.

r? @kvark

Try run here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=31d78e6ce4d91bfedf2716794b5e9656714f285d

Still has a few jobs to finish, but looks good so far:
R1 and R8 are new PASSes from a previous PR.
R5, Wd, wpt3, mda2 appear to be unrelated intermittents.

@kvark

kvark approved these changes Aug 16, 2018

Simplify determining if a batch can be merged / needs scissor.
Previously, we checked if the bounding box of any child primitives
extended outside the rectangle of the allocated task rect. However,
there is a simpler way to calculate this.

If the allocated size of the render task is >= the unclipped size
of the picture bounding rect, no scissor is needed, since we know
that local clip rects will take care of ensuring nothing is
drawn outside the task boundary.

This is an optimization, but the main benefit is removing one more
piece of code that relies on knowledge of screen / device rects,
which simplifies the ongoing work to be able to rasterize in other
coordinate systems.
@gw3583

This comment has been minimized.

Show comment
Hide comment
@gw3583

gw3583 Aug 16, 2018

Contributor

Fixed up a compile error in the pathfinder feature code path.

Contributor

gw3583 commented Aug 16, 2018

Fixed up a compile error in the pathfinder feature code path.

@gw3583

This comment has been minimized.

Show comment
Hide comment
@gw3583

gw3583 Aug 16, 2018

Contributor

@bors-servo r=kvark

Contributor

gw3583 commented Aug 16, 2018

@bors-servo r=kvark

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 16, 2018

Contributor

📌 Commit 8ca21b0 has been approved by kvark

Contributor

bors-servo commented Aug 16, 2018

📌 Commit 8ca21b0 has been approved by kvark

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 16, 2018

Contributor

⌛️ Testing commit 8ca21b0 with merge 733e825...

Contributor

bors-servo commented Aug 16, 2018

⌛️ Testing commit 8ca21b0 with merge 733e825...

bors-servo added a commit that referenced this pull request Aug 16, 2018

Auto merge of #2974 - gw3583:batch-fix, r=kvark
Simplify determining if a batch can be merged / needs scissor.

Previously, we checked if the bounding box of any child primitives
extended outside the rectangle of the allocated task rect. However,
there is a simpler way to calculate this.

If the allocated size of the render task is >= the unclipped size
of the picture bounding rect, no scissor is needed, since we know
that local clip rects will take care of ensuring nothing is
drawn outside the task boundary.

This is an optimization, but the main benefit is removing one more
piece of code that relies on knowledge of screen / device rects,
which simplifies the ongoing work to be able to rasterize in other
coordinate systems.

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

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 16, 2018

Contributor

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

Contributor

bors-servo commented Aug 16, 2018

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

@bors-servo bors-servo merged commit 8ca21b0 into servo:master Aug 16, 2018

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details

@gw3583 gw3583 deleted the gw3583:batch-fix branch Aug 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment