[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: JDK-8269921 Text in Textflow and listeners on bounds can cause endless loop/crash and other
From:       danielpeintner <duke () openjdk ! org>
Date:       2022-09-22 6:31:29
Message-ID: bhZQB0Wa0EQ7vsvCWRvHcnjm9lKhALUm3P0M-OE5aIY=.5f8b925f-158e-479a-adf4-5603d5748646 () github ! com
[Download RAW message or body]

On Wed, 21 Sep 2022 13:27:46 GMT, Michael Strauß <mstrauss@openjdk.org> wrote:

> > I got this problem as well today. NPE as `runs` is null in line `359`. Does it \
> > make sense to first 'resolve' this by adding a simple null check and later we may \
> > try your other change which removes the call to `getParent().layout()`? 
> > Edit: I also got another interesting NPE in `PrismTextLayout`:
> > 
> > java.lang.NullPointerException: Cannot read the array length because "this.runs" \
> > is null at com.sun.javafx.text.PrismTextLayout.addTextRun(PrismTextLayout.java:770)
> >  at com.sun.javafx.text.GlyphLayout.addTextRun(GlyphLayout.java:140)
> > at com.sun.javafx.text.GlyphLayout.breakRuns(GlyphLayout.java:210)
> > at com.sun.javafx.text.PrismTextLayout.buildRuns(PrismTextLayout.java:785)
> > at com.sun.javafx.text.PrismTextLayout.layout(PrismTextLayout.java:1036)
> > at com.sun.javafx.text.PrismTextLayout.ensureLayout(PrismTextLayout.java:223)
> > at com.sun.javafx.text.PrismTextLayout.getBounds(PrismTextLayout.java:246)
> > at javafx.scene.text.Text.getLogicalBounds(Text.java:432)
> > at javafx.scene.text.Text.doComputeGeomBounds(Text.java:1187)
> > at javafx.scene.text.Text$1.doComputeGeomBounds(Text.java:149)
> > at com.sun.javafx.scene.shape.TextHelper.computeGeomBoundsImpl(TextHelper.java:90)
> >  at com.sun.javafx.scene.NodeHelper.computeGeomBounds(NodeHelper.java:116)
> > at javafx.scene.Node.updateGeomBounds(Node.java:3818)
> > at javafx.scene.Node.getGeomBounds(Node.java:3780)
> > at javafx.scene.Node.updateBounds(Node.java:776)
> > at javafx.scene.Parent.updateBounds(Parent.java:1833)
> > ...
> > <<pulse>>
> 
> > The simple null check helps in this case. But sometimes I see the exception, \
> > posted by @Maran23 . So the root cause is probably not fixed - but the symptoms \
> > are. I guess, this PR is then a good progress.
> 
> Maybe it's not good progress. You've investigated the issue, and have identified a \
> likely root cause. Not fixing the root cause just means that it most likely won't \
> be fixed for years to come, and the insights you've accumulated will fade. 
> I think we should just fix the root cause. If some application breaks as a result \
> of this, not much harm is done: we'd gain valuable insight into a real-world \
> situation that depends on the current behavior, and can then decide whether to \
> revert the fix, or provide a viable alternative solution (for users) that \
> accomplishes the same goal.

@mstr2 I see you arguments and your concerns. On the other hand, as a developer \
affected by this problem, it is different. Costumers are complaining and now it seems \
to occur even more frequently.

Note: I do not have any voting rights and I do not want to make any claims. Anyhow, I \
think we all agree that the simple null check *solves* the issue by hiding it and \
definitely does not create any regression. Hence, affected applications may get a \
quick fix and are running stable. The *actual* fix can come later, taking more time, \
with more sophisticated testing et cetera. I am happy to help there with our \
application as well. Thanks!

-------------

PR: https://git.openjdk.org/jfx/pull/564


[prev in list] [next in list] [prev in thread] [next in thread]