Skip to content
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

Bad zSorting when zIndex is the same #443

Closed
jorge-graca-sky opened this issue Jan 19, 2023 · 6 comments · Fixed by #441 or #514
Closed

Bad zSorting when zIndex is the same #443

jorge-graca-sky opened this issue Jan 19, 2023 · 6 comments · Fixed by #441 or #514

Comments

@jorge-graca-sky
Copy link
Contributor

jorge-graca-sky commented Jan 19, 2023

Hi, I have been investigating why some Elements randomly pop in front of other in our app. This has revealed a bug in the lightning’s zSorting algorithm, it does not sort Elements with the same zIndex by updateTreeOrder.

The issue is on sortZIndexedChildren of ElementCore, on lines 2062 “// Merge-sort arrays;” it assumes both arrays are sorted, however same zIndex items may not be (since _updateTreeOrder change does not trigger _zIndexResort = true the a array may have unsorted items)

The logic that ensured this was removed on version 1.0.4: performance(updateTreeOrder): refactor, make simpler and (in most cas… · rdkcentral/Lightning@38285e0 · GitHub

My suggestion of fix:
#441

@frank-weindel
Copy link
Contributor

Hi @jorge-graca-sky, do you have any reproduction code/steps that consistently cause the "random pop" issue? If it helps we have a playground now where you can easily build examples and share them with a link: https://lightningjs.io/playground/

@jorge-graca-sky
Copy link
Contributor Author

Hi @frank-weindel, thanks for the quick reply! Here is the requested playground example:

  1. To see the bug press left.
  2. Can see the issue with _updateTreeOrder not begin sorted by pressing right (after pressing left).
  3. Can fix the zSort algorithm by pressing down.

Playground example

@jorge-graca-sky
Copy link
Contributor Author

Hi @frank-weindel, is the example understandable? Anything else I could help?

@chiefcll
Copy link
Contributor

chiefcll commented Mar 6, 2023

@uguraslan is this fixed - I think the bug needs to be closed

@frank-weindel
Copy link
Contributor

frank-weindel commented Mar 9, 2023

@chiefcll I don't think this bug was ever addressed. Perhaps we should elevate it?

@uguraslan
Copy link
Contributor

@jorge-graca-sky suggested PR #441 as a fix, we need to review the suggested fix.

@uguraslan uguraslan added the bug label Mar 13, 2023
@uguraslan uguraslan added the fix-in-review This issue has a PR in review label Apr 3, 2023
@uguraslan uguraslan linked a pull request Apr 11, 2023 that will close this issue
@uguraslan uguraslan moved this to In Progress in Lightning May 2, 2023
@uguraslan uguraslan moved this from In Progress to In Review in Lightning May 2, 2023
@uguraslan uguraslan moved this from In Review to In Progress in Lightning May 2, 2023
@uguraslan uguraslan added this to the October 2023 release milestone Jul 27, 2023
@uguraslan uguraslan moved this from In Progress to Done in Lightning Oct 10, 2023
@uguraslan uguraslan added fixed and removed fix-in-review This issue has a PR in review labels Oct 10, 2023
@uguraslan uguraslan mentioned this issue Oct 11, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
4 participants