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 how local rects are accumulated for 3d contexts. #2975

Merged
merged 1 commit into from Aug 16, 2018

Conversation

Projects
None yet
3 participants
@gw3583
Contributor

gw3583 commented Aug 16, 2018

Pictures in a 3d rendering context are re-parented during
flattening, to ensure correct ordering during plane splitting
when un-transformed content is present.

Previously, we determined the spatial node index based on
the re-parented hierarchy, and this complicated local rect
calculation for items inside a 3d rendering context. For
these, we needed to retain the original spatial node index
and do extra calculations.

Now, we determine the correct spatial node index during
flattening, and store it inside each primitive directly
(rather than via the primitive runs inside a picture).

This greatly simplifies the process of accumulating a
local rect for a picture as we recurse through the
picture tree - allowing pictures inside a 3d context
to be processed just as any normal primitives are.

This simplifies some upcoming work to rasterize pictures
in local space, and is also a small optimization that
reduces the number of local rects we need to build and
transform.


This change is Reviewable

Simplify how local rects are accumulated for 3d contexts.
Pictures in a 3d rendering context are re-parented during
flattening, to ensure correct ordering during plane splitting
when un-transformed content is present.

Previously, we determined the spatial node index based on
the re-parented hierarchy, and this complicated local rect
calculation for items inside a 3d rendering context. For
these, we needed to retain the original spatial node index
and do extra calculations.

Now, we determine the correct spatial node index during
flattening, and store it inside each primitive directly
(rather than via the primitive runs inside a picture).

This greatly simplifies the process of accumulating a
local rect for a picture as we recurse through the
picture tree - allowing pictures inside a 3d context
to be processed just as any normal primitives are.

This simplifies some upcoming work to rasterize pictures
in local space, and is also a small optimization that
reduces the number of local rects we need to build and
transform.
@gw3583

This comment has been minimized.

Show comment
Hide comment
@gw3583

gw3583 Aug 16, 2018

Contributor

r? @kvark

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=79dabb3040fc72e6dbc73a0069d0b1ff1658b99c

Still has a few jobs to finish, but looks good so far.
R1 and R8 are PASS results from a previous PR.
Wr1 and wpt3 are unrelated intermittent failures.

Contributor

gw3583 commented Aug 16, 2018

r? @kvark

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=79dabb3040fc72e6dbc73a0069d0b1ff1658b99c

Still has a few jobs to finish, but looks good so far.
R1 and R8 are PASS results from a previous PR.
Wr1 and wpt3 are unrelated intermittent failures.

@kvark

kvark approved these changes Aug 16, 2018

looks much cleaner!

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


webrender/src/frame_builder.rs, line 243 at r1 (raw file):

            &frame_context,
            &mut frame_state,
            &mut local_rect_builder,

neither the calling body or the restore_context function care about LocalRectBuilder. It would be simpler to move it's creation and destruction to inside prepare_prim_runs and just return the resulting local rect from it.

@kvark

This comment has been minimized.

Show comment
Hide comment
@kvark
Member

kvark commented Aug 16, 2018

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 16, 2018

Contributor

📌 Commit 842c1a2 has been approved by kvark

Contributor

bors-servo commented Aug 16, 2018

📌 Commit 842c1a2 has been approved by kvark

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

Auto merge of #2975 - gw3583:mapping, r=kvark
Simplify how local rects are accumulated for 3d contexts.

Pictures in a 3d rendering context are re-parented during
flattening, to ensure correct ordering during plane splitting
when un-transformed content is present.

Previously, we determined the spatial node index based on
the re-parented hierarchy, and this complicated local rect
calculation for items inside a 3d rendering context. For
these, we needed to retain the original spatial node index
and do extra calculations.

Now, we determine the correct spatial node index during
flattening, and store it inside each primitive directly
(rather than via the primitive runs inside a picture).

This greatly simplifies the process of accumulating a
local rect for a picture as we recurse through the
picture tree - allowing pictures inside a 3d context
to be processed just as any normal primitives are.

This simplifies some upcoming work to rasterize pictures
in local space, and is also a small optimization that
reduces the number of local rects we need to build and
transform.

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

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Aug 16, 2018

Contributor

⌛️ Testing commit 842c1a2 with merge e70bae0...

Contributor

bors-servo commented Aug 16, 2018

⌛️ Testing commit 842c1a2 with merge e70bae0...

@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 e70bae0 to master...

Contributor

bors-servo commented Aug 16, 2018

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

@bors-servo bors-servo merged commit 842c1a2 into servo:master Aug 16, 2018

3 of 4 checks passed

code-review/reviewable 1 discussion left (gw3583)
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