Skip to content

Conversation

@elsassph
Copy link
Contributor

@elsassph elsassph commented Aug 20, 2025

Fixes #554 - getChildrenByPosition wrong order
Might fix #129 - zIndexedChildren order obsolete when out of bounds

Issue

Stage.getChildrenByPosition(x, y) collects elements at the (global) x, y location, internally calling ElementCore.collectAtCoord(x, y, children)

    getChildrenByPosition(x, y) {
        // collector target
        const children = [];

        // force updating from the root
        this.root.core.update();

        // collect children recursively
        this.root.core.collectAtCoord(x, y, children);

        return children;
    }

Internally, collectAtCoord sorts the result using ElementCore.sortZIndexedChildren. The issue with this sorting function is that it doesn't just sort by zIndex, but also by core._updateTreeOrder, but this value can be wrong.

One may think that this.root.core.update() would ensure the tree order is correct, but this is wrong: tree ordering depends on ctx.updateTreeOrder to be reset before running the update.

The result is randomly wrong elements ordering:

  • tree order is updated following some (slightly confusing) heuristics, so it only happens occasionally,
  • when this happens it increments based on ctx.updateTreeOrder,
  • consequently, when Stage calls root.core.update(), some parts of the tree may be ordered based on the current (non reinitialised) value of ctx.updateTreeOrder, resulting in some subtree having randomly a "high" tree order value, meaning these elements will appear to be on top during in the resulting collectAtCoord.

Solution

Stage should only request a tree order update, ensuring ctx.updateTreeOrder is properly reset.

Additionally, collectAtCoord quite inefficiently re-sorts the children on every iteration, so it's better to only sort at the very end.

Testing

This issue was happening in our (large and complex) Lightning application in a fairly consistent way (but very hard to extract in a sample).

We did work around it by doing a brute-force updating of all the elements' tree order before calling root.collectAtCoord directly. Now this PR attempts at fixing the root cause properly and for all Lightning users.

We could confirm this finer fix allowed us to remove our workaround without causing regressions. However we don't use any zIndex so can't verify this use case.

}
} else {
a.splice(n); // Remove items that were sorted in b array, so that we can sort a
+ // Beware that the function passed here to Array.sort must be stable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fun that it still worked with these + copy-pasta

Copy link
Contributor

@wouterlucas wouterlucas left a comment

Choose a reason for hiding this comment

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

LGTM

@michielvandergeest michielvandergeest merged commit d5e61a9 into rdkcentral:dev Sep 8, 2025
4 checks passed
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.

3 participants