Accessibility - empty accessibility node for any ADT added, even if empty

The team may already be aware of this one as @Deltakosh observed it directly. HTMLTwinTree is creating empty dom nodes for elements that have no accessibility info, and we are seeing performance issues with large numbers of babylon nodes.
I was aware of the empty nodes but they aren’t read by the screenreader, so I didn’t see this as an issue before.

I wasn’t there personally but I’m guessing this could be a mix of things going on . I am updating accessibilityTags.description on every babylon node update, even if same value, and I’m guessing that might be causing excessive HTMLTwinTree updates at scale. From memory, I saw that there is a bit of code in there that loops over the whole babylon tree I think?? maybe?

At the very least it shouldn’t create dom nodes for things that have no accessibility tags at all.

Creating this issue for tracking purposes.

1 Like

@NightSkies, it would be amazing to have a repro in the playground :slight_smile:

I witnessed the issue as well in Frame but the playground would help @carolhmj when she ll be back from vacation to address the issue.

Hey there! I’d like to gather a bit more information about this, if possible (and I second Seb a PG would be fantastic!)
What version of Babylon are you using?
You are correct in that the HTML tree is updated whenever an accessible element’s tag description changes, and also when an accessible element changes visibility: https://github.com/BabylonJS/Babylon.js/blob/751fd9dee514e7ac778dabc10e21e6102f3b0f64/packages/tools/accessibility/src/HtmlTwin/htmlTwinHostComponent.tsx#L219 which is called by https://github.com/BabylonJS/Babylon.js/blob/751fd9dee514e7ac778dabc10e21e6102f3b0f64/packages/tools/accessibility/src/HtmlTwin/htmlTwinHostComponent.tsx#L69C46-L69C46. Also when meshes are added: https://github.com/BabylonJS/Babylon.js/blob/751fd9dee514e7ac778dabc10e21e6102f3b0f64/packages/tools/accessibility/src/HtmlTwin/htmlTwinHostComponent.tsx#L146 or removed: https://github.com/BabylonJS/Babylon.js/blob/751fd9dee514e7ac778dabc10e21e6102f3b0f64/packages/tools/accessibility/src/HtmlTwin/htmlTwinHostComponent.tsx#L174.

About creating empty DOM nodes, this can happen for GUI elements which don’t have a description, but one of their children has a description, so we still need to render the tree. Also when addAllControls is true, all GUI Buttons and TextBlocks are added, regardless of description. You can see the function that filters controls here: https://github.com/BabylonJS/Babylon.js/blob/751fd9dee514e7ac778dabc10e21e6102f3b0f64/packages/tools/accessibility/src/HtmlTwin/htmlTwinHostComponent.tsx#L304
And this function changed a bit recently, so that’s why I’m asking about version :slight_smile:

I’ll be waiting further information!

2 Likes

I made this playground but it didn’t seem to repro the bug.
I’m lacking some details on my end unfortunately but @Deltakosh observed things directly. I will try to get you more details soon!

At time of posting, I believe these were the package versions:

    "@babylonjs/accessibility": "^6.15.0",
    "@babylonjs/core": "6.16.0",
    "@babylonjs/gui": "6.16.0",
    "@babylonjs/inspector": "6.16.0",
    "@babylonjs/loaders": "6.16.0",
    "@babylonjs/materials": "6.16.0",

That is a good insight about GUI controls with no description. If it’s not a button, perhaps those should not be generated?

We have addAllControls: false.

I observed it as well and it is why we need a repro :frowning: You could try to add a lot of meshes in the scene maybe.

I remember a lot of update being called in the accessibility.

But do these controls have a child with description? Because if so, we still need to go into them to reach the child. Through, the version you’re using was before this change: Release 6.16.1 · BabylonJS/Babylon.js (github.com), so we shouldn’t even be going in these nodes in your case :thinking: Blaming Babylon.js/packages/tools/accessibility/src/HtmlTwin/htmlTwinHostComponent.tsx at 52c99f36e3fd02d06fe94795a0ca36d2edcab502 · BabylonJS/Babylon.js (github.com) this is how the calculation occurs in 5.15, so we don’t even try to go in the node when it doesn’t have a description and addAllControls is false.

I have reproduced the bug! Here’s a playground!

Turns out any ADT creates the empty accessibility HTML node, even if there’s nothing in it.

When I toggle a11y entirely and add them with a delay, I’m not sure a11y is making much of a difference. I’m sure if i made a graph of the FPS I might see something slight.

