RFR: 8311492: FontSmoothingType LCD produces wrong color when transparency is used [v2]

[Andy Goryachev]

On Fri, 9 Feb 2024 14:29:18 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> JavaFX LCD text rendering (aka sub-pixel antialiasing) uses a pixel shader and alpha blending. The alpha channel is used is ways that interfere with its use for transparency. The existing logic checks that the current blend equation is SRC_OVER and that the surface is opaque, and that we are rendering using a Paint of type Color. It fails to check that the text color is opaque. When it isn't, the resulting alpha value is not preserved, even in the middle of the filled portion of the text, resulting in a visually noticeable difference in color.
>> 
>> ![transparent-lcd-text](https://github.com/openjdk/jfx/assets/34689748/81f9503c-c706-44d8-b4db-6b47f0eb5fea)
>> 
>> The solution is to add the missing check for alpha == 1 to the test that checks whether we can use LCD text rendering. I note that Java2D falls back to gray scale when the text color is transparent for a similar reason.
>> 
>> I added a robot test that checks the color in the middle of the filled portion of a rendered text character and also checks that we use LCD for opaque colors and GRAY scale for transparent colors.
>
> Kevin Rushforth has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review feedback

tests/system/src/test/java/test/robot/javafx/scene/TransparentLCDTest.java line 67:

> 65:  *
> 66:  * @test
> 67:  * @bug 8311492

for my education: what is the meaning of these tags?  how are they used?

tests/system/src/test/java/test/robot/javafx/scene/TransparentLCDTest.java line 81:

> 79:     // color, or in the middle of the text fill area, where we expect to find
> 80:     // the unadjusted text color.
> 81:     private static final double TOLERANCE = 2.0 / 255.0;

if the intent is to allow for 2 levels of difference, perhaps it should be (2.0 / 255) + 0.00001 or something

tests/system/src/test/java/test/robot/javafx/scene/TransparentLCDTest.java line 190:

> 188:     @AfterEach
> 189:     public void doTeardown() {
> 190:         Platform.runLater(() -> {

should this be Util.runAndWait() ?

tests/system/src/test/java/test/robot/javafx/scene/TransparentLCDTest.java line 272:

> 270:         runTest(true);
> 271:     }
> 272: 

extra newline?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484506628
PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484509375
PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484512177
PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484513289