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

Optimize how local rects are calculated during culling. #2968

Merged
merged 1 commit into from Aug 13, 2018

Conversation

Projects
None yet
3 participants
@gw3583
Contributor

gw3583 commented Aug 13, 2018

Apply similar logic to the clip code to minimize the
amount of work that is needed when building local
rects for a picture. This change is quite a
significant optimization - in most cases it halves
the amount of work done here, by skipping the extra
rect calculation for non-3d rendering context
pictures. It also saves some rect x matrix transforms
in various cases.

This is a bit more preparation work for rasterizing
pictures in difference coordinate spaces. There are a
few other miscellaneous improvements too:

  • Rename some fields to be more appropriate.
  • Reduce number of matrix inverse calls.
  • Remove reference_frame_stack, which is no longer
    required.
  • Remove BuiltDisplayList from PrimitiveContext, which
    will be helpful in future patches.

This change is Reviewable

@gw3583

This comment has been minimized.

Show comment
Hide comment
@gw3583
Contributor

gw3583 commented Aug 13, 2018

@kvark

kvark approved these changes Aug 13, 2018

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @gw3583)


webrender/src/prim_store.rs, line 142 at r1 (raw file):

        let target_spatial_node = &spatial_nodes[target_node_index.0];

        if self.ref_spatial_node_index == target_node_index {

nit: let's reword this functionally as self.kind = if ...


webrender/src/prim_store.rs, line 1893 at r1 (raw file):

                ) {
                    frame_state.profile_counters.visible_primitives.inc();
                    result.accumulate(&prim_local_rect);

much nicer 👍

Optimize how local rects are calculated during culling.
Apply similar logic to the clip code to minimize the
amount of work that is needed when building local
rects for a picture. This change is quite a
significant optimization - in most cases it halves
the amount of work done here, by skipping the extra
rect calculation for non-3d rendering context
pictures. It also saves some rect x matrix transforms
in various cases.

This is a bit more preparation work for rasterizing
pictures in difference coordinate spaces. There are a
few other miscellaneous improvements too:

 * Rename some fields to be more appropriate.
 * Reduce number of matrix inverse calls.
 * Remove reference_frame_stack, which is no longer
   required.
 * Remove BuiltDisplayList from PrimitiveContext, which
   will be helpful in future patches.
@gw3583

This comment has been minimized.

Show comment
Hide comment
@gw3583

gw3583 Aug 13, 2018

Contributor

Addressed review comments, thanks!

@bors-servo r=kvark

Contributor

gw3583 commented Aug 13, 2018

Addressed review comments, thanks!

@bors-servo r=kvark

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 13, 2018

Contributor

📌 Commit da19712 has been approved by kvark

Contributor

bors-servo commented Aug 13, 2018

📌 Commit da19712 has been approved by kvark

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

Auto merge of #2968 - gw3583:pic-context, r=kvark
Optimize how local rects are calculated during culling.

Apply similar logic to the clip code to minimize the
amount of work that is needed when building local
rects for a picture. This change is quite a
significant optimization - in most cases it halves
the amount of work done here, by skipping the extra
rect calculation for non-3d rendering context
pictures. It also saves some rect x matrix transforms
in various cases.

This is a bit more preparation work for rasterizing
pictures in difference coordinate spaces. There are a
few other miscellaneous improvements too:

 * Rename some fields to be more appropriate.
 * Reduce number of matrix inverse calls.
 * Remove reference_frame_stack, which is no longer
   required.
 * Remove BuiltDisplayList from PrimitiveContext, which
   will be helpful in future patches.

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

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 13, 2018

Contributor

⌛️ Testing commit da19712 with merge 890f374...

Contributor

bors-servo commented Aug 13, 2018

⌛️ Testing commit da19712 with merge 890f374...

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 13, 2018

Contributor

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

Contributor

bors-servo commented Aug 13, 2018

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

@bors-servo bors-servo merged commit da19712 into servo:master Aug 13, 2018

3 of 4 checks passed

code-review/reviewable 1 file, 2 discussions left (gw3583, kvark)
Details
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