It seems that having an ADT per sphere decreases the performance by half, but I feel like thats expected because 2x the objects.

On my machine (Nvidia 2060, 3 monitors (1/3 performance)):

  • ADT=true, 1250 - 53-58 fps (<60)
  • ADT=false, 4000 - 55 fps (<60)

Thanks a ton for the PG! I’ve improved our logic for rendering the twin tree so we don’t fall in that scenario anymore :slight_smile: PR: avoid rendering empty trees on a11y tree by carolhmj · Pull Request #14239 · BabylonJS/Babylon.js (github.com)

2 Likes

I still have a concern about the a11y tree update performance. This isn’t a thing i gathered data on. (I unfortunately wasn’t present to record all of the data on the supposed performance lag that was observed). I’d have to find some way of graphing the FPS in above playground as the sphere count goes up.

I thought it mainly only updated the tree when the .accessibilityTag is set. Does it iterate over the entire scene when this happens? I fear it doing this whenever a new mesh is added.

Yeah, that’s what happens https://github.com/BabylonJS/Babylon.js/blob/3333a8797636776e76eeadd612d159e877fdc8cc/packages/tools/accessibility/src/HtmlTwin/htmlTwinHostComponent.tsx#L143C112-L143C112

I did a bit of analysis on the scene by using Chrome’s profiler, and it doesn’t look that the a11y update is taking much time, at least relative to everything else that goes on the scene. When looking at the highest self times in the capture I made with a11y on, and 2000 spheres:

Doesn’t look much different from the capture with a11y off:

Of course, I might be missing something on the scenario, I think we should continue the discussions about performance as it’s always very important :slight_smile:

In above profiles, its a test of incremental adding of the spheres? I figured the execution % time of a11y update would go up and up and up as more things are added.
In my playground if the creation delay is 0, it will add all the spheres at once, and I figure it will only be 1 call to the a11y update.

If the above profile captures include it being called for every mesh add, and its still a small % of time, then I’m ok with it.

Hello! These profiles weren’t using incremental adding, so I tried again with an add delay of 100, but I still couldn’t see significant differences:

With a11y:

Without a11y:

One difference I notice in both from the previous capture is that there seems to be a lot more GC involved, so investigating the memory allocation is a good idea.

It seems the mover() function that moves the nodes is causing a lot of the GC, which makes sense since it creates and clears quite a bunch of timeouts. I updated the PG with a flag to turn off the mover: bug - empty nodes, bad perf when accessibility on | Babylon.js Playground (babylonjs.com). I took it out, and I’m investigating further into the memory allocations of the _updateGUI function on the twin tree.

@NightSkies do you have any kind of memory capture of the problem in frame? A performance profiling too? These would be very helpful to understand the issue better :slight_smile:

I don’t think there’s any more info. We fixed the cause ultimately. On our end, our implementation was actually creating ADTs after a certain number for our nametags. We did notice the empty accessibility nodes, and i’m glad thats being fixed, but I think the ADTs were naturally decreasing performance due to the additional object per peer.

I hoping the above playground will help track down the allocations!

Thanks for all the help.

2 Likes

Hi @carolhmj , following up on this with something else we’ve noticed and another question. We are wondering if we can disable the default behavior that creates accessibility nodes for any new mesh added to the scene. Interestingly, while we didn’t notice any problems when doing importMeshAsync, we notice some very severe lag in this example, which happens to be using the method we use to load in Frame environments : bug - a11y lags performance for every new node created | Babylon.js Playground (babylonjs.com)

For now we have to do a patch hat disables our accessibility stuff, because when a user switches to this lovely environment (that has thousands of meshes!), something in the html twin renderer starts lagging out pretty hard. You can see if you comment out the renderer in that PG then the load in for this model is quite smooth otherwise!

We’re planning on letting users explicitly set accessibility tags for 3D models that they choose to tag, but we don’t want nodes created for other meshes so that we can avoid issues like this when users change environments and end up loading thousands of other meshes into their Frames that don’t need accessibility tags (we manually set a tag on the one rootmesh of the model to cover the accessibility tag for the environment model!).

What do you think?

We look forward to bringing it back, though, or if there’s a setting we have to tweak that we aren’t understanding or setting properly then please let us know and we’ll update quick as we can!

Currently, we add observers for every model created so we can check if an accessibility description is set and then add it to the tree, so we could add a flag to completely skip this process, and create a function/observable to manually add nodes to the tree, would that work for you?

1 Like