On Mon, 30 Oct 2023 15:03:05 GMT, Florian Kirmaier <fkirma...@openjdk.org> wrote:
>> It's "a bit" complicated. >> In some situations, getRuns get's called because listeners on bounds are set. >> This causes TextFlow to layout to compute the runs. >> Afterward, the bounds of the parents get updated. >> This triggers a call to compute bounds - which cascades up to the children. >> When the geometry of the previous Text gets computed in this big stack - it >> throws an nullpointer. >> The Text doesn't have its runs, and calling TextFlow.layout is now a noop >> (it detects repeated calls in the same stack) >> >> In the case it happened - it didn't repair and the application kinda crashed. >> This bug most likely can also be triggered by ScenicView or similar tools, >> which sets listeners to the bounds. >> It also can cause unpredictable performance issues. >> >> Unit test and example stacktrace are in the ticket. >> >> The suggested fix makes sure that recomputing the geometry of the Text, >> doesn't trigger the layout of the TextFlow. >> The Textflow should be layouting by the Parent. >> This might change the behavior in some cases, but as far as I've tested it >> works without issues in TextFlow Heavy applications. >> >> Benefits: >> * Better Tooling Support For ScenicView etc. >> * Fixes complicated but reproducible crashes >> * Might fix some rare crashes, which are hard to reproduce >> * Likely improves performance - might fix some edge cases with >> unpredictable bad performance > > Florian Kirmaier has updated the pull request incrementally with one > additional commit since the last revision: > > updated test to junit5 > improved name for method onEveryNode -> addBoundsListener As long as subsequent layout cycles fix the bounds (and apps like ScenicView show them correctly), I am ok with this fix. ------------- Marked as reviewed by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/564#pullrequestreview-1730205022
