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

ComponentFallbackProvider and greatly simplified edit_ui #6465

Merged
merged 24 commits into from
May 31, 2024

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented May 30, 2024

What

Two major intertwined changes:

  • ComponentFallbackProvider replaces initial_override_value (motivation see ticket)
  • use new default mechanism in edit_ui, allowing us to introduce strongly typed ui that always iterates on a present value which simplified existing edit_ui implementations a lot

Things to be iterated on:

  • edit_ui needs to be extended to deal with some form of layouting again - for and foremost we need to be able to generate list items (-> distinguish between being part of a list item and generating its own)
    • a bit unclear what happens if we move towards making almost everything an edit ui (just disabled in some cases). What's the future of UiLayout in that case? cc: @abey79
  • QueryContext has a bold name but is only used on the fallback provider right now and lacks some context that we'll need for upcoming fallback provider impls (view state etc.!)
    All of these will end up being part of
  • Make all queries in all visualizers aware of blueprint DataResult #6435

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 the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@Wumpf Wumpf added ui concerns graphical user interface 🚜 refactor Change the code, not the functionality exclude from changelog PRs with this won't show up in CHANGELOG.md labels May 30, 2024
@Wumpf Wumpf marked this pull request as ready for review May 30, 2024 15:38
@Wumpf Wumpf requested a review from jleibs May 31, 2024 06:54
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Nice! Code LGTM, but I haven't tested clicking around the UI (yet)

crates/re_edit_ui/src/corner2d.rs Outdated Show resolved Hide resolved
crates/re_edit_ui/src/lib.rs Show resolved Hide resolved
Comment on lines +46 to +48
value: &mut ScalarScattering,
) -> egui::Response {
let scattered_text = if value.0 { "Scattered" } else { "Line" };
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to this PR, but this highlights that ScalarScattering should probably been an enum instead (it was created before we had enum support in codegen)

Copy link
Member Author

Choose a reason for hiding this comment

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

👍
worth breaking the api (now is a good time!) taking it in a followup!

Copy link
Member Author

Choose a reason for hiding this comment

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

oh wait that thing is so old, we stopped using it :O

crates/re_edit_ui/src/lib.rs Outdated Show resolved Hide resolved
crates/re_edit_ui/src/marker_shape.rs Outdated Show resolved Hide resolved
crates/re_edit_ui/src/marker_shape.rs Outdated Show resolved Hide resolved
crates/re_ui/src/lib.rs Outdated Show resolved Hide resolved
crates/re_viewer_context/src/component_fallbacks.rs Outdated Show resolved Hide resolved
crates/re_viewer_context/src/component_ui_registry.rs Outdated Show resolved Hide resolved
@Wumpf Wumpf merged commit 707b037 into main May 31, 2024
34 checks passed
@Wumpf Wumpf deleted the andreas/fallback-provider branch May 31, 2024 09:35
Wumpf added a commit that referenced this pull request Jun 3, 2024
### What

... and use that to further simplify edit ui using this (that part is a
refactor though, the title line is the user facing change here :))

* this addresses a comment on #6465

### 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 the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6470?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6470?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6470)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 🚜 refactor Change the code, not the functionality ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement initial_override_value with a "FallbackProvider" trait which can be used for edit_ui
3 participants