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

Change DataSourceDisplay ready state behavior for bounding sphere calculations #12429

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jjspace
Copy link
Contributor

@jjspace jjspace commented Jan 10, 2025

Description

Slightly reverting the change in #12230 and replacing PR #12322 in favor of changing the ready value to fix both #4647 and the newer issue #12303.

The core of this change is to make the DataSourceDisplay._ready boolean to remain true once it's set to true even if the internal datasources become "un-ready" due to transformations or loading.

Issue number and link

Alternative fix for #4647
Fixes #12303

Testing plan

  • Test this sandcastle which demonstrates the error when setting the viewer.selectedEntity
  • Test this sandcastle to test the original issue of a "flickering" entity when it moves (it should not flicker)
  • Test some other sandcastles that pertain to entities and selections to make sure selection and tracking still works as expected

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

@jjspace jjspace requested a review from ggetz January 10, 2025 20:01
Copy link

Thank you for the pull request, @jjspace!

✅ We can confirm we have a CLA on file for you.

CHANGES.md Show resolved Hide resolved
@jfayot
Copy link
Contributor

jfayot commented Jan 10, 2025

That looks good to me @jjspace ! Having different scratch objects seems so obvious now that I see it... Thanks

@jjspace
Copy link
Contributor Author

jjspace commented Jan 10, 2025

Awesome, thanks for taking a look @jfayot. The primary thing that helped was making sure the ready state stays true because it was flipping between ready and not ready as entities moved which caused the flicker before. The separate scratch variables is more of a sanity thing. They were interacting really weirdly because of how it's passed into and referenced in the EntityView and used in multiple locations that made it very hard to debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError on setting the Viewer selectedEntity
3 participants