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

Remove clip rect hack from every widget #6156

Closed
abey79 opened this issue Apr 29, 2024 · 1 comment · Fixed by #6248
Closed

Remove clip rect hack from every widget #6156

abey79 opened this issue Apr 29, 2024 · 1 comment · Fixed by #6248
Assignees
Labels
ui concerns graphical user interface

Comments

@abey79
Copy link
Member

abey79 commented Apr 29, 2024

As part of the #6075 effort, the reliance on clip rect for full-span highlighting will be removed:

However, other widget of ours rely on clip rect for rendering. That should be addressed, including:

  • full_span_separator
  • list_item_popup
  • panel_title_bar_with_buttons
  • large_collapsing_header

My plan to address that is to "fork" the background_x_range stuff from list_item_scope and create a new full_span_scope that sets some egui-wide state to be used by all full-span widgets.

@abey79 abey79 added the ui concerns graphical user interface label Apr 29, 2024
abey79 added a commit that referenced this issue May 2, 2024
…elContent` legacy back-port (#6161)

### What

This PR does the following:
- Introduces the fundamental content-generic `ListItem` infrastructure
(`ListItem`, `trait ListItemContent`, `list_item_scope()`.
- Introduces `LabelContent`, a `ListItemContent` implementation which
implements the exact same features as the legacy `ListItem`.
- Updates `re_ui_example` to demonstrate the use of the new list item,
including a fairly extensive clean-up of the right panel code.

<img width="411" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/dcd960d8-2fd1-48ed-ba5b-6f36bd35c65c">
 <br/> <br/>


- Part of #6075 
- Follow-up to #6148
- Fixes #5740

### Limitation and todos

- The handling of the X coordinate range for the background highlight
needs (introduced here to part with the clip rect hack) needs splitting
of to include _all_ full-span widgets: #6156.
- The state management currently looks meaningless as state will only be
used by the future `PropertyContent`. Funnily, all the state management
currently does is what is to be split off as per above :)
- Docstrings needs more work (in particular top-level overview)
- `ListItem` + `LabelContent` should be deployed wherever we currently
use `ListItem` 1.0, which should be then entirely removed.
- And of course, we need a two-column `PropertyContent`…

### 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/6161?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/6161?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/6161)
- [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`.
@abey79
Copy link
Member Author

abey79 commented May 3, 2024

Much easier after #6211, but maybe way for 0.16 to be released?

@abey79 abey79 self-assigned this May 6, 2024
abey79 added a commit that referenced this issue May 6, 2024
…ted module (#6211)

### What

So far, the full span x coordinate rang needed for highlighting was
maintained by `LayoutInfo` and `list_item_scope`. However, other widgets
also need this mechanism. This PR splits of full-span management to a
dedicated module, and deploys it both in `re_ui_example` and where
`list_item2` is currently used.

Other changes:
- This PR also improves the sanity checks regarding both
`list_item_scope` and `full_span_scope`. When missing, debug build will
panic and release build will emit warnings.
- Both scopes are now `ui.scope()`, so it's safe to modify `ui.style()`
from the closure.
- `list_item_scope` now sets `ui.spacing().item_spacing.y = 0`, since
this is required anyway for `ListItem`s to look good.

Follow-up PR will be needed to deploy `full_span` to all relevant
widgets and their use in the viewer (#6156).

- Part of #6075 
- Part of #6156
- Follow-up to #6184

### 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/6211?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/6211?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/6211)
- [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`.
abey79 added a commit that referenced this issue May 8, 2024
### What

Migrate all widgets to the "full span scope" mechanism introduced in
#6211, including the legacy `ListItem`. This completes the migration
away from the clip rect hack, but does highlight a number of issues and
improvements:
- #6245 
- #6246

Showcase of `full_span_scope()` nestability:

<img width="324" alt="image"
src="https://github.com/rerun-io/rerun/assets/49431240/be8e72b1-7f1d-4533-a456-870a8c51572d">
<br/> <br/>


- Fixes #6156

### 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/6248?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/6248?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/6248)
- [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
ui concerns graphical user interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant