Skip to content

Feature/property tree topic#195

Open
ylvaselling wants to merge 43 commits intomasterfrom
feature/property-tree-topic
Open

Feature/property tree topic#195
ylvaselling wants to merge 43 commits intomasterfrom
feature/property-tree-topic

Conversation

@ylvaselling
Copy link
Collaborator

@ylvaselling ylvaselling commented Jan 30, 2026

A bunch of performance optimizations regarding the data passing to and from the UI with the purpose of doing a long-term solve for the UI meltdown.

So for testing, try and make the meltdown happen!

  • Moved the property and propertyOwners to Redux entityAdapters. This is because we were pretty much implementing that anyway, but now we can instead rely on RTK and get some nice selectors generated for us.
    • Batch dispatching (upsertMany) instead of multiple dispatches for updating the store (upsertOne in a loop) has a huge impact on startup performance.
  • Created a visibility state in the local slice to track which scene graph nodes are currently visible. This updates whenever a new property value is set for the fade or enable for that scene graph node.
  • Batching updates from properties with a new PropertyBatcher class. This essentially functions as a throttle, but without the risk of missing values.
  • Throttling updates that are streaming, such as the time and the camera.
  • Topic for properties are now sent as one topic to rule them all, rather than individual topic subscriptions. This means less overhead + we get to have a reliable copy of the property tree in Redux.
    • Removed all property subscriptions
  • Also made a radial sweep icon for the checkboxes when fading because it is ✨ fancy ✨ and also so multiple instances of the UI display the same thing (before they didn't, when you checked a scene graph node icon).
  • Added a new hook for property value, and changed useProperty to that when applicable (possible because we now don't need to subscribe to properties)
  • Also did some refactoring of the property tree helpers and hooks to help distinguish small uri-functions from functions that do more logic

To be used with engine PR

Some future work:

  • There are some places where all properties are extracted from a useAppSelector. This is suboptimal as whenever any property changes, this will trigger a re-render. I started to look into this but this PR is big enough as it is. Also I think it will require a big refactor of the scene tree hooks. To extract all propertyOwners is not as bad, as the propertyOwner state doesn't change that often.
  • We probably could optimize the startup of the topics with more granular control, for example by moving the subscribing of the topics to the connectionMiddleware instead of the middlewares for the topics. Now, all of them fire on onConnection, but we don't have a lot of control over which fire first. This would help to make the UI feel way snappier on startup, probably by first getting the property tree as it basically freezes everything for a bit.

Recommended reading: https://redux-toolkit.js.org/api/createEntityAdapter

Edit:

  • Use batching for the time instead of throttling as we were missing values
    • PropertyBatcher is therefore renamed to Batcher

@ylvaselling ylvaselling marked this pull request as ready for review January 30, 2026 15:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the property tree data model and subscriptions to improve UI performance and consistency, introducing normalized Redux entity adapters, a batched property update topic, and derived visibility state for scene graph nodes.

Changes:

  • Introduces propertyTree slices using RTK createEntityAdapter for properties and property owners, plus a new propertyTree topic and PropertyBatcher to batch property updates into updateMany calls.
  • Centralizes URI and property-tree helper logic into src/util/uris.ts, propertyTreeHelpers, and propertyTreeSelectors, and rewires hooks/components (usePropertyValue, property-owner visibility, scene tree, settings, navigation, night sky, etc.) to consume the new structure and visibility state.
  • Adds throttling for high-frequency topics (time and camera) and simplifies property access hooks to read from Redux rather than per-property subscriptions.

Reviewed changes

Copilot reviewed 71 out of 71 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/util/uris.ts New centralized URI helpers for scene graph nodes, renderables, enabled/fade properties, globe layers, and transformations to support consistent URI handling across the app.
src/util/propertyTreeSelectors.ts New selectors for computing property-owner visibility and whether a property owner has visible children, based on the normalized properties/owners structures.
src/util/propertyTreeHooks.ts Updates hooks for featured, anchor, and aim nodes to use entity selectors and the new URI helpers; note that useFeaturedNodes now maps marked IDs to owners via sgnUri.
src/util/propertyTreeHelpers.ts Simplifies property-tree helpers to focus on visibility computation and displayName, introducing Visibility-returning checkVisibilityTest while keeping boolean checkVisibility.
src/util/networkingHooks.ts Switches useWebGuiUrl to use usePropertyValue instead of useProperty, aligning with the new property access pattern.
src/types/types.ts Adds a Visibility string union, redefines Properties/PropertyOwners as non-optional maps, and removes the old PropertyOverview map in favor of using meta data directly.
src/redux/time/timeMiddleware.ts Wraps time topic updates in a 500ms lodash.throttle to reduce dispatch frequency while keeping UI-refresh behavior.
src/redux/store.ts Replaces legacy propertytree reducers with new propertyTree/propertyOwnerSlice and propertyTree/propertySlice reducers in the root store configuration.
src/redux/sessionrecording/sessionRecordingMiddleware.ts Uses propertySelectors plus the new setPropertyValue action to control GUI-related properties during session playback; removes direct dependency on the old properties slice.
src/redux/propertytree/propertyowner/propertyOwnerSlice.ts Deletes the old hand-rolled PropertyOwnersState slice in favor of the new entity-adapter-based implementation.
src/redux/propertytree/propertyTreeMiddleware.ts Deletes the old property-tree middleware that flattened owners/properties and managed per-URI propertyTree reload/remove actions.
src/redux/propertytree/properties/types.ts Removes the old PropertyPayload type used by the previous per-property subscription middleware.
src/redux/propertytree/properties/propertiesSlice.ts Removes the legacy properties slice (including propertyOverview maintenance) in favor of normalized entities.
src/redux/propertytree/properties/propertiesMiddleware.ts Removes the old per-property subscription middleware (subscribe/unsubscribe logic and throttled property updates).
src/redux/propertyTree/propertyTreeMiddleware.ts New middleware that loads the full property tree via getRoot, subscribes to a unified propertyTree topic (batched by PropertyBatcher), normalizes entities, computes scene-graph-node visibility, and exposes addUriToPropertyTree, removeUriFromPropertyTree, and setPropertyValue.
src/redux/propertyTree/propertySlice.ts New properties slice using createEntityAdapter keyed by property URI with sorted IDs and standard CRUD reducers/selectors.
src/redux/propertyTree/propertyOwnerSlice.ts New propertyOwners slice using createEntityAdapter, with custom reducers to upsert owners, maintain parent–subowner links, and recursively remove owners plus their flattened children.
src/redux/propertyTree/propertyBatcher.ts New PropertyBatcher class that buffers Update<AnyProperty, Uri> records and flushes them after a short delay to a provided update function (used to batch updateMany dispatches).
src/redux/local/localSlice.ts Extends local state with a sceneTree.visibility map keyed by SGN URI and adds setSceneGraphNodesVisibility/setVisibility reducers to track visibility state.
src/redux/local/localMiddleware.ts Adds listeners that watch property updates for SGN Enabled/Fade properties, derive Visibility via checkVisibilityTest, and maintain sceneTree.visibility; also introduces an internal setVisibilityForUri action.
src/redux/listenerMiddleware.ts Updates listeners to remove the old properties/propertyTree middlewares and register the new propertyTree middleware in the correct order.
src/redux/groups/util.ts Adjusts computeGroups to operate on a PropertyOwner[] plus normalized Properties, using isSceneGraphNode from uris.ts.
src/redux/groups/groupsSliceMiddleware.ts Reworks groups middleware to pull owners/properties from the entity adapters, recompute groups/tags on addUriToPropertyTree.fulfilled, and use the new PropertyOwner-array signature.
src/redux/events/eventsMiddleware.ts Points property-tree add/remove imports to the new propertyTree middleware path.
src/redux/camera/cameraMiddleware.ts Wraps camera topic updates in a 500ms lodash.throttle to reduce dispatch load on high-frequency camera updates.
src/panels/SkyBrowserPanel/WorldWideTelescope/hooks.ts Switches inverse-zoom property access to usePropertyValue.
src/panels/SkyBrowserPanel/WorldWideTelescope/WwtProvider/WwtProvider.tsx Uses usePropertyValue for the WWT image collection URL while preserving connection logic.
src/panels/SettingsPanel/SettingsSearchListItem.tsx Uses removeLastWordFromUri from the new uris helper module.
src/panels/SettingsPanel/SettingsPanel.tsx Refactors settings search to use property/owner selectors, the new visibility helpers, and inlined meta data (metaData.guiName and metaData.visibility) instead of a separate propertyOverview map.
src/panels/SessionRecordingPanel/PlaySession.tsx Removes explicit useSubscribeToProperty calls for GUI properties, relying on the new global property-tree topic instead.
src/panels/Scene/hooks.ts Uses usePropertyValue for time-frame membership checks while still using usePropertyOwner for TimeFrame subowners.
src/panels/Scene/SceneTree/hooks.ts Adapts scene tree data assembly to the normalized property/owner selectors and the new sceneTree.visibility filter for “show only visible” nodes.
src/panels/Scene/SceneTree/SceneTree.tsx Passes the tree instance into SceneTreeNodeContent for search results, and otherwise keeps tree usage aligned with the updated hooks.
src/panels/Scene/SceneGraphNode/SceneGraphNodeView.tsx Switches isRenderable/isSgnTransform imports to the new uris utilities.
src/panels/Scene/SceneGraphNode/SceneGraphNodeMoreMenu.tsx Uses displayName from propertyTreeHelpers and identifierFromUri from the new uris helper.
src/panels/Scene/SceneGraphNode/SceneGraphNodeMetaInfo.tsx Uses usePropertyValue for GuiPath/GuiDescription, adjusts URI helper imports, and keeps description cleaning logic intact.
src/panels/Scene/SceneGraphNode/SceneGraphNodeHeader.tsx Integrates the new visibility model by using useSceneGraphNodeVisibility and useSetPropertyOwnerVisibility, and wires these into PropertyOwnerVisibilityCheckbox.
src/panels/Scene/Scene.tsx Checks whether the scene has loaded using propertyOwnerSelectors.selectIds instead of the old state.propertyOwners.propertyOwners map.
src/panels/NightSkyPanel/tabs/SunTab.tsx Uses property-owner entities selector for sun-related nodes and sgnUri from uris.ts.
src/panels/NightSkyPanel/tabs/StarsTab.tsx Adds usePropertyOwnerVisibility for stars and label visibility checkboxes and passes the visibility/setter into PropertyOwnerVisibilityCheckbox.
src/panels/NightSkyPanel/tabs/MarkingsTab/MarkingBox.tsx Uses usePropertyOwnerVisibility plus uris.ts helpers for markings visibility and URIs.
src/panels/NightSkyPanel/tabs/MarkingsTab/ConstellationsShowLinesBox.tsx Switches to uris.ts helpers and uses visibility from usePropertyOwnerVisibility to gate the “show lines” checkbox.
src/panels/NightSkyPanel/tabs/MarkingsTab/ConstellationsShowArtBox.tsx Adjusts URI helpers import to use the new uris module.
src/panels/NightSkyPanel/tabs/MarkingsTab/CardinalDirectionsBox.tsx Uses usePropertyValue and the new visibility-based API from usePropertyOwnerVisibility to compute whether a given cardinal-direction variant is active.
src/panels/NavigationPanel/RemainingFlightTimeIndicator.tsx Uses sgnUri from uris.ts for flight-time calculations.
src/panels/NavigationPanel/NavigationPanel.tsx Reads navigation anchor/aim properties via propertySelectors and uses useFeaturedNodes from propertyTreeHooks.
src/panels/NavigationPanel/FocusView/FocusView.tsx Uses usePropertyValue for the current aim node and otherwise keeps focus logic the same.
src/panels/NavigationPanel/AnchorAimView/AnchorAimView.tsx Uses property-owner entities for node lookup and sgnUri from uris.ts.
src/panels/Menu/hooks.ts Switches the user-level property lookup to usePropertyValue while preserving menu filtering behavior.
src/panels/GlobeImageryBrowserPanel/hooks.ts Uses sgnRenderableUri/sgnUri from uris.ts when working with globe imagery.
src/panels/GlobeImageryBrowserPanel/GlobeImageryBrowserPanel.tsx Reads the current anchor via usePropertyValue instead of useProperty.
src/panels/GettingStartedPanel/Tasks/SetPropertyTask.tsx Uses usePropertyValue for detecting when a property reaches its target value.
src/panels/GettingStartedPanel/Tasks/NavigationTask.tsx Tracks the current anchor node via usePropertyValue and keeps camera-subscription logic intact.
src/panels/GettingStartedPanel/Tasks/FocusTask.tsx Uses usePropertyValue for NavigationAnchorKey to compute task completion.
src/panels/GettingStartedPanel/Tasks/Components/CurrentFocus.tsx Uses usePropertyValue for the current anchor and useSceneGraphNode to show its label.
src/panels/GettingStartedPanel/Tasks/ChangePropertyTask.tsx Uses usePropertyValue for detecting property changes in tasks.
src/panels/GettingStartedPanel/Tasks/AltitudeTask.tsx Uses usePropertyValue for NavigationAnchorKey while subscribing to camera altitude as before.
src/panels/GeoLocationPanel/AddedCustomNodes.tsx Switches URI helpers to the uris module.
src/panels/ExoplanetsPanel/ExoplanetsPanel.tsx Uses property-owner entities for exoplanet scene graph nodes and sgnUri from uris.ts.
src/hooks/sceneGraphNodes/hooks.ts Reworks scene-graph-node hooks to read from the property/owner entity adapters and uses them to compute GUI settings and focusability.
src/hooks/propertyOwner.ts Refactors property-owner hooks to use the new selectors, replaces useProperty with usePropertyValue where needed, introduces useSceneGraphNodeVisibility and useSetPropertyOwnerVisibility, and adapts usePropertyOwnerVisibility to the new visibility model.
src/hooks/properties.ts Introduces usePropertyValue, simplifies useProperty to read from the entity adapter and send setPropertyValue actions (without manual subscription), and uses usePropertyValue for the global confirmation modal setting.
src/components/PropertyOwner/VisiblityCheckbox.tsx Reworks the visibility checkbox to take explicit visibility and setVisibility props, show a radial sweep icon while fading, and rely on usePropertyValue for the fade percentage.
src/components/PropertyOwner/PropertyOwnerCollapsable.tsx Retrieves visibility and setter via usePropertyOwnerVisibility and passes them into PropertyOwnerVisibilityCheckbox.
src/components/PropertyOwner/PropertyOwner.tsx Uses usePropertyValue and uris.ts helpers to detect globe-layer owners and delegates to the appropriate specialized property-owner components.
src/components/PropertyOwner/ProgressIcon.tsx New RadialSweepIcon component for showing a radial progress/fade state, used by the visibility checkbox during transitions.
src/components/PropertyOwner/Custom/GlobeLayers/GlobeLayersPropertyOwner.tsx Uses sgnIdentifierFromSubownerUri from uris.ts and displayName to label custom globe layer groups.
src/components/PropertyOwner/Custom/GlobeLayers/GlobeLayersGroup.tsx Uses propertySelectors and isPropertyOwnerVisible to compute the count of active layers in each group.
src/components/PropertyOwner/Custom/GlobeLayers/GlobeLayer.tsx Integrates the new usePropertyOwnerVisibility API and passes visibility/setter into PropertyOwnerVisibilityCheckbox, also using the visibility state to color active layer titles.
src/components/Property/Property.tsx Reads property meta data through propertySelectors.selectById to determine which property component to render.
src/components/DynamicMap/ViewCone.tsx Uses usePropertyValue for the evaluated interaction sphere property while leaving the cone computation intact.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@WeirdRubberDuck WeirdRubberDuck left a comment

Choose a reason for hiding this comment

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

Here's a round with some code comments and questions. Will do stress-testing as well :)

Comment on lines +11 to +15
/**
* @class PropertyBatcher
* @description A class that batches property updates to avoid flooding the Redux store with too many updates.
*
* @constructor
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool. Do we plan to start using JSDoc, in general? Is this something you have discussed?

Copy link
Collaborator Author

@ylvaselling ylvaselling Feb 25, 2026

Choose a reason for hiding this comment

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

@engbergandreas and I have not discussed it, I'm open to discussion. I guess we want to have some kind of comment style for ts classes as we now have two

if (visibility === 'Fading' && fade !== undefined) {
return (
<ActionIcon size={20}>
<RadialSweepIcon value={fade * 100} background={'transparent'} size={20} />
Copy link
Collaborator

@WeirdRubberDuck WeirdRubberDuck Feb 3, 2026

Choose a reason for hiding this comment

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

This is a fun detail, but maybe we should check with the team if this type of "status icon" is something we want.

Mentioning because I once made a version that communicates the status, but this was replaced with just a regular checkbox after feedback 😅 IIRC the comment was something like "the users don't care about the current fade state", and that we should keep it simple

Copy link
Member

Choose a reason for hiding this comment

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

Ultimately iirc I came down to that it was hard to keep consistent and also introduced a lot of rerenders that had some performance input 🤔
If both of these are not an issue, I wouldn't see a problem add the marker. I see it more of "visual flair that something is happening" rather than indicating the exact state

Copy link
Collaborator

@WeirdRubberDuck WeirdRubberDuck left a comment

Choose a reason for hiding this comment

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

Found an issue. When adding renderables/SGNs during runtime, the checkbox is not added.

Image

@WeirdRubberDuck
Copy link
Collaborator

Found another issue - error when opening layers:

2026-02-04.15-46-34.mov

I was planning on testing this issue OpenSpace/OpenSpace#3868, which seems like it could be related to the layers not being updated in the store correctly once removed 🤔 Seemed like it might be relevant to this PR, but since it is not possible to open the layers list, I could not try it with this PR

@engbergandreas
Copy link
Member

Getting an error in the console when opening GUI in browser window:
image

useAppSelector((state) => isSgnFocusable(uri, state.properties.properties)) || false
useAppSelector((state) =>
isSgnFocusable(uri, propertySelectors.selectEntities(state))
) || false
Copy link
Member

Choose a reason for hiding this comment

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

I guess React question... why is the || false necessary here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It isn't :) Good catch

@ylvaselling
Copy link
Collaborator Author

Fixed the reported issues now - layers + adding and removing scene graph nodes should now work. OpenSpace/OpenSpace#3868 is not going to be fixed with this PR as it is related to an issue with the usePropListeningState hook and not the redux store (did some investigation)

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.

5 participants