Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upPretty print the flow tree #12675
Pretty print the flow tree #12675
Comments
|
Or maybe we should make |
|
+1 for post-processor. Or maybe render it into some HTML-formatted page? so we can let browser to handle indentation etc. |
|
@glennw Do you have any opinion on this? I heard that you are the last one who worked on this recently. |
|
WebKit has a specialized test client called DumpRenderTree, which is like a headless browser which dumps the render tree for layout testing. The output looks like this:
Ref: https://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree# |
|
Both tools dumps the tree in plain text, which is console-friendly but not optimal for human reader or other tools to parse. I'll make it emit a JSON object and write a python (?) formatter and/or visualization tool. I'd also want to create a debug tool that dumps the flow tree in every key stage in the layout process, and we can have a time slider to explore how the flow tree evolves during the layout process |
|
Thanks for taking this on, and a huge I also agree that having JSON (or some other format that is SUPER easily parsed by off the shelf libraries) is a great strategy. |
|
I'm also considering YAML. YAML is more human readable, so before we build any fancy tool, developers can still look at the raw output and benefit from the improved readability |
|
@nox points out that we can use I would probably keep the old |
|
And as @nox said it would be even better to switch to |
|
This looks exactly what we want: acedb16 But there is a bug in it so it always print an empty trace. I'll submit a patch for that right away. They way to use it is |
|
I found it, but looks like it was removed: #3350 Removed by:
|
Fixed layout flow tree JSON serialization <!-- Please describe your changes on the following line: --> The second argument for the `emit_struct()` is the number of fields, if given `0`, the output JSON will always be empty. This is used in `./mach run -d -Z trace-layout https://servo.org`, which will dump the layout flow tree into a `layout_trace.json` file for debugging. This also unblocks #12675 --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors <!-- Either: --> - [x] These changes do not require tests because its a debugging tool, not critical for normal code path <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12935) <!-- Reviewable:end -->
Migrated -Z trace-layout to serde_json <!-- Please describe your changes on the following line: --> Migrated the trace-layout code from old `rustc-serialize` to `serde_json`. This will help us iterate faster on the layout viewer (#13432), #13436, #12675 and fix #12936. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #12936 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because it's a relatively low risk debug tool <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13740) <!-- Reviewable:end -->
Migrated -Z trace-layout to serde_json <!-- Please describe your changes on the following line: --> Migrated the trace-layout code from old `rustc-serialize` to `serde_json`. This will help us iterate faster on the layout viewer (#13432), #13436, #12675 and fix #12936. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #12936 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because it's a relatively low risk debug tool <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13740) <!-- Reviewable:end -->
Migrated -Z trace-layout to serde_json <!-- Please describe your changes on the following line: --> Migrated the trace-layout code from old `rustc-serialize` to `serde_json`. This will help us iterate faster on the layout viewer (#13432), #13436, #12675 and fix #12936. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #12936 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because it's a relatively low risk debug tool <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13740) <!-- Reviewable:end -->
Migrated -Z trace-layout to serde_json <!-- Please describe your changes on the following line: --> Migrated the trace-layout code from old `rustc-serialize` to `serde_json`. This will help us iterate faster on the layout viewer (#13432), #13436, #12675 and fix #12936. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #12936 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because it's a relatively low risk debug tool <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13740) <!-- Reviewable:end -->
Migrated -Z trace-layout to serde_json <!-- Please describe your changes on the following line: --> Migrated the trace-layout code from old `rustc-serialize` to `serde_json`. This will help us iterate faster on the layout viewer (#13432), #13436, #12675 and fix #12936. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #12936 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because it's a relatively low risk debug tool <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13740) <!-- Reviewable:end -->
Migrated -Z trace-layout to serde_json <!-- Please describe your changes on the following line: --> Migrated the trace-layout code from old `rustc-serialize` to `serde_json`. This will help us iterate faster on the layout viewer (#13432), #13436, #12675 and fix #12936. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #12936 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because it's a relatively low risk debug tool <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13740) <!-- Reviewable:end -->
|
Previous attempt and discussion: #13092 |
|
Now I use |
|
Hi! I am a very noobie to Servo, but may I work on it with some help? (: |
|
@Ilphrin That should be fine! Take a look at the previous discussion and attempt and ask questions about anything that's unclear! |
|
Unassigning due to lack of response. |
|
Please do! |
|
@shinglyu, i saw the flow of dump-flow-tree, so I've to make the all flow class(inheriting flow trait) serializable and then do serde_json::to_string(root_flow) ? |
|
@shinglyu please review the changes made in this tree: https://github.com/murarisumit/servo/tree/12675 |
|
@murarisumit Sorry for the delay, the code looks fine to me (master...murarisumit:12675) Would you mind posting an example of the json output? Just want to make sure it's valid json and easily parsable. |
|
@shinglyu It's slightly large file, so sharing it via gist: https://gist.github.com/murarisumit/63667a8376fe74099bb9bb5faedc369b |
|
@shinglyu Did you look at the json output? Is this ready for a PR? |
|
@murarisumit The JSON output looks nice. Feel free to open a PR. |
|
cc @mihajlija, who was looking into something like this. |
|
hi @shinglyu, there are many changes since my last commit. My forked repo is around 3000 commit behind the master, and there are changes in master repo that I'm not aware of it. I've merge it with the current master repo, let me know what next should I do next with it ? |
|
@murarisumit So are you having trouble rebasing (taking in the master changes) or creating a PR? Otherwise, push your code to your fork and go to servo/servo's Pull requests page, there is a button to create a pull request. We can then continue to review and test your code in the pull request page. |
|
Was this done, in the end? |
|
Never mind… #18266. |
|
Latest attempt was at #18266, with a review comment that needs to be addressed. |
|
I am going to do this |
|
@highfive: assign me |
|
Hey @tigercosmos! Thanks for your interest in working on this issue. It's now assigned to you! |
pretty print tree
<!-- Please describe your changes on the following line: -->
```
│ │ │ │ │ │ │ floatspec-out=L 0px R 0px
│ │ │ │ │ │ │ overflow=Overflow { scroll: TypedRect(0px×0px at (0px,0px)), paint: TypedRect(0px×0px at (0px,0px)) }
│ │ │ │ │ │ │ damage=BUBBLE_ISIZES
│ │ │ │ │ │ │ └─ ↑↑ Fragment for block:
│ │ │ │ │ │ │ │ SpecificFragmentInfo::Table(2671) []
│ │ │ │ │ │ │ │ border_box=LogicalRect(H LTR, i0px×b0px, @ (i0px,b0px))
│ │ │ │ │ │ │ │ damage=REPOSITION | STORE_OVERFLOW | BUBBLE_ISIZES | REFLOW_OUT_OF_FLOW | REFLOW
│ │ │ │ ├─ Block(1268a9710)
│ │ │ │ │ sc=StackingContextId(0)
│ │ │ │ │ pos=LogicalRect(H LTR, i720px×b36px, @ (i152px,b2636.883333333333px))
│ │ │ │ │ floatspec-in=L 0px R 0px
│ │ │ │ │ floatspec-out=L 0px R 0px
│ │ │ │ │ overflow=Overflow { scroll: TypedRect(750px×36px at (-15px,0px)), paint: TypedRect(750px×36px at (-15px,0px)) }
│ │ │ │ │ children=1
│ │ │ │ │ damage=BUBBLE_ISIZES
│ │ │ │ │ ├─ ↑↑ Fragment for block:
│ │ │ │ │ │ SpecificFragmentInfo::Generic(2673) []
│ │ │ │ │ │ border_box=LogicalRect(H LTR, i720px×b36px, @ (i0px,b0px))
│ │ │ │ │ │ border_padding=LogicalMargin(H LTR, i:0px..0px b:6px..0px)
│ │ │ │ │ │ damage=REPOSITION | STORE_OVERFLOW
│ │ │ │ │ ├─ Block(128554f10)
│ │ │ │ │ │ sc=StackingContextId(0)
│ │ │ │ │ │ pos=LogicalRect(H LTR, i750px×b30px, @ (i0px,b6px))
│ │ │ │ │ │ floatspec-in=L 0px R 0px
│ │ │ │ │ │ floatspec-out=L 0px R 0px
│ │ │ │ │ │ overflow=Overflow { scroll: TypedRect(750px×30px at (-15px,0px)), paint: TypedRect(750px×30px at (-15px,0px)) }
│ │ │ │ │ │ children=6
│ │ │ │ │ │ damage=BUBBLE_ISIZES
│ │ │ │ │ │ ├─ ↑↑ Fragment for block:
│ │ │ │ │ │ │ SpecificFragmentInfo::Generic(2361) []
│ │ │ │ │ │ │ border_box=LogicalRect(H LTR, i750px×b30px, @ (i-15px,b0px))
│ │ │ │ │ │ │ margin=LogicalMargin(H LTR, i:-15px..-15px b:0px..0px)
│ │ │ │ │ │ │ damage=REPOSITION | STORE_OVERFLOW
│ │ │ │ │ │ ├─ TableWrapperFlow: Block(128555410)
│ │ │ │ │ │ │ sc=StackingContextId(0)
│ │ │ │ │ │ │ pos=LogicalRect(H LTR, i750px×b0px, @ (i-15px,b0px))
│ │ │ │ │ │ │ floatspec-in=L 0px R 0px
│ │ │ │ │ │ │ floatspec-out=L 0px R 0px
│ │ │ │ │ │ │ overflow=Overflow { scroll: TypedRect(0px×0px at (0px,0px)), paint: TypedRect(0px×0px at (0px,0px)) }
│ │ │ │ │ │ │ children=1
│ │ │ │ │ │ │ damage=BUBBLE_ISIZES
```
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12675 (github issue number if applicable).
<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____
<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->
<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19622)
<!-- Reviewable:end -->
… shinglyu:layout_json); r=glennw <!-- Please describe your changes on the following line: --> The second argument for the `emit_struct()` is the number of fields, if given `0`, the output JSON will always be empty. This is used in `./mach run -d -Z trace-layout https://servo.org`, which will dump the layout flow tree into a `layout_trace.json` file for debugging. This also unblocks servo/servo#12675 --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors <!-- Either: --> - [x] These changes do not require tests because its a debugging tool, not critical for normal code path <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 3993fde90acf08fd2ec423ce0d1f60f3a69a4b37 UltraBlame original commit: 42cc8b34d9bfc3c6800f1d647f2109251a25a2f8
… shinglyu:layout_json); r=glennw <!-- Please describe your changes on the following line: --> The second argument for the `emit_struct()` is the number of fields, if given `0`, the output JSON will always be empty. This is used in `./mach run -d -Z trace-layout https://servo.org`, which will dump the layout flow tree into a `layout_trace.json` file for debugging. This also unblocks servo/servo#12675 --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors <!-- Either: --> - [x] These changes do not require tests because its a debugging tool, not critical for normal code path <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 3993fde90acf08fd2ec423ce0d1f60f3a69a4b37 UltraBlame original commit: 42cc8b34d9bfc3c6800f1d647f2109251a25a2f8
… shinglyu:layout_json); r=glennw <!-- Please describe your changes on the following line: --> The second argument for the `emit_struct()` is the number of fields, if given `0`, the output JSON will always be empty. This is used in `./mach run -d -Z trace-layout https://servo.org`, which will dump the layout flow tree into a `layout_trace.json` file for debugging. This also unblocks servo/servo#12675 --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors <!-- Either: --> - [x] These changes do not require tests because its a debugging tool, not critical for normal code path <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 3993fde90acf08fd2ec423ce0d1f60f3a69a4b37 UltraBlame original commit: 42cc8b34d9bfc3c6800f1d647f2109251a25a2f8

When running
./mach run -Z dump-flow-tree, the output is not very human friendly:We can try to wrap the lines like
I'm not sure if we should hack the
Debug::fmttrait or create a post-processor. The problem withDebug::fmtis that it's hard to keep the indentation level. But doing a post process will require us to keep theDebug::fmtand the post postprocessor format in sync.