On Tue, 14 Apr 2026 20:19:05 GMT, Marius Hanl <[email protected]> wrote:
>> This one took me actually some hours to fully understand. >> I recommend reading the ticket description made by Phil as it already >> explains most of it. >> >> What is the problem (in `Utils`): >> - `Utils.computeTextWidth(..)` does not initialize the `boundsType`. As a >> consequence, we do not cache the layout result >> - For the text width, it makes sense to set it, so the result is cached. >> Note that the `boundsType` does not matter for the width calculation >> itself, but we really want it set so the result will be cached if possible >> - At one point due to `Utils.computeTextHeight(..)` method call, the >> `boundsType` is initialized. Now calls to `Utils.computeTextWidth(..)` do >> cache the result >> - So in order to fix that, `Utils.computeTextWidth(..)` does now set the >> the property as well >> >> Ok, but why only cache this specific usecase (Phil was also wondering about >> that) (in `PrismTextLayout`): >> - First of all, the previous method name was confusing (me). >> What it actually does: >> - decide if the layout result should be cached >> - when we have a cache hit, if we can reuse it completely or ONLY the text >> runs >> - So what `PrismTextLayout` does is to cache the layout result for simple >> cases with the default setting. >> - This is biased towards modena, which specifies `-fx-bounds-type: >> logical_vertical_center;` for ever `Text` node >> - If one of those setting or the bounds type do not match, we will not >> cache the text layout or reuse it completely, but the text runs instead >> - That means the logic is fine and there's a thought behind it: Only cache >> if the settings match those we are most likely to receive from the >> application >> - In case we have already a cache result but for example the `wrapWidth` >> or the `boundsType` is different, we can reuse the runs but not the whole >> cached layout result >> - So we should not cache results with `boundsType = 0`, as those will have >> another height than with `boundsType = BOUNDS_CENTER` >> - So the logic is unchanged there >> >> As `StubTextLayout` extends `PrismTextLayout`, we can actually write >> meaningful tests: >> - So I wrote a bunch that make clear how the cache works and when a >> `GlyphLayout` is created (due to a complete cache miss) >> - Note that the tests are green before and after this PR >> - Because `PrismTextLayout.setContent(..)` will request the font strike (for >> `hashCode` calculation for the layout result cache), I needed to implement >> `hashCode` and `equals` for some of the stub test cla... > > Marius Hanl has updated the pull request incrementally with two additional > commits since the last revision: > > - .classpath > - rephrase class javadoc @Ziad-Mid could you be the second reviewer please? ------------- PR Comment: https://git.openjdk.org/jfx/pull/2145#issuecomment-4502951543
