Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Sat, 10 Feb 2024 14:40:45 GMT, Kevin Rushforth wrote: >>> And on Linux I get similar failures to my Windows system due to no Monaco >>> font. >> >> Was the Tahoma font available there (you can see from the `TextLayoutTest > >> fail()` test) ? >> >> If so, I could rewrite most of the tests to use the Tahoma font, and then >> probably disable most of the tests for MacOS, unless we can find a font that >> is common to all platforms -- searching around the web, I get the impression >> that the fonts "Arial", "Verdana" and "Times New Roman" may be present on >> all three platforms. >> >> The test isn't so much a test of the font rendering or engine, but a test of >> alignment and soft wrapping, so having it run on even one OS is a big win. >> Variations in font, sizes, etc, don't really change what code paths are hit, >> just the resulting widths will be off by a few pixels (still need a specific >> font to use as baseline though as the test widths are manually picked to >> match). > >> > And on Linux I get similar failures to my Windows system due to no Monaco >> > font. >> >> Was the Tahoma font available there (you can see from the `TextLayoutTest > >> fail()` test) ? > > It doesn't look like it: > > > org.opentest4j.AssertionFailedError: Check if this system test actually runs > on the build environment; > font loaded was [System Regular, System, Regular, System Regular, 12.0] > and [System Regular, System, Regular, System Regular, 12.0] > ==> expected: but was: > > >> ...searching around the web, I get the impression that the fonts "Arial", >> "Verdana" and "Times New Roman" may be present on all three platforms. > > That might make sense. Whichever font you do choose, you will want to add an > Assumption to check for its existence and skip the tests if not found. @kevinrushforth @andy-goryachev-oracle I've modified the test to use the Arial font instead of Monaco (hoping that at least Arial is available on all systems). The Tahoma font I kept intact as the original test is doing some special case with it (but it's only one test). With any luck, 95% of the tests work on all systems now. The test using Tahoma probably only works on a Windows system. I've added assumptions as well, so the tests are automatically skipped when fonts are returned that are unexpected. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1937076930
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Sat, 10 Feb 2024 03:58:24 GMT, John Hendrikx wrote: > > And on Linux I get similar failures to my Windows system due to no Monaco > > font. > > Was the Tahoma font available there (you can see from the `TextLayoutTest > > fail()` test) ? It doesn't look like it: org.opentest4j.AssertionFailedError: Check if this system test actually runs on the build environment; font loaded was [System Regular, System, Regular, System Regular, 12.0] and [System Regular, System, Regular, System Regular, 12.0] ==> expected: but was: > ...searching around the web, I get the impression that the fonts "Arial", > "Verdana" and "Times New Roman" may be present on all three platforms. That might make sense. Whichever font you do choose, you will want to add an Assumption to check for its existence and skip the tests if not found. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1937022815
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Sat, 10 Feb 2024 00:40:41 GMT, Kevin Rushforth wrote: > And on Linux I get similar failures to my Windows system due to no Monaco > font. Was the Tahoma font available there (you can see from the `TextLayoutTest > fail()` test) ? If so, I could rewrite most of the tests to use the Tahoma font, and then probably disable most of the tests for MacOS, unless we can find a font that is common to all platforms -- searching around the web, I get the impression that the fonts "Arial", "Verdana" and "Times New Roman" may be present on all three platforms. The test isn't so much a test of the font rendering or engine, but a test of alignment and soft wrapping, so having it run on even one OS is a big win. Variations in font, sizes, etc, don't really change what code paths are hit, just the resulting widths will be off by a few pixels (still need a specific font to use as baseline though as the test widths are manually picked to match). - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936846984
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Fri, 9 Feb 2024 23:48:15 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 one additional > commit since the last revision: > > Add failing test to confirm build is running the TextLayoutTest Here are the failures from our macOS 14.x headful test system (run on Jenkins). TextLayoutTest > fail() FAILED org.opentest4j.AssertionFailedError: Check if this system test actually runs on the build environment; font loaded was [Monaco, Monaco, Regular, Monaco, 12.0] and [Tahoma, Tahoma, Regular, Tahoma, 12.0] ==> expected: but was: at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55) at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40) at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210) at app//test.com.sun.javafx.text.TextLayoutTest.fail(TextLayoutTest.java:570) Failed to map supported failure 'org.opentest4j.AssertionFailedError: left aligned rich text (spans): line 0 for Parameters[text=The quick brown लोमड़ी jumps over the lazy कुत्ता, font=Font[name=Monaco, family=Monaco, style=Regular, size=12.0], wrapWidth=200.0, lineWidths=[189.89648, 122.583984], ascent=12.0, descent=4.001953] ==> expected: but was: ' with mapper 'org.gradle.api.internal.tasks.testing.failure.mappers.OpenTestAssertionFailedMapper@38b27cdc': Cannot invoke "Object.getClass()" because "expectedValue" is null Failed to map supported failure 'org.opentest4j.AssertionFailedError: left aligned rich text (spans): line 0 for Parameters[text=The quick brown लोमड़ी jumps over the lazy कुत्ता, font=Font[name=Monaco, family=Monaco, style=Regular, size=12.0], wrapWidth=200.0, lineWidths=[189.89648, 122.583984], ascent=12.0, descent=4.001953] ==> expected: but was: ' with mapper 'org.gradle.api.internal.tasks.testing.failure.mappers.OpenTestAssertionFailedMapper@38b27cdc': Cannot invoke "Object.getClass()" because "expectedValue" is null TextLayoutTest > caseTest(Case) > [10] SOFT_WRAP_WITH_COMPLEX_TEXT FAILED org.opentest4j.AssertionFailedError: left aligned rich text (spans): line 0 for Parameters[text=The quick brown लोमड़ी jumps over the lazy कुत्ता, font=Font[name=Monaco, family=Monaco, style=Regular, size=12.0], wrapWidth=200.0, lineWidths=[189.89648, 122.583984], ascent=12.0
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Sat, 10 Feb 2024 00:32:18 GMT, John Hendrikx wrote: > That's interesting, your local Windows 11 system does not have the Monaco > font (it is falling back to System Regular it seems). I guess I installed it > on mine at some point. Probably so. I just verified that I don't have Monaco on my Windows 11 system. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936773233
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Fri, 9 Feb 2024 23:48:15 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 one additional > commit since the last revision: > > Add failing test to confirm build is running the TextLayoutTest That's interesting, your local Windows 11 system does not have the Monaco font (it is falling back to System Regular it seems). I guess I installed it on mine at some point. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936771799
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Sat, 10 Feb 2024 00:16:33 GMT, John Hendrikx wrote: > Also I think the `TextLayoutTest` isn't really headful, it never displays > anything (no stages, uses no `Node`s), but it does need a real `PrismFont` -- > it may be difficult to get that to work though Yeah, without a proper headless glass toolkit (which Johan is working on), this will be difficult. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936770950
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Fri, 9 Feb 2024 23:48:15 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 one additional > commit since the last revision: > > Add failing test to confirm build is running the TextLayoutTest Results on my local Windows 11 system: $ gradle --info -PFULL_TEST=true :systemTests:test --tests TextLayoutTest ... > Task :systemTests:test FAILED TextLayoutTest > shouldIgnoreAlignmentWhenWrappingIsDisabled() FAILED org.opentest4j.AssertionFailedError: line 0 ==> expected: but was: at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55) at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62) at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182) at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1152) at app//test.com.sun.javafx.text.TextLayoutTest.assertLineBounds(TextLayoutTest.java:98) at app//test.com.sun.javafx.text.TextLayoutTest.shouldIgnoreAlignmentWhenWrappingIsDisabled(TextLayoutTest.java:314) TextLayoutTest > fail() FAILED org.opentest4j.AssertionFailedError: Check if this system test actually runs on the build environment; font loaded was [System Regular, System, Regular, System Regular, 12.0] and [Tahoma, Tahoma, Regular, Tahoma, 12.0] ==> expected: but was: at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55) at app//org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:40) at app//org.junit.jupiter.api.Assertions.assertTrue(Assertions.java:210) at app//test.com.sun.javafx.text.TextLayoutTest.fail(TextLayoutTest.java:570) TextLayoutTest > caseTest(Case) > [1] NO_WRAP FAILED org.opentest4j.AssertionFailedError: left aligned rich text (spans): line 0 for Parameters[text=The quick brown fox jumps over the lazy dog, font=Font[name=System Regular, family=System, style=Regular, size=12.0], wrapWidth=0.0, lineWidths=[309.6504], ascent=12.0, descent=4.001953] ==> expected: but was: at app//org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55) at app//org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62) at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Fri, 9 Feb 2024 23:48:15 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 one additional > commit since the last revision: > > Add failing test to confirm build is running the TextLayoutTest Also I think the `TextLayoutTest` isn't really headful, it never displays anything (no stages, uses no `Node`s), but it does need a real `PrismFont` -- it may be difficult to get that to work though - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936763262
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Sat, 10 Feb 2024 00:10:33 GMT, Kevin Rushforth wrote: > That's intentional. We can only run headless tests in the GHA build. We do > the same thing in our (Oracle's) CI build system. I was already pleasantly surprised that the three environments all had the correct Monaco and Tahoma font... guess I was being too optimistic, and the test simply never ran. > We do have a nightly Jenkins job that runs on physical systems that are set > up for headful testing, and we can similarly submit dev jobs that use those > machines. That's what I just did and will post the list of failing tests (if > any). We don't yet have a headful Windows machine, so I'll run that locally. Thanks, at least on my Windows they are all passing, but I can add assumes, or fix them for a different platform based on the failure output (that may take 2 or 3 passes as it doesn't display all failures in one go). - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936761354
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Sat, 10 Feb 2024 00:02:38 GMT, John Hendrikx wrote: > Okay, so I'm getting the impression that the system tests are not run during > the build (ie. `TextLayoutTest`). Is that an oversight or intended, or is my > test somehow not qualifying? > > clarification: I meant during the Github CI run, it does run during a normal > (local) build That's intentional. We can only run headless tests in the GHA build. We do the same thing in our (Oracle's) CI build system. We do have a nightly Jenkins job that runs on physical systems that are set up for headful testing, and we can similarly submit dev jobs that use those machines. That's what I just did and will post the list of failing tests (if any). We don't yet have a headful Windows machine, so I'll run that locally. - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936759766
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Fri, 9 Feb 2024 23:48:15 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 one additional > commit since the last revision: > > Add failing test to confirm build is running the TextLayoutTest Okay, so I'm getting the impression that the system tests are not run during the build (ie. `TextLayoutTest`). Is that an oversight or intended, or is my test somehow not qualifying? - PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936754982
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
On Fri, 9 Feb 2024 23:48:15 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 one additional
> commit since the last revision:
>
> Add failing test to confirm build is running the TextLayoutTest
We can't have tests that fail consistently on our CI systems. I'll do a headful
test run and post the results.
For each test that doesn't pass on some platforms, we have a three options:
1. If we can characterize a set of requirements (platform, font) that make that
test pass reliably, add the necessary JUnit5 Assumptions.
2. If the test passes most of the time, but is unstable, we can mark it as
unstable using `assumeTrue(Boolean.getBoolean("unstable.test"));`
3. Skip the failing test with `@Disabled`.
For the 2nd and 3rd options we need to reference an open bug ID. I suspect that
it will be some combination of 1 and 3.
-
PR Comment: https://git.openjdk.org/jfx/pull/1236#issuecomment-1936753990
Re: RFR: JDK-8314215 Trailing Spaces before Line Breaks Affect the Center Alignment of Text [v14]
> 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 one additional commit since the last revision: Add failing test to confirm build is running the TextLayoutTest - Changes: - all: https://git.openjdk.org/jfx/pull/1236/files - new: https://git.openjdk.org/jfx/pull/1236/files/cec1171a..00d83030 Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=1236&range=13 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=1236&range=12-13 Stats: 8 lines in 1 file changed: 8 ins; 0 del; 0 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
