[RFC] Introduce a web based debugger and some basic examples. #1607

glennw · 2017-08-24T06:44:49Z

This introduces a simple web based debugger for WR, as an optional feature.

The initial implementation allows toggling the debug flags (e.g. profiler, texture cache debug view) and dumping a list of draw call batches to the browser.

In the future, I'd plan to expand the feature set significantly - for example, a render tree visualization, and more detailed timing. We can also use this as a debugging tool - for example, adding support for stepping through a single frame etc.

The main things to answer are:

Should the client live in this repo or elsewhere?
Is the current setup of the feature going to work OK with vendoring?
Is the current implementation / threading strategy reasonable?

The implementation uses Vue.js, websockets and Bulma.

This change is

glennw · 2017-08-24T06:45:15Z

glennw · 2017-08-24T06:45:27Z

Thoughts @kvark @jrmuizel @nical ?

glennw · 2017-08-24T07:05:22Z

BTW, it's listed as [RFC], but it's actually ready to go from my perspective, if everyone is happy with it as-is.

nical · 2017-08-24T07:18:41Z

I would like this to be in the repo. Hopefully, having it behind a feature flag like you did should be enough for vendoring.

kvark · 2017-08-24T16:01:43Z

webrender/src/renderer.rs

+                DebugMsg::FetchBatches(sender) => {
+                    let mut batch_list = BatchList::new();
+
+                    if let Some(ref frame) = self.current_frame {


nit: could combine this pattern match with the next one to avoid nesting

kvark · 2017-08-24T16:01:43Z

webrender/src/renderer.rs

+                        if let Some(ref frame) = frame.frame {
+                            for pass in &frame.passes {
+                                for target in &pass.alpha_targets.targets {
+                                    batch_list.push("[Clip] Clear", target.clip_batcher.border_clears.len());


can we add (to JSON) any information about the target itself?

Definitely - the batch list is very much a proof of concept - I'll expand on the information with a more detailed view as a follow up.

kvark · 2017-08-24T16:02:03Z

webrender/src/debug_server.rs

+#[derive(Serialize)]
+pub struct BatchList {
+    kind: &'static str,
+    batches: Vec<BatchInfo>,


we might consider having a bit stronger types here, e.g.

#[derive(Serialize)] pub struct ClipBatcherInfo { clears: usize, borders: usize, rectangles: usize, } #[derive(Serialize)] pub struct CacheBatcherInfo { ... } pub struct BatchList { kind: &'static str, clip: ClipBatcherInfo, cache: CacheBatcherInfo, alpha: HashMap<&'static str, usize>, }

Yep - sounds good. OK to do as a follow up?

kvark

Thanks @glennw , amazing stuff!

Should the client live in this repo or elsewhere?

Since we are already settled with multi-project single-repo approach, I think it would be great to have the debugger in the repo as well, for easier maintenance at the very least.

Is the current implementation / threading strategy reasonable?

Currently, it's doesn't appear that RenderBackend contributes much by being in the debug loop. I assume we need it for more debugging logic added in the future.

kvark · 2017-08-24T16:15:52Z

webrender/src/render_backend.rs

+                    notifier.unwrap()
+                            .as_mut()
+                            .unwrap()
+                            .new_frame_ready();


looks like new_frame_ready gets abused more. Maybe we should just add a ping method to be used both here and for MemoryPressure?

Sounds like a good idea.

kvark · 2017-08-24T16:15:52Z

webrender/src/renderer.rs

+
+    fn handle_debug_command(&mut self, command: DebugCommand) {
+        match command {
+            DebugCommand::EnableProfiler(enable) => {


since we are at it, maybe we can remove the duplicated logic from wrench and examples boilerplate

Do you mean remove the keyboard shortcuts? I think it makes sense to keep those for quick debugging - or did you mean something else?

kvark · 2017-08-24T16:15:52Z

webrender/src/renderer.rs

+
+    #[cfg(not(feature = "debugger"))]
+    fn update_debug_server(&self) {
+    }


perhaps, would be cleaner to do let _ = &self.debug_server than having underscore in _debug_server

glennw · 2017-08-24T21:55:00Z

@kvark Thanks for the review! Fixed up some of those nits. I think it's OK to do the rest as follow ups - does that sound OK? Also added the debugger feature to wrench, which I had missed previously.

kvark · 2017-08-25T02:11:26Z

@glennw

Do you mean remove the keyboard shortcuts?

yeah, I don't see much point in keeping the shortcuts if you can do the same thing by just enabling debugger feature. I won't lose my sleep if the shortcuts are still there though ;)

Fixed up some of those nits. I think it's OK to do the rest as follow ups - does that sound OK?

Absolutely!

One last concern before r=me is CI-testing the debugger feature. I don't see you modifying the travis/appveyor scripts.

bors-servo · 2017-08-25T02:57:53Z

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

glennw · 2017-08-25T03:24:11Z

@kvark Ah, good point - I've rebased and updated the travis file to build wrench in release + debugger mode.

kvark · 2017-08-25T16:36:10Z

Wonderful, it's shipping time!
@bors-servo r+

bors-servo · 2017-08-25T16:36:11Z

📌 Commit 3fd351f has been approved by kvark

bors-servo · 2017-08-25T16:36:13Z

⌛ Testing commit 3fd351f with merge e3b2e00...


        Auto merge of #1607 - glennw:ws4, r=kvark

[RFC] Introduce a web based debugger and some basic examples. This introduces a simple web based debugger for WR, as an optional feature. The initial implementation allows toggling the debug flags (e.g. profiler, texture cache debug view) and dumping a list of draw call batches to the browser. In the future, I'd plan to expand the feature set significantly - for example, a render tree visualization, and more detailed timing. We can also use this as a debugging tool - for example, adding support for stepping through a single frame etc. The main things to answer are: * Should the client live in this repo or elsewhere? * Is the current setup of the feature going to work OK with vendoring? * Is the current implementation / threading strategy reasonable? The implementation uses Vue.js, websockets and Bulma.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1607)

bors-servo · 2017-08-25T18:02:34Z

💔 Test failed - status-travis

kvark · 2017-08-25T18:15:44Z

CI has legitimate errors:

error: no field opaque_batches on type tiling::BatchList
--> /home/travis/build/servo/webrender/webrender/src/renderer.rs:1564:53
|
1564 | .opaque_batches
| ^^^^^^^^^^^^^^ did you mean opaque_batch_list?


        Add 3rd party dependencies.


        Add debugger application.


        Basic WR debugger integration.

glennw · 2017-08-25T20:32:54Z

Ah, it needed a rebase after the batching PR landed - done.

kvark · 2017-08-25T20:38:38Z

thanks!
@bors-servo r+

bors-servo · 2017-08-25T20:38:39Z

📌 Commit dcbe7c9 has been approved by kvark

bors-servo · 2017-08-25T20:38:42Z

⌛ Testing commit dcbe7c9 with merge 56f75e7...


        Auto merge of #1607 - glennw:ws4, r=kvark

[RFC] Introduce a web based debugger and some basic examples. This introduces a simple web based debugger for WR, as an optional feature. The initial implementation allows toggling the debug flags (e.g. profiler, texture cache debug view) and dumping a list of draw call batches to the browser. In the future, I'd plan to expand the feature set significantly - for example, a render tree visualization, and more detailed timing. We can also use this as a debugging tool - for example, adding support for stepping through a single frame etc. The main things to answer are: * Should the client live in this repo or elsewhere? * Is the current setup of the feature going to work OK with vendoring? * Is the current implementation / threading strategy reasonable? The implementation uses Vue.js, websockets and Bulma.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1607)

bors-servo · 2017-08-25T22:07:04Z

☀️ Test successful - status-travis
Approved by: kvark
Pushing 56f75e7 to master...

kvark approved these changes Aug 24, 2017

View changes

kvark reviewed Aug 24, 2017

View changes

kvark approved these changes Aug 24, 2017

View changes

glennw force-pushed the glennw:ws4 branch 2 times, most recently from e4421a7 to 4fe0d28 Aug 24, 2017

glennw force-pushed the glennw:ws4 branch from 4fe0d28 to 3fd351f Aug 25, 2017

gw3583 added 3 commits Aug 24, 2017

Add 3rd party dependencies.

75fdc34

Add debugger application.

03a77be

Basic WR debugger integration.

Loading status checks…

dcbe7c9

glennw force-pushed the glennw:ws4 branch from 3fd351f to dcbe7c9 Aug 25, 2017

bors-servo merged commit dcbe7c9 into servo:master Aug 25, 2017
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

This was referenced Aug 25, 2017

[WIP] Add coverage recording #1229

Closed

Add an experimental geometry api #1461

Closed

servo / webrender

[RFC] Introduce a web based debugger and some basic examples. #1607

[RFC] Introduce a web based debugger and some basic examples. #1607

glennw commented Aug 24, 2017 •

edited

glennw commented Aug 24, 2017

glennw commented Aug 24, 2017 •

edited

glennw commented Aug 24, 2017

nical commented Aug 24, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

glennw commented Aug 24, 2017

kvark commented Aug 25, 2017

bors-servo commented Aug 25, 2017

glennw commented Aug 25, 2017

kvark commented Aug 25, 2017

bors-servo commented Aug 25, 2017

bors-servo commented Aug 25, 2017

bors-servo commented Aug 25, 2017

kvark commented Aug 25, 2017

glennw commented Aug 25, 2017

kvark commented Aug 25, 2017

bors-servo commented Aug 25, 2017

bors-servo commented Aug 25, 2017

bors-servo commented Aug 25, 2017

servo / webrender

Join GitHub today

[RFC] Introduce a web based debugger and some basic examples. #1607

[RFC] Introduce a web based debugger and some basic examples. #1607

Conversation

glennw commented Aug 24, 2017 • edited

glennw commented Aug 24, 2017

glennw commented Aug 24, 2017 • edited

glennw commented Aug 24, 2017

nical commented Aug 24, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

kvark left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

glennw commented Aug 24, 2017

kvark commented Aug 25, 2017

bors-servo commented Aug 25, 2017

glennw commented Aug 25, 2017

kvark commented Aug 25, 2017

bors-servo commented Aug 25, 2017

bors-servo commented Aug 25, 2017

bors-servo commented Aug 25, 2017

kvark commented Aug 25, 2017

glennw commented Aug 25, 2017

kvark commented Aug 25, 2017

bors-servo commented Aug 25, 2017

bors-servo commented Aug 25, 2017

bors-servo commented Aug 25, 2017

glennw commented Aug 24, 2017 •

edited

glennw commented Aug 24, 2017 •

edited