Wire the remove-entity buttons to query exclusions #4381
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jleibs
force-pushed
the
jleibs/remove_entities
branch
from
04db64b
to
da82cfc
Compare
jleibs
added
exclude from changelog
This was referenced Nov 29, 2023
jleibs
force-pushed
the
jleibs/query_exclusions
branch
from
9881959
to
b815a02
Compare
jleibs
force-pushed
the
jleibs/remove_entities
branch
from
bc874a4
to
e50352e
Compare
This was referenced Nov 29, 2023
jleibs
force-pushed
the
jleibs/query_exclusions
branch
from
b815a02
to
ed32043
Compare
jleibs
force-pushed
the
jleibs/remove_entities
branch
from
e50352e
to
347c792
Compare
Wumpf
approved these changes
Nov 29, 2023
Comment on lines
+104
to
+110
| let mut inclusions: Vec<EntityPathExpr> = self | ||
| .expressions | ||
| .inclusions | ||
| .iter() | ||
| .filter(|exp| !exp.as_str().is_empty()) | ||
| .map(|exp| EntityPathExpr::from(exp.as_str())) | ||
| .collect(); |
There was a problem hiding this comment.
nit: gives me deja vu. Maybe you want a utility to collect non-empty exclusions/inclusions
jleibs
added a commit
that referenced
this pull request
Nov 29, 2023
### 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
added a commit
that referenced
this pull request
Nov 29, 2023
### 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)
jleibs
force-pushed
the
jleibs/query_exclusions
branch
from
ed32043
to
45acbc7
Compare
jleibs
added a commit
that referenced
this pull request
Nov 29, 2023
### 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.  ### 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
force-pushed
the
jleibs/remove_entities
branch
from
347c792
to
4841164
Compare
teh-cmc
pushed a commit
that referenced
this pull request
Nov 30, 2023
### 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
### 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
### 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.  ### 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
### 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Now that we have functioning query exclusions, wire them up to the remove-entity buttons.
Checklist