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.

Sign up for GitHub

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

Jump to bottom

Fully migrate to DataQueryBlueprint #4311

Merged
merged 26 commits into from
Nov 29, 2023
Merged

Fully migrate to DataQueryBlueprint #4311

merged 26 commits into from
Nov 29, 2023

Conversation

jleibs
Copy link
Member

@jleibs jleibs commented Nov 22, 2023 •
edited by github-actions bot
Loading

What

This activates the new expression-based DataQueries and removes the usages of SpaceViewContents.
Also includes a very primitive query-expression editor.

Known issues

However, for the sake of reviewer sanity, that work will be broken out as a follow-on PR. This PR is going to be painful enough as it is.

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

@jleibs jleibs added 🟦 blueprint The data that defines our UI exclude from changelog PRs with this won't show up in CHANGELOG.md labels Nov 22, 2023
@jleibs jleibs force-pushed the jleibs/real_data_queries branch 2 times, most recently from e9b0397 to 67f996e Compare November 23, 2023 01:10
@jleibs jleibs mentioned this pull request Nov 28, 2023
Closed
11 tasks
@jleibs jleibs marked this pull request as ready for review November 28, 2023 21:18
@jleibs jleibs changed the title Work-in-progress: migrate to proper DataQueries Fully migrate to DataQueryBlueprint Nov 28, 2023
This was referenced Nov 28, 2023
Merged
Closed
Merged
Merged
@jleibs jleibs mentioned this pull request Nov 29, 2023
Closed
4 tasks
@teh-cmc teh-cmc self-requested a review November 29, 2023 08:08
teh-cmc
teh-cmc approved these changes Nov 29, 2023
View reviewed changes
Copy link
Member

@teh-cmc teh-cmc left a comment

Choose a reason for hiding this comment

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

Overall made sense though I kinda lost track at some point tbh, it's just so much :/

Do we have any kind of very high-level test suite somewhere that just injects queries and spaceviewcomponents into a store and checks that the resulting views make sense?

