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

gfx: Make display lists serializable using `serde`. #6583

Merged
merged 1 commit into from Jul 16, 2015

Conversation

Projects
None yet
5 participants
@pcwalton
Contributor

pcwalton commented Jul 8, 2015

This commit introduces the serde dependency, which we will use to
serialize messages going between processes in multiprocess Servo.

This also adds a new debugging flag, -Z print-display-list-json,
allowing the output of display list serialization to be visualized.
This will be useful for our experiments with alternate rasterizers.

r? @metajack

Review on Reviewable

@metajack

This comment has been minimized.

Show comment
Hide comment
@metajack

metajack Jul 8, 2015

Contributor

-S-awaiting-review +S-needs-code-changes


Reviewed 34 of 34 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


components/gfx/platform/macos/font_template.rs, line 52 [r1] (raw file):
This needs some documentation.


Comments from the review on Reviewable.io

Contributor

metajack commented Jul 8, 2015

-S-awaiting-review +S-needs-code-changes


Reviewed 34 of 34 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


components/gfx/platform/macos/font_template.rs, line 52 [r1] (raw file):
This needs some documentation.


Comments from the review on Reviewable.io

@pcwalton

This comment has been minimized.

Show comment
Hide comment
@pcwalton

pcwalton Jul 9, 2015

Contributor

Switched to Deref on LinkedList.

r? @metajack

Contributor

pcwalton commented Jul 9, 2015

Switched to Deref on LinkedList.

r? @metajack

@metajack

This comment has been minimized.

Show comment
Hide comment
@metajack

metajack Jul 9, 2015

Contributor

Missing Cargo.lock updates for the other ports.

-S-awaiting-review +S-needs-code-changes


Reviewed 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


components/gfx/font_context.rs, line 44 [r2] (raw file):
CachedCTFont should probably get deref treatment instead of .0 too.


components/gfx/platform/macos/font.rs, line 67 [r2] (raw file):
Same here.


Comments from the review on Reviewable.io

Contributor

metajack commented Jul 9, 2015

Missing Cargo.lock updates for the other ports.

-S-awaiting-review +S-needs-code-changes


Reviewed 5 of 5 files at r2.
Review status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.


components/gfx/font_context.rs, line 44 [r2] (raw file):
CachedCTFont should probably get deref treatment instead of .0 too.


components/gfx/platform/macos/font.rs, line 67 [r2] (raw file):
Same here.


Comments from the review on Reviewable.io

@pcwalton

This comment has been minimized.

Show comment
Hide comment
@pcwalton

pcwalton Jul 11, 2015

Contributor

@metajack Updated per your comments. r?

Contributor

pcwalton commented Jul 11, 2015

@metajack Updated per your comments. r?

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Jul 12, 2015

Contributor

