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