Skip to content

Fix updateViewGroupOffline() so that it doesn't hang#1336

Merged
j9liu merged 2 commits intoCesiumGS:mainfrom
WorldscapeAI:fix-updateViewGroupOffline
Mar 31, 2026
Merged

Fix updateViewGroupOffline() so that it doesn't hang#1336
j9liu merged 2 commits intoCesiumGS:mainfrom
WorldscapeAI:fix-updateViewGroupOffline

Conversation

@ELeeScape
Copy link
Copy Markdown
Contributor

@ELeeScape ELeeScape commented Mar 30, 2026

Description

Tileset::updateViewGroupOffline() was hanging because it calls updateViewGroup() and loadTiles() in a loop without dispatching main thread tasks. This was a regression resulting from #1315.

Added main thread dispatches to updateViewGroupOffline() before every call to updateViewGroup() and loadTiles() and added a test to demonstrate the fix.

Also made the same fix for the deprecated updateView() function. There are no existing tests for updateView() so I didn't add one.

Issue number or link

Fixes #1335.

Author checklist

  • I have submitted a Contributor License Agreement (only needed once).
  • I have done a full self-review of my code.
  • [] I have updated CHANGES.md with a short summary of my change (for user-facing changes).
  • I have added or updated unit tests to ensure consistent code coverage as necessary.
  • I have updated the documentation as necessary.

Testing plan

The new test subcase "updateViewGroupOffline does its own dispatching of main thread tasks so it doesn't get stuck" hangs without the fix, passes with the fix.

Reviewer checklist

Thank you for taking the time to review this PR. By approving a PR you are taking as much responsibility for these changes as the author.

As you review, please go through the checklist below:

  • Review and run all parts of the test plan on this branch and verify it matches expectations.
    • If the issue is a bug please make sure you can reproduce the bug in the main branch and then checkout this branch to make sure it actually solved the issue.
  • Review the code and make sure you do not have any remaining questions or concerns. You should understand the code change and the chosen approach. If you are not confident or have doubts about the code, please do not hesitate to ask questions.
  • Review the unit tests and make sure there are no missing tests or edge cases.
  • Review documentation changes and updates to CHANGES.md to make sure they accurately cover the work in this PR.
  • Verify that the Contributor License Agreement has been submitted, if needed.

@j9liu j9liu self-requested a review March 30, 2026 22:04
Copy link
Copy Markdown
Contributor

@j9liu j9liu left a comment

Choose a reason for hiding this comment

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

Thank you so much @ELeeScape! Can I trouble you to format the TestTilesetSelectionAlgorithm.cpp class for CI to pass? You can do this with npm by doing:

npm install
npm run format

Otherwise, these changes look great! I'm just reproducing the error locally to confirm the fix (per our reviewer checklist).

@ELeeScape
Copy link
Copy Markdown
Contributor Author

@j9liu Formatting done.

@j9liu
Copy link
Copy Markdown
Contributor

j9liu commented Mar 31, 2026

Thank you so much @ELeeScape! 🙏

@j9liu j9liu merged commit 65be492 into CesiumGS:main Mar 31, 2026
27 of 28 checks passed
@j9liu j9liu added this to the April 2026 Release milestone Mar 31, 2026
@ELeeScape ELeeScape deleted the fix-updateViewGroupOffline branch March 31, 2026 18:16
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.

Main thread dispatch change (#1315) causes updateViewGroupOffline to hang

2 participants