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 script to generate RRD vs. screenshots comparisons #3946

Merged
merged 10 commits into from Oct 23, 2023

Conversation

abey79
Copy link
Contributor

@abey79 abey79 commented Oct 20, 2023

What

This PR introduces a script that builds a local, web-based comparison between our various example code and the screenshot we provide along them, to help identify discrepancies.

❯ python scripts/screenshot_compare/build_screenshot_compare.py --help
usage: build_screenshot_compare.py [-h] [--serve] [--skip-build]
                                   [--skip-example-build]

Generate comparison between examples and their related screenshots.

This script builds/gather RRDs and corresponding screenshots and displays
them side-by-side. It pulls from the following sources:

- The screenshots listed in .fbs files (crates/re_types/definitions/rerun/**/*.fbs),
  and the corresponding code examples in the docs (docs/code-examples/*.rs)
- The `demo.rerun.io` examples, as built by the `build_demo_app.py` script.

The comparison is generated in the `compare_screenshot` directory. Use the `--serve`
option to show them in a browser.

options:
  -h, --help            show this help message and exit
  --serve               Serve the app on this port after building [default:
                        8080]
  --skip-build          Skip building the Python SDK and web viewer Wasm.
  --skip-example-build  Skip building the RRDs.

image

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@abey79 abey79 added 🧑‍💻 dev experience developer experience (excluding CI) include in changelog labels Oct 20, 2023
Copy link
Member

@jleibs jleibs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than copying all of the index business it seems like this could potentially be simplified by using a iframe pointed at app.rerun.io.

@abey79
Copy link
Contributor Author

abey79 commented Oct 20, 2023

Rather than copying all of the index business it seems like this could potentially be simplified by using a iframe pointed at app.rerun.io.

Yeah I took all possible shortcuts here. I've copied what build_demo_app.py does without modification (nor understanding it fully tbh). I just tried to have app.rerun.io load a RRD from the local server without success—not sure what's going on. Also, I believe this script has more value by running a viewer built from main since it would typically be used during the doc update in prep of a coming release.

@teh-cmc
Copy link
Member

teh-cmc commented Oct 23, 2023

Agree with @jleibs, copying all the index stuff is a bit too much.

I just tried to have app.rerun.io load a RRD from the local server without success

Not sure what you mean by "from the local server" -- the web app can load rrd files from the local disk, can't it?
EDIT: No it can't, not without going through the file dialog first.

Also, I believe this script has more value by running a viewer built from main since it would typically be used during the doc update in prep of a coming release.

We generate a hosted app.rerun.io for each PR commit (https://app.rerun.io/commit/{short_git_hash}) so an iframe would actually work great here (just pick the one from the release PR and you're set).

Copy link
Member

@jleibs jleibs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works for me. Thanks for cleaning up to avoid the duplication:

The only confusion is when it runs the log messages:

Serving examples at http://127.0.0.1:8080/

Press enter to reload static files
[2023-10-23T15:42:49Z INFO  re_ws_comms::server] Listening for WebSocket traffic on ws://localhost:9877. Connect with a Rerun Web Viewer.
[2023-10-23T15:42:49Z INFO  re_web_viewer_server] Started web server on http://localhost:9090
[2023-10-23T15:42:49Z INFO  re_sdk::web_viewer] Web server is running - view it at http://localhost:9090?url=ws://localhost:9877

Make it a bit confusing to know which url to use.

scripts/screenshot_compare/build_screenshot_compare.py Outdated Show resolved Hide resolved
@abey79
Copy link
Contributor Author

abey79 commented Oct 23, 2023

The only confusion is when it runs the log messages:

Indeed. I've changed it to use a sub-process and silence the warning via the environment variables.

@abey79 abey79 merged commit 14206ed into main Oct 23, 2023
32 checks passed
@abey79 abey79 deleted the antoine/screenshot_compare branch October 23, 2023 19:58
abey79 added a commit that referenced this pull request Oct 24, 2023
### What

Updated a bunch of example and/or screenshot based on discrepancies
identified with #3946. In the process, several examples were brought to
their previous state (higher resolution, etc.), including depth image,
etc.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] ~~I've included a screenshot or gif (if applicable)~~
* [x] ~~I have tested [demo.rerun.io](https://demo.rerun.io/pr/3954) (if
applicable)~~
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/3954)
- [Docs
preview](https://rerun.io/preview/842a10be1377e241915731431235d64f5e8624c5/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/842a10be1377e241915731431235d64f5e8624c5/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI) include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants