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

[John Hendrikx]

On Fri, 9 Feb 2024 12:56:15 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.

Looks good to me

modules/javafx.graphics/src/main/java/com/sun/prism/sw/SWGraphics.java line 664:

> 662:                 getRenderTarget().isOpaque() &&
> 663:                 (this.paint.getType() == Paint.Type.COLOR) &&
> 664:                 ((Color)this.paint).getAlpha() == 1.0f &&

Could avoid the cast here if you want:

Suggestion:

                this.paint instanceof Color c && 
                c.getAlpha() == 1.0f &&

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

> 94: 
> 95:     private static final Color transpColor = Color.color(0.5, 0.5, 0.5, 0.6);
> 96:     private static final Color opaqueColor = makeOpaque(transpColor);

minor: These look like constants to me, but are not in caps.

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

> 144:         int a = (int)(c.getOpacity() * 255.0);
> 145:         return "rgba(" + r + "," + g + "," + b + "," + a + ")";
> 146:     }

Color has a decent `toString` already

    @Override
    public String toString() {
        return "Color[" +
            "r=" + r +
            ", g=" + g +
            ", b=" + b +
            ", a=" + a +
            "]";
    }

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

Marked as reviewed by jhendrikx (Committer).

PR Review: https://git.openjdk.org/jfx/pull/1361#pullrequestreview-1872337676
PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484297571
PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484299457
PR Review Comment: https://git.openjdk.org/jfx/pull/1361#discussion_r1484301947