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

Space view scenes (parts) are now independent of space view type #2522

Closed
wants to merge 8 commits into from

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jun 26, 2023

.. but space view types are ofc still dependent on the parts collection.

Things happening here:

  • no longer pass SpaceViewState to population of SceneContext and SceneParts
  • SpaceViewClass no longer specifies the SceneContext and ScenePartData - this is now done by the ScenePartCollection instead

The first two changes allow us to use the same scene(part collection) for several space views, as it is needed by 2D & 3D space views which are right now the same type (but we want different types).

Checklist

PR Build Summary: https://build.rerun.io/pr/2522

Docs preview: https://rerun.io/preview/f25972d/docs
Examples preview: https://rerun.io/preview/f25972d/examples

@Wumpf Wumpf added 🪳 bug Something isn't working 🚜 refactor Change the code, not the functionality 📺 re_viewer affects re_viewer itself labels Jun 26, 2023
@Wumpf Wumpf marked this pull request as draft June 28, 2023 09:58
@Wumpf Wumpf changed the title Remove defunct text space view entity filter, space view scenes (parts) are now independent of space view type Space view scenes (parts) are now independent of space view type Jun 28, 2023
Wumpf added a commit that referenced this pull request Jun 29, 2023
<!--
Open the PR up as a draft until you feel it is ready for a proper
review.

Do not make PR:s from your own `main` branch, as that makes it difficult
for reviewers to add their own fixes.

Add any improvements to the branch as new commits to make it easier for
reviewers to follow the progress. All commits will be squashed to a
single commit once the PR is merged into `main`.

Make sure you mention any issues that this PR closes in the description,
as well as any other related issues.

To get an auto-generated PR description you can put "copilot:summary" or
"copilot:walkthrough" anywhere.
-->

### What

Noticed while testing: Removed text entity filter ui; this was already
defunct on main and is redundant to entity visibility property.

Was originally part of #2522 

Test via `python ./examples/python/text_logging/main.py`

### 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)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2544

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/8e1e06b/docs
Examples preview: https://rerun.io/preview/8e1e06b/examples
<!-- pr-link-docs:end -->
@Wumpf Wumpf force-pushed the andreas/space-view-class-independent-scenes branch from bdac16b to c6ec303 Compare June 29, 2023 08:54
emilk pushed a commit that referenced this pull request Jun 29, 2023
<!--
Open the PR up as a draft until you feel it is ready for a proper
review.

Do not make PR:s from your own `main` branch, as that makes it difficult
for reviewers to add their own fixes.

Add any improvements to the branch as new commits to make it easier for
reviewers to follow the progress. All commits will be squashed to a
single commit once the PR is merged into `main`.

Make sure you mention any issues that this PR closes in the description,
as well as any other related issues.

To get an auto-generated PR description you can put "copilot:summary" or
"copilot:walkthrough" anywhere.
-->

### What

Noticed while testing: Removed text entity filter ui; this was already
defunct on main and is redundant to entity visibility property.

Was originally part of #2522 

Test via `python ./examples/python/text_logging/main.py`

### 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)

<!-- This line will get updated when the PR build summary job finishes.
-->
PR Build Summary: https://build.rerun.io/pr/2544

<!-- pr-link-docs:start -->
Docs preview: https://rerun.io/preview/8e1e06b/docs
Examples preview: https://rerun.io/preview/8e1e06b/examples
<!-- pr-link-docs:end -->
Wumpf added a commit that referenced this pull request Jul 12, 2023
As discussed in todays call and detailed in
https://github.com/rerun-io/rerun/blob/andreas/space_views-rfc/design/space_views.md#viewpartsystem
(WIP rfc)

There's a whole bunch of consecutive renames arising from that, none of
which I brought up since they are much lower stakes: They all revolve
around the idea that "scene" doesn't make sense for the temporary
datastructure we build up every frame. That said, it is still around for
the moment and will be likely removed or renamed as well very soon.
Similarly, there's some follow-up to do in the docs (and the
aforementioned rfc document)

Obsoletes #2522 since it causes too many conflicts there - will
reimplement key ideas from that PR though, seeing a bit clearer by now
where we want to end up.

This 100% pure refactoring, no logic has been touched, only renaming and
moving things around.

Renames:
* ScenePart -> ViewPartSystem
* SceneContextPart -> ViewContextSystem
* ScenePartCollection -> ViewPartSystemCollection
* SceneContext -> ViewContext (todo: weird discrepancy to system
collection)
* SceneQuery -> ViewQuery
* derived types, mostly in SpatialSpaceView crate
* related type fields
* related argument names
* file renamings, flattening out of `scene` folder for
`SpatialSpaceView`

### What

### 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/2674) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2674)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fviewpartsystem-rename/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fviewpartsystem-rename/examples)
@Wumpf
Copy link
Member Author

Wumpf commented Jul 13, 2023

Superseeded by #2688

@Wumpf Wumpf closed this Jul 13, 2023
@Wumpf Wumpf deleted the andreas/space-view-class-independent-scenes branch July 13, 2023 07:03
Wumpf added a commit that referenced this pull request Jul 14, 2023
(Replaces  #2522)
Part of 
* #2649

For in-depth overview of how space views work now see #2533

Replaces custom user types for context and parts with typemap style
collections. These are filled every frame from a set of registered parts
& contexts. Registration happens on the `SpaceViewRegistry` where we
already register space view classes themselves.

Other rippling changes in overview:
* space view parts and context no longer have access to the space view's
state struct, this fully decouples them from any concrete space view
* space view part's data object is no longer strongly typed, but an
`&Any` instead, fullfills otherwise the same purpose
* `Scene`/`TypedScene` is gone now
* most of what was previously the scene buildup is now part of
`SpaceViewClass::ui`

For reviewing it's recommended to start with everything in the
`re_viewer_context` crate.


Overview of major trait entry points - see also #2533
<img width="1120" alt="image"
src="https://github.com/rerun-io/rerun/assets/1220815/ffdb1cdf-7efe-47a0-ac38-30262d770e69">




### What

### 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/2688) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2688)
- [Docs
preview](https://rerun.io/preview/pr%3Aandreas%2Fregistered-space-view-systems/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aandreas%2Fregistered-space-view-systems/examples)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working 📺 re_viewer affects re_viewer itself 🚜 refactor Change the code, not the functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant