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 option to record frames for playback #12630

Closed
wants to merge 1 commit into from

Conversation

@lanamorgan
Copy link
Collaborator

lanamorgan commented Jul 28, 2016

Adds a debug option, -Z record, to record webrender's display lists for debugging.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive
Copy link

highfive commented Jul 28, 2016

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @emilio (or someone else) soon.

@paulrouget
Copy link
Contributor

paulrouget commented Jul 28, 2016

I'm not sure to understand what this is supposed to do. Where is enable_recording used?

@jdm
Copy link
Member

jdm commented Jul 28, 2016

@lanamorgan
Copy link
Collaborator Author

lanamorgan commented Jul 28, 2016

This is for an issue in webrender :)
servo/webrender#6

@lanamorgan lanamorgan force-pushed the lanamorgan:serialization branch from 05ce98f to 72a9f29 Jul 28, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Aug 3, 2016

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

@lanamorgan lanamorgan force-pushed the lanamorgan:serialization branch 2 times, most recently from 4a76b52 to a76a2f8 Aug 5, 2016
@lanamorgan lanamorgan force-pushed the lanamorgan:serialization branch from a76a2f8 to d212086 Aug 8, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Aug 21, 2016

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

Copy link
Member

shinglyu left a comment

Sorry for the wait, LGTM with some naming nits.

@@ -811,6 +820,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
dump_display_list: debug_options.dump_display_list,
dump_display_list_json: debug_options.dump_display_list_json,
dump_layer_tree: debug_options.dump_layer_tree,
enable_recording: debug_options.enable_recording,

This comment has been minimized.

@shinglyu

shinglyu Dec 15, 2016

Member

There is a similar feature for dumping layout frame tree called trace_layout, maybe we should follow the convention and call this trace_webrender or record_webrender, or even record_display_list.

@@ -361,6 +368,7 @@ pub fn print_debug_usage(app: &str) -> ! {
print_option("dump-display-list", "Print the display list after each layout.");
print_option("dump-display-list-json", "Print the display list in JSON form.");
print_option("dump-layer-tree", "Print the layer tree whenever it changes.");
print_option("record", "Serializes all frame data sent to webrender and writes to record folder.");

This comment has been minimized.

@shinglyu

shinglyu Dec 15, 2016

Member

Is "frame data" == display list? Just say "Serialize the display list ..." sounds clearer to me.

@@ -167,6 +167,9 @@ pub struct Opts {
/// Dumps the layer tree when it changes.
pub dump_layer_tree: bool,

///Record and write to disk each frame sent to webrender.

This comment has been minimized.

@shinglyu

shinglyu Dec 15, 2016

Member

Does this one do the similar thing as the one in DebugOptions? Then we should probably use the same comment.

@shinglyu
Copy link
Member

shinglyu commented Dec 15, 2016

Travis build was failing, is the webrender side ready?

@jdm
Copy link
Member

jdm commented Dec 19, 2016

This was subsumed by #14249.

@jdm jdm closed this Dec 19, 2016
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

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