crates/re_viewport/src/space_view.rs Show resolved Hide resolved
rerun_py/src/python_bridge.rs Outdated Show resolved Hide resolved
crates/re_space_view/src/data_query_blueprint.rs Show resolved Hide resolved
crates/re_viewer/src/ui/selection_panel.rs Outdated Show resolved Hide resolved
Base automatically changed from jleibs/early_queries to main November 29, 2023 14:52
jleibs added a commit that referenced this pull request Nov 29, 2023
@jleibs
Move DataQueryResults into the ViewerContext (#4310)
ab1490b 
### What
- Part of the #4308 stack:
  - THIS PR: #4310
  - #4311
  - #4380
  - #4381

Rather than doing queries on-demand, we now do all the queries up-front
when creating the ViewerContext.

This keeps us from re-running duplicate queries, and also eliminates the
need to maintain a parallel implementation of resolve instead of just
looking up the query results.

Probably best reviewed commit-by-commit.

This already hits some snags related to ViewerContext lifecycle that
will hopefully be addressed in
#1325

### 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/4310) (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/4310)
- [Docs
preview](https://rerun.io/preview/2239f89de45fe0ac81fc5660ca13f3b95d7a8799/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/2239f89de45fe0ac81fc5660ca13f3b95d7a8799/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@jleibs jleibs merged commit 086b681 into main Nov 29, 2023
43 checks passed
@jleibs jleibs deleted the jleibs/real_data_queries branch November 29, 2023 16:47
jleibs added a commit that referenced this pull request Nov 29, 2023
@jleibs
Add support for exclusion expressions to the DataQueryBlueprint (#4380)
ecb8e87 
### What
- Part of the #4308 stack:
  - #4310
  - #4311
  - THIS PR: #4380
  - #4381
- Resolves: #4158

The query logic is as follows because it was simple and was sufficient
to mostly match our existing add/remove semantics. It basically
prioritizes exclusions over inclusions. Anything recursively excluded
will be pruned, even if explicitly included. We likely want to change
this behavior, but we can do so in a future PR.

Expands the query evaluator to support exclusions, and UI mechanism for
editing.

![image](https://github.com/rerun-io/rerun/assets/3312232/e35fba49-bbce-48da-8e29-03d0dbb6d985)

### 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 [app.rerun.io](https://app.rerun.io/pr/4380) (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/4380)
- [Docs
preview](https://rerun.io/preview/45acbc744dddc65ea6682d5c58755ed27e96db78/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/45acbc744dddc65ea6682d5c58755ed27e96db78/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
jleibs added a commit that referenced this pull request Nov 29, 2023
@jleibs
Wire the remove-entity buttons to query exclusions (#4381)
0ab1412 
### What
- Part of the #4308 stack:
  - #4310
  - #4311
  - #4380
  - THIS PR: #4381

Now that we have functioning query exclusions, wire them up to the
remove-entity buttons.

### 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 [app.rerun.io](https://app.rerun.io/pr/4381) (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/4381)
- [Docs
preview](https://rerun.io/preview/2d33bf3b63da8df3c49e1379ca2af17bd22fc75e/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/2d33bf3b63da8df3c49e1379ca2af17bd22fc75e/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
teh-cmc pushed a commit that referenced this pull request Nov 30, 2023
@jleibs @teh-cmc
Move DataQueryResults into the ViewerContext (#4310)
0ffa61c 
### What
- Part of the #4308 stack:
  - THIS PR: #4310
  - #4311
  - #4380
  - #4381

Rather than doing queries on-demand, we now do all the queries up-front
when creating the ViewerContext.

This keeps us from re-running duplicate queries, and also eliminates the
need to maintain a parallel implementation of resolve instead of just
looking up the query results.

Probably best reviewed commit-by-commit.

This already hits some snags related to ViewerContext lifecycle that
will hopefully be addressed in
#1325

### 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/4310) (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/4310)
- [Docs
preview](https://rerun.io/preview/2239f89de45fe0ac81fc5660ca13f3b95d7a8799/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/2239f89de45fe0ac81fc5660ca13f3b95d7a8799/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
teh-cmc pushed a commit that referenced this pull request Nov 30, 2023
@jleibs @teh-cmc
Fully migrate to DataQueryBlueprint (#4311)
c2217d0 
### What
- Part of the #4308 stack:
  - #4310
  - THIS PR: #4311
  - #4380
  - #4381

This activates the new expression-based DataQueries and removes the
usages of SpaceViewContents.
Also includes a very primitive query-expression editor.

### Known issues
- #4377 captures several issues, specifically:
- Add/Remove Entity logic is broken. (See:
#4381)
  - Heuristics run way too slowly for complex scenes

However, for the sake of reviewer sanity, that work will be broken out
as a follow-on PR. This PR is going to be painful enough as it is.

### 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/4311) (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/4311)
- [Docs
preview](https://rerun.io/preview/68b22fecfb162e692db791f53e2cdbce1969b53d/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/68b22fecfb162e692db791f53e2cdbce1969b53d/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
teh-cmc pushed a commit that referenced this pull request Nov 30, 2023
@jleibs @teh-cmc
Add support for exclusion expressions to the DataQueryBlueprint (#4380)
f090adc 
### What
- Part of the #4308 stack:
  - #4310
  - #4311
  - THIS PR: #4380
  - #4381
- Resolves: #4158

The query logic is as follows because it was simple and was sufficient
to mostly match our existing add/remove semantics. It basically
prioritizes exclusions over inclusions. Anything recursively excluded
will be pruned, even if explicitly included. We likely want to change
this behavior, but we can do so in a future PR.

Expands the query evaluator to support exclusions, and UI mechanism for
editing.

![image](https://github.com/rerun-io/rerun/assets/3312232/e35fba49-bbce-48da-8e29-03d0dbb6d985)

### 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 [app.rerun.io](https://app.rerun.io/pr/4380) (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/4380)
- [Docs
preview](https://rerun.io/preview/45acbc744dddc65ea6682d5c58755ed27e96db78/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/45acbc744dddc65ea6682d5c58755ed27e96db78/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
teh-cmc pushed a commit that referenced this pull request Nov 30, 2023
@jleibs @teh-cmc
Wire the remove-entity buttons to query exclusions (#4381)
5d8b399 
### What
- Part of the #4308 stack:
  - #4310
  - #4311
  - #4380
  - THIS PR: #4381

Now that we have functioning query exclusions, wire them up to the
remove-entity buttons.

### 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 [app.rerun.io](https://app.rerun.io/pr/4381) (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/4381)
- [Docs
preview](https://rerun.io/preview/2d33bf3b63da8df3c49e1379ca2af17bd22fc75e/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/2d33bf3b63da8df3c49e1379ca2af17bd22fc75e/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teh-cmc teh-cmc teh-cmc approved these changes

Assignees
No one assigned
Labels
🟦 blueprint The data that defines our UI exclude from changelog PRs with this won't show up in CHANGELOG.md
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jleibs @teh-cmc