Feature/property tree topic by ylvaselling · Pull Request #195 · OpenSpace/OpenSpace-WebGui

ylvaselling · 2026-01-30T09:40:34Z

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

# Conflicts: # src/hooks/properties.ts # src/redux/store.ts

… visibility in redux

…cene graph nodes as well

…t need to subscribe anymore

This is not guaranteed to run after the propertyowners are added

ylvaselling · 2026-02-25T13:44:56Z

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)

Copilot

Pull request overview

Copilot reviewed 76 out of 76 changed files in this pull request and generated 14 comments.

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

src/util/propertyTreeHelpers.ts

src/types/types.ts

src/redux/propertyTree/propertyOwnerSlice.ts

src/components/DynamicMap/DynamicMap.tsx

src/panels/Scene/SceneGraphNode/SceneGraphNodeMetaInfo.tsx

src/hooks/sceneGraphNodes/hooks.ts

src/redux/propertyTree/propertyTreeMiddleware.ts

src/redux/time/timeMiddleware.ts

src/redux/local/localMiddleware.ts

WeirdRubberDuck · 2026-03-12T12:23:33Z

src/redux/propertyTree/propertyTreeMiddleware.ts

+    effect: () => {
+      unsubscribe();
+    }


Valid points :)

WeirdRubberDuck

Here are some more comments.

I think that the checkbox fading should be brought up during a team meeting just to verify that everyone thinks this change is a good idea, but otherwise, I don't really have many comments

Did some testing and could not find any issues. I will also try a two-node setup locally on my machine and do some testing in the dome

src/util/batcher.ts

src/util/uris.ts

WeirdRubberDuck · 2026-03-12T12:31:54Z

src/redux/propertyTree/propertyTreeMiddleware.ts

+      properties.forEach((p) => {
+        propertiesMap[p.uri] = p;
+      });


I see your point - it makes sense 👍 would be great if we came to a consensus on a style to use and stick to that everywhere

WeirdRubberDuck · 2026-03-12T12:43:09Z

src/panels/Scene/SceneTree/hooks.ts

+  const properties = useAppSelector((state) => propertySelectors.selectEntities(state));
+  const propertyOwners = useAppSelector((state) =>
+    propertyOwnerSelectors.selectEntities(state)
+  );


Thanks for the details! 🙏

My general feeling is that having the hooks would be worth it, at least in the places where we get all properties or property owners. It's a lot easier to read a hook that says, for example, allProperties and allPropertyowners instead of these two lines

const properties = useAppSelector((state) => propertySelectors.selectEntities(state)); const propertyOwners = useAppSelector((state) => propertyOwnerSelectors.selectEntities(state));

But this might be biased on how often we had to e.g. get all properties before, and just not being used to the new "entities" concept. So I'm open to discussion and being convinced otherwise

What do you think @engbergandreas ?

WeirdRubberDuck · 2026-03-12T12:46:31Z

src/components/PropertyOwner/VisiblityCheckbox.tsx

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


Do we want to discuss it during a meeting? To make sure? Considering that it's a relatively big visual change, and it changes the "instantaneous" feeling of clicking the checkbox, it might be good to get more opinions

The new "fading" checkbox is not as much of a visual change, but I found it a little confusing when enabling/disabling multiple things in sequence. The visual feedback is slightly delayed, which is quite noticeable

Co-authored-by: Emma Broman <emma.broman@liu.se>

…Space/OpenSpace-WebGui into feature/property-tree-topic

ylvaselling added 22 commits January 19, 2026 11:11

First draft of property tree topic

b306ae4

# Conflicts: # src/hooks/properties.ts # src/redux/store.ts

Make the new property tree kind of work

00c8ce7

Fix the updating of properties

340d6cb

Remove previous redux for properties & property owners, and switch to…

12a3494

… visibility in redux

Fix some bugs

e2b330f

A fancy progress icon

a4af65a

Some polishing of the checkbox

3e507c9

Throttle time updates

3f99532

First draft of more efficient property setting; minimize dispatches

9cd5f99

Remove duplicate comment

4d1f18d

Make the middleware get the root explicitly

f0412d6

Throttle camera updates

d827caf

Remove unneccessary subscriptions

cf60a1a

Rename propertyTreeTest to propertyTree and add remove uri action

8be7b4a

Split the usePropertyOwnerVisibility into two hooks and add one for s…

d0a9910

…cene graph nodes as well

Fix progress bar icon

d57c1cc

Add comments on entity adapters

a2739ee

Fix the glitch of the icon

7bab1cb

Rename propertyActive to propertyVisible

e562cd8

Use redux visibility instead of properties to calculate visibility

e4b30f9

Some cleanup

5ca4d9c

Update from useProperty to usePropertyValue where possible, as we don…

75397ba

…t need to subscribe anymore

ylvaselling mentioned this pull request Jan 30, 2026

Feature/topics OpenSpace/OpenSpace#3896

Open

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

ylvaselling requested review from WeirdRubberDuck, Copilot and engbergandreas and removed request for WeirdRubberDuck and engbergandreas January 30, 2026 15:45

ylvaselling added 5 commits February 25, 2026 12:51

Remove refreshing of group as a dispatch listener

5f72eb1

This is not guaranteed to run after the propertyowners are added

Add visibility for new property owner sgns

6ad812c

Add explaining comment

5d655f9

Fix issue with updating fading correctly

c38633d

Set visibility when adding new property owners

fff3435

ylvaselling added 6 commits February 25, 2026 15:22

Remove unnecessary false

da2678c

Change the fade icon as it was a bit choppy

adb9912

Set throttling time to a better value

62d1c42

Lint fix

862d28c

Stop showing map markers when delta time is too fast

20f85cd

Fix render error with visible filter

71c53ea

ylvaselling requested review from WeirdRubberDuck, alexanderbock, Copilot and engbergandreas March 11, 2026 10:07

Copilot started reviewing on behalf of ylvaselling March 11, 2026 10:08 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

WeirdRubberDuck reviewed Mar 12, 2026

View reviewed changes

ylvaselling and others added 10 commits March 18, 2026 13:27

Address PR comments

4c56a22

Fix the bug with making new property updates

e1dd87c

Do some proper type checks for enabled and fade

658e9cc

Add visibility calculations if someone upserts many properties

b216c33

Add some behaviour to the fading action icon

9878c46

Update src/util/batcher.ts

e547f47

Co-authored-by: Emma Broman <emma.broman@liu.se>

Update src/util/batcher.ts

759e1af

Co-authored-by: Emma Broman <emma.broman@liu.se>

Address PR comments

eafbaa9

Merge branch 'feature/property-tree-topic' of https://github.com/Open…

f0f6cc7

…Space/OpenSpace-WebGui into feature/property-tree-topic

Add metadata to property tree topic

a2ccecf

Conversation

ylvaselling commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ylvaselling commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WeirdRubberDuck Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

WeirdRubberDuck left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WeirdRubberDuck Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

WeirdRubberDuck Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

WeirdRubberDuck Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ylvaselling commented Jan 30, 2026 •

edited

Loading