> 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 classes (the JavaFX font > classes do that as well) > - I ren...
Marius Hanl has updated the pull request incrementally with two additional commits since the last revision: - .classpath - rephrase class javadoc ------------- Changes: - all: https://git.openjdk.org/jfx/pull/2145/files - new: https://git.openjdk.org/jfx/pull/2145/files/1baaa116..8cc6d9c8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=2145&range=01 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=2145&range=00-01 Stats: 5 lines in 2 files changed: 2 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jfx/pull/2145.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/2145/head:pull/2145 PR: https://git.openjdk.org/jfx/pull/2145
