Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v5]
On Mon, 29 Jan 2024 16:10:01 GMT, Andy Goryachev wrote:
> > ancient test case that was disabled (see
> > https://bugs.openjdk.org/browse/JDK-8087615).
>
> should the old ticket be added to this PR?
If this fixes all tests that are `@Ignore`d with this old bug ID, then yes. I
still see a skipped test, though. If this is still being skipped be cause of
JDK-8087615, then it should use `@Ignore("JDK-8087615")` (or
`@Disabled("JDK-8087615")` if that is preferred in JUnit5).
-
PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1915073445
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v5]
On Sun, 28 Jan 2024 23:30:56 GMT, John Hendrikx wrote: > ancient test case that was disabled (see > https://bugs.openjdk.org/browse/JDK-8087615). should the old ticket be added to this PR? - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1915034433
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v5]
On Sun, 28 Jan 2024 23:29:49 GMT, John Hendrikx 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 incrementally with two additional > commits since the last revision: > > - Clean-up, re-enable and extend ancient TextLayout test > - TextRun getAdvance should return 0 when positions array is null I found an ancient test case that was disabled (see https://bugs.openjdk.org/browse/JDK-8087615). I re-enabled it, found that a lot of the test cases now fail, but not all. Since it is still useful, I've enabled the parts that still worked, and "fixed" the parts that didn't work to work as they do now (to at least avoid further regression). I also extended this test with cases that cover the LEFT/CENTER/RIGHT/JUSTIFY alignments. The test checks that the amount of trailing spaces when a line break occurs has no effect on the left offset positioning. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1913758923
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v5]
> 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, and IMHO does not need further attent... John Hendrikx has updated the pull request incrementally with two additional commits since the last revision: - Clean-up, re-enable and extend ancient TextLayout test - TextRun getAdvance should return 0 when positions array is null - Changes: - all: https://git.openjdk.org/jfx/pull/1236/files - new: https://git.openjdk.org/jfx/pull/1236/files/7d11a992..cb837343 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx=1236=04 - incr: https://webrevs.openjdk.org/?repo=jfx=1236=03-04 Stats: 339 lines in 2 files changed: 234 ins; 18 del; 87 mod Patch: https://git.openjdk.org/jfx/pull/1236.diff Fetch: git fetch https://git.openjdk.org/jfx.git pull/1236/head:pull/1236 PR: https://git.openjdk.org/jfx/pull/1236
