-
Notifications
You must be signed in to change notification settings - Fork 7
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
Accessor cache using CeslumAsync::SqliteCache #666
Conversation
No UI for maxiumum cache items yet.
I did some benchmarking with this USDA results: First load, clear cache: 74 seconds, 116mb downloaded Appears to be working as expected. I do notice a rather large hang (~10-20 seconds) when I hit Reload all Tilesets in Cesium Debugging UI (this is what I'm doing to trigger a reload). This isn't unique to this branch, and I suspect it has something to do with unloading all of the tileset resources before it begins reloaded. Point is, if we ignore the hang, the actual loading of the cached data goes down to ~24 seconds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The caching functionality is working great, I just have some comments about cache location and settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The powertools extension is for our internal use and isn't included in the release zip, so if we expect users to be able to modify the cache settings it needs to move into the main cesium.omniverse
extension. @r-veenstra would you have a recommended spot there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@r-veenstra bump
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timoore @lilleyse I'm thinking a small settings button could live here:
To get what i have in my image, replace line 191 in main_window.py
with below
with ui.HStack(height=24, spacing=3):
self._profile_widget = CesiumOmniverseProfileWidget(self._cesium_omniverse_interface, height=20)
self._add_button = ui.Button(
"",
image_url=f"{self._icon_path}/FontAwesome/plus-solid.png",
style=CesiumOmniverseUiStyles.setting_button_style,
enabled=True,
width=24
)
And add this to styles.py
setting_button_style = {
"Button": {"padding": 0.0, "stack_direction": Direction.TOP_TO_BOTTOM, "margin": 0},
"Button.Image": {
"alignment": Alignment.CENTER,
},
"Button.Label": {"alignment": Alignment.CENTER_BOTTOM},
"Button.Image:disabled": {"color": cl("#808080")},
"Button.Label:disabled": {"color": cl("#808080")},
}
The FA gear icon would be ok to use I think https://fontawesome.com/icons/gear?f=classic&s=solid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no problem with adding a settings button up there and having the cache parameters window evolve into a settings window. But are settings any less important than "Token", "Learn", and "Help"? What do you think about moving "Token" to the same settings window as the cache, and / or combining Learn, Help, and Settings into an "other" menu?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timoore I think we'd want to keep Token settings separate, as the are configured and stored per USD. I think we want to keep persistent extension settings separate.
It also introduces UI inconsistency across our runtimes, where the goal has been to keep the UI similar across Unity / Unreal / Omniverse.
You know, I just had another (and probably easier) thought. Let's just add another entry here:
Something like Cesium Settings or Cesium Extension Settings.
That way we don't need to touch any existing UI.
@timoore could you update |
FilesystemUtil has functions for getting the user's home directory and the preferred directory for a cache. Change Context setup code to use these new functions.
I added this to |
The window is accessable from the same menu as the other Cesium windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks @timoore
Addresses #650.
This builds on #634, which needs to be merged first.