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

Extract nine patch segment generation into create_nine_patch_segments. #3300

Merged
merged 2 commits into from Nov 12, 2018

Conversation

gw3583
Copy link
Contributor

@gw3583 gw3583 commented Nov 12, 2018

This change is Reviewable

@gw3583
Copy link
Contributor Author

gw3583 commented Nov 12, 2018

The patches I'm working on towards border interning are getting quite large, so I'm trying to split it up into smaller chunks. This patch is a straightforward move of code from flattening into a reusable function. We will need this later since the segment generating when interning only happens when a unique primitive is created, not every time a new scene is built.

r? @kvark or anyone

@gw3583
Copy link
Contributor Author

gw3583 commented Nov 12, 2018

Added a follow up comment that reduces the size of PrimitiveInstance, and ensures that interned primitives don't use the gpu cache handle that (was) in the instance.

Pending try with both patches:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0348b4407434029ec50b94b3ab0e0babc44573dd

@gw3583
Copy link
Contributor Author

gw3583 commented Nov 12, 2018

Try run looks good.

Copy link
Member

@kvark kvark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

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


webrender/src/border.rs, line 1146 at r2 (raw file):

}

pub fn create_nine_patch_segments(

I assume this code isn't changed, just moved over


webrender/src/prim_store.rs, line 951 at r2 (raw file):

    }

    fn write_gpu_blocks(

this method is now inconsistent with all the other write_gpu_blocks, which receive GpuDataRequest. Let's rename it to avoid confusion

It's not used by interned primitives, and it's a performance win
on talos whenever we can reduce the size of PrimitiveInstance.
Copy link
Contributor Author

@gw3583 gw3583 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @kvark)


webrender/src/border.rs, line 1146 at r2 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

I assume this code isn't changed, just moved over

Yep, that's right.


webrender/src/prim_store.rs, line 951 at r2 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

this method is now inconsistent with all the other write_gpu_blocks, which receive GpuDataRequest. Let's rename it to avoid confusion

Renamed to write_gpu_blocks_if_required

@gw3583
Copy link
Contributor Author

gw3583 commented Nov 12, 2018

@bors-servo r=kvark

@bors-servo
Copy link
Contributor

📌 Commit 6599760 has been approved by kvark

@bors-servo
Copy link
Contributor

⌛ Testing commit 6599760 with merge fbf6516...

bors-servo pushed a commit that referenced this pull request Nov 12, 2018
Extract nine patch segment generation into create_nine_patch_segments.

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

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

@bors-servo bors-servo merged commit 6599760 into servo:master Nov 12, 2018
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