On Tue, 26 Sep 2023 08:43:47 GMT, Florian Kirmaier <[email protected]>
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 with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains five additional
> commits since the last revision:
>
> - Merge remote-tracking branch 'origjfx/master' into JDK-8269921-textflow-bug
> - JDK-8269921
> Reverted the change to the layout, so we can fix the main-bug without
> further discussions.
> - Merge remote-tracking branch 'origjfx/master' into JDK-8269921-textflow-bug
> - JDK-8269921
> Added a copyright header
> - JDK-8269921
> Fixing a crash related to Text and TextFlow with bounds listeners
modules/javafx.graphics/src/main/java/javafx/scene/text/Text.java line 359:
> 357: if (spanBoundsInvalid) {
> 358: GlyphList[] runs = getRuns();
> 359: if (runs != null && runs.length != 0) {
on a surface, this check looks ok.
however, in getRuns(), Text:388 we have
/* List of run is initialized when the TextFlow layout the children
*/
getParent().layout();
meaning that `textRuns` did not initialize even though the
`getParent().layout();` was invoked - why?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/564#discussion_r1372377419
