On Wed, 7 Feb 2024 15:06:18 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> There are a number of tickets open related to text rendering:
>> 
>> https://bugs.openjdk.org/browse/JDK-8314215
>> 
>> https://bugs.openjdk.org/browse/JDK-8145496
>> 
>> https://bugs.openjdk.org/browse/JDK-8129014
>> 
>> They have in common that wrapped text is taking the trailing spaces on each 
>> wrapped line into account when calculating where to wrap.  This looks okay 
>> for text that is left aligned (as the spaces will be trailing the lines and 
>> generally aren't a problem, but looks weird with CENTER and RIGHT 
>> alignments.  Even with LEFT alignment there are artifacts of this behavior, 
>> where a line like `AAA  BBB  CCC` (note the **double** spaces) gets split up 
>> into `AAA  `, `BBB  ` and `CCC`, but if space reduces further, it will wrap 
>> **too** early because the space is taken into account (ie. `AAA` may still 
>> have fit just fine, but `AAA  ` doesn't, so the engine wraps it to `AA` + `A 
>>  ` or something).
>> 
>> The fix for this is two fold; first the individual lines of text should not 
>> include any trailing spaces into their widths; second, the code that is 
>> taking the trailing space into account when wrapping should ignore all 
>> trailing spaces (currently it is ignoring all but one trailing space).  With 
>> these two fixes, the layout in LEFT/CENTER/RIGHT alignments all look great, 
>> and there is no more early wrapping due to a space being taking into account 
>> while the actual text still would have fit (this is annoying in tight 
>> layouts, where a line can be wrapped early even though it looks like it 
>> would have fit).
>> 
>> If it were that simple, we'd be done, but there may be another issue here 
>> that needs solving: wrapped aligned TextArea's.
>> 
>> TextArea don't directly support text alignment (via a setTextAlignment 
>> method like Label) but you can change it via CSS.
>> 
>> For Left alignment + wrapping, TextArea will ignore any spaces typed before 
>> a line that was wrapped.  In other words, you can type spaces as much as you 
>> want, and they won't show up and the cursor won't move.  The spaces are all 
>> getting appended to the previous line.  When you cursor through these 
>> spaces, the cursor can be rendered out of the control's bounds.  To 
>> illustrate, if you have the text `AAA                 BBB CCC`, and the text 
>> gets wrapped to `AAA`, `BBB`, `CCC`, typing spaces before `BBB` will not 
>> show up.  If you cursor back, the cursor may be outside the control bounds 
>> because so many spaces are trailing `AAA`.
>> 
>> The above behavior has NOT changed, is pretty standard for wrapped text 
>> controls,...
>
> John Hendrikx 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 16 additional 
> commits since the last revision:
> 
>  - Merge branch 'openjdk:master' into feature/wrapped-aligned-text-rendering
>  - Fix test comment
>  - More test cases (for hard wrapping) and another small fix
>  - Improve tests, fix a bug
>  - Fix bug which confused char index with glyph index
>  - Move test that needs QuantumToolkit to system tests
>  - Clean-up, re-enable and extend ancient TextLayout test
>  - TextRun getAdvance should return 0 when positions array is null
>  - Fix justify alignment to take all trailing spaces into account
>  - Fix trailing space calculation to access positions array directly
>  - ... and 6 more: https://git.openjdk.org/jfx/compare/fed78522...388696a6

I've been doing some more investigation, and digging up different kinds of 
problems, that are not entirely related to this change.

- Font calculation is using `float`s, which are converted to/from `double`s 
which FX uses. This causes slight inaccuracies at the places these translations 
occur.  For example, wrap width is converted from double to float, then later 
bounds are returned (also as floats) which are then promoted back to double.  A 
scroll pane which may be expecting `(double)wrapWidth` but sees a bounds value 
returned that is `(double)(float)wrapWidth` which may be slightly smaller or 
larger than what the pane expects.  If it is larger, it will suddenly display a 
horizonal scroll bar to compensate for some tiny difference of 0.00001 pixels.
- The `Group` wrapper around `Text` used by MonkeyTester is somehow amplifying 
these inaccuracies (or is adding its own), causing a horizontal scroll bar to 
be almost always displayed (flickering during resizes) -- `StackPane` seems to 
not cause such additional problems (as `Group` has a very specific purpose, I 
think using it here is incorrect).
- Text justification (`JUSTIFY`) is creating cumulative errors -- it divides 
the amount of extra space by the number of white spaces in the line (which then 
gets rounded to fit in a `float`), then for each white space, it adds this 
amount, each time increasing the error a bit more.  A better approach is to 
substract the distributed space from the total space each time, and for the 
last space use the left over amount.
- Text justification with certain fonts cuts off the last portion of the last 
character on a line sometimes (primarily with serif fonts, a small part of the 
serif is outside the expected bounds).  This seems to be more than the above 
accumulated error can account for.  Oddly enough, when switching to LEFT/RIGHT 
alignment, it is clear the text should fit easily (the lines are not wrapped 
differently), but as soon as JUSTIFY alignment is used, the spaces between the 
words get large enough that a small part of the last character is out of 
bounds.  Still investigating why this happens.

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

PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1934033307

Reply via email to