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

Add primitive interning for YUV images. #3349

Merged
merged 1 commit into from Nov 26, 2018
Merged

Conversation

@gw3583
Copy link
Collaborator

gw3583 commented Nov 24, 2018

Also restructure and tidy up some of the code that handles
segment building + interned primitives. This reduces some of
the code duplication and will simplify porting the remaining
primitives to interning.


This change is Reviewable

@gw3583
Copy link
Collaborator Author

gw3583 commented Nov 24, 2018

r? @kvark or anyone

This includes #3348 and #3346 - it might be easier to review this after those land.

Pending try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c12f066f054dc542f3b726426f1de41c1e7859de

@gw3583 gw3583 force-pushed the gw3583:intern-yuv branch from 70c5e3b to c56d311 Nov 25, 2018
Also restructure and tidy up some of the code that handles
segment building + interned primitives. This reduces some of
the code duplication and will simplify porting the remaining
primitives to interning.
@gw3583 gw3583 force-pushed the gw3583:intern-yuv branch from c56d311 to d3187bf Nov 25, 2018
@gw3583
Copy link
Collaborator Author

gw3583 commented Nov 25, 2018

Rebased this on top of the recent patches that have landed, so there is just one commit here now.

@kvark
kvark approved these changes Nov 26, 2018
Copy link
Member

kvark left a comment

:lgtm: , looks fairly mechanical

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


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

        color: ColorF,
    },
    YuvImage {

similarly, it's unfortunate that this is duplicated with PrimitiveKeyKind


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

        };

        debug_assert!(segment_instance_index != SegmentInstanceIndex::INVALID);

do we really need the UNUSED variant here as opposed to Option<>?

@kvark
Copy link
Member

kvark commented Nov 26, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Nov 26, 2018

📌 Commit d3187bf has been approved by kvark

@bors-servo
Copy link
Contributor

bors-servo commented Nov 26, 2018

Testing commit d3187bf with merge f3e489e...

bors-servo added a commit that referenced this pull request Nov 26, 2018
Add primitive interning for YUV images.

Also restructure and tidy up some of the code that handles
segment building + interned primitives. This reduces some of
the code duplication and will simplify porting the remaining
primitives to interning.

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

bors-servo commented Nov 26, 2018

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

@bors-servo bors-servo merged commit d3187bf into servo:master Nov 26, 2018
3 of 4 checks passed
3 of 4 checks passed
code-review/reviewable 2 discussions left (gw3583)
Details
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 27, 2018
…4c3a7e6cb31d (WR PR #3349). r=kats

servo/webrender#3349

Differential Revision: https://phabricator.services.mozilla.com/D13030

--HG--
extra : moz-landing-system : lando
jankeromnes pushed a commit to jankeromnes/gecko that referenced this pull request Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.