simulate: fix display of loading message #1070
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
yuvaltassa
reviewed
Sep 25, 2023
| @@ -173,6 +181,7 @@ class Simulate { | |||
| std::atomic_int uiloadrequest = 0; | |||
|
|
|||
| // loadrequest | |||
| // 3: display a loading message | |||
There was a problem hiding this comment.
Why do we need "3" Isn't this already what "1" does (did 🙂) ?
There was a problem hiding this comment.
There are two stages to the loading process. Transitioning from state 3 to 2 is handled by the physics thread (this is optional for the user). Transitioning from 2 to 0 is handled by the UI thread.
The UI thread needs two states so it can put a loading message on the screen (transition from 2 to 1) prior to the blocking call to Simulate's LoadOnRenderThread (transition from 1 to 0).
|
Can you resync please so the tests pass? |
aftersomemath
force-pushed
the
simulate-load-message
branch
from
13c37e4
to
9b00d3e
Compare
aftersomemath
force-pushed
the
simulate-load-message
branch
from
9b00d3e
to
e7d1f2f
Compare
|
For sure, it is done. |
yuvaltassa
approved these changes
Sep 27, 2023
|
@aftersomemath thanks so much ❤️ |
|
You are welcome! |
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.
Model loading was moved to the physics thread sometime back (for good reason). This broke the "loading" message shown in UI because the UI did not a know a model was being loaded until after the fact.
The proposed solution adds optional methods to the Simulate class that set and clear a loading message as part of the UI's model loading state machine.
This works with C++ simulate application and the Python managed viewer. It should not affect the passive viewer.
EDIT: It also moves the loading message for command line based reloads to the left of the screen. Previously it was on the right for command line filenames and the left for UI based reloads (button or drag-n-drop) which is inconsistent.