☔️ The latest upstream changes (presumably #6606) made this pull request unmergeable. Please resolve the merge conflicts.

Contributor

bors-servo commented Jul 12, 2015

☔️ The latest upstream changes (presumably #6606) made this pull request unmergeable. Please resolve the merge conflicts.

@metajack

This comment has been minimized.

Show comment
Hide comment
@metajack

metajack Jul 13, 2015

Contributor

-S-awaiting-review +S-needs-code-changes +S-needs-rebase

r=me once you add the comment and rebase.


Reviewed 6 of 6 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.


components/gfx/platform/macos/font_template.rs, line 53 [r3] (raw file):
This still needs a code comment. Why does this exist? When is it None vs. Some(CTFont)?

You already explained this to me in the discussion, but that needs to be recorded in the source.


Comments from the review on Reviewable.io

Contributor

metajack commented Jul 13, 2015

-S-awaiting-review +S-needs-code-changes +S-needs-rebase

r=me once you add the comment and rebase.


Reviewed 6 of 6 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.


components/gfx/platform/macos/font_template.rs, line 53 [r3] (raw file):
This still needs a code comment. Why does this exist? When is it None vs. Some(CTFont)?

You already explained this to me in the discussion, but that needs to be recorded in the source.


Comments from the review on Reviewable.io

@pcwalton

This comment has been minimized.

Show comment
Hide comment
@pcwalton

pcwalton Jul 13, 2015

Contributor

Done. However, this is now blocked on servo/string-cache#92 due to some changes that landed while this was being reviewed.

Contributor

pcwalton commented Jul 13, 2015

Done. However, this is now blocked on servo/string-cache#92 due to some changes that landed while this was being reviewed.

@pcwalton

This comment has been minimized.

Show comment
Hide comment
@pcwalton

pcwalton Jul 14, 2015

Contributor

Unfortunately, because string-cache has taken changes that require a rustup, this is now blocked on either the rustup or servo/string-cache#93.

Contributor

pcwalton commented Jul 14, 2015

Unfortunately, because string-cache has taken changes that require a rustup, this is now blocked on either the rustup or servo/string-cache#93.

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Jul 14, 2015

Contributor

☔️ The latest upstream changes (presumably #6596) made this pull request unmergeable. Please resolve the merge conflicts.

Contributor

bors-servo commented Jul 14, 2015

☔️ The latest upstream changes (presumably #6596) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Jul 15, 2015

Contributor

☔️ The latest upstream changes (presumably #6591) made this pull request unmergeable. Please resolve the merge conflicts.

Contributor

bors-servo commented Jul 15, 2015

☔️ The latest upstream changes (presumably #6591) made this pull request unmergeable. Please resolve the merge conflicts.

gfx: Make display lists serializable using `serde`.
This commit introduces the `serde` dependency, which we will use to
serialize messages going between processes in multiprocess Servo.

This also adds a new debugging flag, `-Z print-display-list-json`,
allowing the output of display list serialization to be visualized.
This will be useful for our experiments with alternate rasterizers.
@pcwalton

This comment has been minimized.

Show comment
Hide comment
@pcwalton

pcwalton Jul 16, 2015

Contributor

@bors-servo: r=metajack

Contributor

pcwalton commented Jul 16, 2015

@bors-servo: r=metajack

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Jul 16, 2015

Contributor

📌 Commit 6eacb0c has been approved by metajack

Contributor

bors-servo commented Jul 16, 2015

📌 Commit 6eacb0c has been approved by metajack

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Jul 16, 2015

Contributor

⌛️ Testing commit 6eacb0c with merge ef97152...

Contributor

bors-servo commented Jul 16, 2015

⌛️ Testing commit 6eacb0c with merge ef97152...

bors-servo pushed a commit that referenced this pull request Jul 16, 2015

bors-servo
Auto merge of #6583 - pcwalton:serializable-display-list, r=metajack
gfx: Make display lists serializable using `serde`.

This commit introduces the `serde` dependency, which we will use to
serialize messages going between processes in multiprocess Servo.

This also adds a new debugging flag, `-Z print-display-list-json`,
allowing the output of display list serialization to be visualized.
This will be useful for our experiments with alternate rasterizers.

r? @metajack

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6583)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Jul 16, 2015

Contributor

💔 Test failed - mac1

Contributor

bors-servo commented Jul 16, 2015

💔 Test failed - mac1

@pcwalton

This comment has been minimized.

Show comment
Hide comment
@pcwalton

pcwalton Jul 16, 2015

Contributor

@bors-servo: retry

WPT test timeouts don't seem like they could be possibly related to a patch that adds a bunch of dead code.

Contributor

pcwalton commented Jul 16, 2015

@bors-servo: retry

WPT test timeouts don't seem like they could be possibly related to a patch that adds a bunch of dead code.

@pcwalton

This comment has been minimized.

Show comment
Hide comment
@pcwalton

pcwalton Jul 16, 2015

Contributor

@bors-servo: retry

Contributor

pcwalton commented Jul 16, 2015

@bors-servo: retry

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Jul 16, 2015

Contributor

⚡️ Previous build results for android, gonk, linux1, linux2, linux3, mac2, mac3 are reusable. Rebuilding only mac1...

Contributor

bors-servo commented Jul 16, 2015

⚡️ Previous build results for android, gonk, linux1, linux2, linux3, mac2, mac3 are reusable. Rebuilding only mac1...

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Jul 16, 2015

Contributor

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

Contributor

bors-servo commented Jul 16, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

@bors-servo bors-servo merged commit 6eacb0c into servo:master Jul 16, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment