On Fri, 9 Feb 2024 12:56:15 GMT, Kevin Rushforth <k...@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

Reply via email to