ListItem 2.0 (part 4): only allocate space for property action buttons when needed
#6183
Conversation
crates/re_ui/src/list_item2/scope.rs
Outdated
| pub(crate) reserve_action_button_space: bool, | ||
|
|
||
| /// Track whether the action button space should be reserved. | ||
| should_reserve_action_button_space: bool, |
There was a problem hiding this comment.
I find it confusing that we have two bool for this.
The whole State is a bit confusing.
There are, I think, three categories of things in State:
- Fixed settings: Things set once
list_item_scopeand never changed - Frame accumulators: Things reset at the start of
list_item_scopeand written to by children, then stored to be used the next frame (e.g. desired column widths) - Memory: things we remember from previous frames
We should make these categories much clearer, e.g. by grouping them inside of State and commenting it
There was a problem hiding this comment.
Well, to me it made more sense to group things by functionality rather than accumulator vs. something else.
Also, your categories are not quite right. There are (currently) two things:
- Accumulators that saved to memory and read the next frame, and then reset again for accumulation.
- Fixed values set by the scope once (either from external sources or from the last-frame accumulated values).
Anyways, I'll try to clarify this. Maybe it can be split in two structs to make things more explicit.
There was a problem hiding this comment.
Okay, I ended up doing a pretty significant overhaul of the whole thing.
State is gone. Now we have a (private) LayoutStatistics struct that accumulates layout "wants" during the frame. It's stored in egui's memory against the scope id. Then we have the (public) LayoutInfo which contains the layout information + other things that are prepared once by the list_item_scope and provided read-only to the child list items. For convenience, LayoutInfo also provides the public API used to gather the information that ends up in LayoutStatistics.
TL;DR: the API for user code (aka ListItemContent impls) is cleaner, and the implementation is more correct and, I think, much less confusing.
abey79
force-pushed
the
antoine/li3-smart-columns
branch
from
cd2acf6
to
6228024
Compare
abey79
force-pushed
the
antoine/li4-no-act-btn
branch
from
8bd03cc
to
cd0d0ab
Compare
…6184) ### What This PR deploys `list_item2` to the visualizer and overrides UI currently used for timeseries spaces views. Unsurprisingly, contact with the real world required some adjustments: - It's useful to be able to mix and match `LabelContent` and `PropertyContent`. The following changes were needed for that to look nice: - Both contents have now a configurable `min_desired_width`. - `LabelContent` now has a `always_show_buttons` option that perma-displays the buttons. This makes it easy to emulate `PropertyContent`'s action button. This could be improved: #6203 - Now the left column width is computed based on the total width actually allocated by `ListItem` (as opposed to `ui.max_rect()` as seen from `list_item_scope`). This is more correct when the list is in a horizontal scroll area and fixes related visual glitches. This PR also fixes the ShapeMarker editor UI, but required a workaround due to `ui.max_rect()` behaving weird when the ComboBox menu expends due to its contents. - Part of #6075 - Follow up to #6183 - Fixes #4984 - Fixes #6205 <img width="207" alt="image" src="https://github.com/rerun-io/rerun/assets/49431240/60f85e2e-cc67-48a5-8331-e943c246453b"> <br/> ### 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/6184?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/6184?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/6184) - [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`.
What
☝🏻
This PR also slightly adjust the semantics of the
idparameter oflist_item_scope(). Now it just needs to be unique within the current UI. It also addresses review comments from #6161, #6174, and #6182.ListItem2.0 (part 3):PropertyContentcolumn auto-sizing #6182PropertyContent: do not allocate space for acton button if no action button is ever used in the scope #6179Note: the upper group of (nested) items (which makes use of action buttons) has no impact on the lower group, because both sub-groups are in a distinct
list_item_scope.Checklist
mainbuild: rerun.io/viewernightlybuild: rerun.io/viewerTo run all checks from
main, comment on the PR with@rerun-bot full-check.