Hi all, A bug number was issued for this issue [1], and I've created the corresponding PR [2]. Can a couple of reviewers spend a few cycles reviewing it? The fix is relatively simple, I think the main questions are more stylistic / cultural (simplicity vs. performance, compatibility constraints).
Thanks! Daniel [1] https://bugs.openjdk.org/browse/JDK-8339974 [2] https://github.com/openjdk/jdk/pull/20993 On Tue, Sep 10, 2024 at 1:56 AM Daniel Gredler <[email protected]> wrote: > Hi all, > > I've done a little more digging, and there is indeed a deeper bug in > TextLayout.getOutline(AffineTransform), where the transformation is applied > too early (before the layout path applies its mapping). > > There are two uses of TextLayout.getOutline(AffineTransform): the first > one, triggered via a simple Graphics2D.drawString( ) was the one I ran > into; the second one requires the use of a transformed font with a > PSPrinterJob. Both are broken, so it doesn't really make sense to work > around the TextLayout issue for the first scenario, but leave the second > scenario broken. > > The deeper fix, including two test classes verifying that both scenarios > were broken and now work correctly, is available on GitHub [1]. I plan to > submit a PR as soon as I have a final bug ID from my web bug submission. > > Two points I'd like some feedback on: > 1. I've changed the signature of method > TextLine.getOutline(AffineTransform), to just TextLine.getOutline( ); I'm > assuming this is OK since TextLine is a package-private class? > 2. I've added a fast path in TextLayout.getOutline(AffineTransform) to > transform the shape in place if it is a GeneralPath (it should usually be, > and avoids an extra Shape copy vs. the current code); does this look OK? Or > should the code be kept completely generic and a copy always made? I had a > look at just changing the variable type from Shape to GeneralPath, but the > change started to cascade into a few other classes and I pulled the plug. > > Take care, > > Daniel > > [1] > https://github.com/openjdk/jdk/compare/master...gredler:jdk:draw-string-bug-2 > > > > On Thu, Sep 5, 2024 at 8:30 PM Daniel Gredler <[email protected]> wrote: > >> Hi all, >> >> I've run into a bug [1] in the drawing of strings via >> Graphics2D.drawString(), when used with a Font derived from a rotated + >> scaled AffineTransform. If the AffineTransform scaling is small, the text >> draws correctly. But if the scale is too large, the text does not draw. [3] >> >> It looks like it's an effect of the OutlineTextRenderer.THRESHHOLD; text >> larger than this threshold is sent to the OutlineTextRenderer and fails to >> draw. The fix [2] seems to be to apply the position translation >> transformation *after* the text outline shape is created, rather than >> passing it into TextLayout.getOutline(). However, I do not know if this is >> the correct fix, or if it is just a workaround to a deeper issue in >> TextLayout.getOutline(), which in theory might need to wait to apply the >> provided translation until after the layout path has had a chance to map to >> user space. >> >> Can someone familiar with this code comment on whether the proposed fix >> is the correct fix, or whether deeper surgery is needed in >> TextLayout.getOutline()? >> >> My other small worry with the current fix [2] is that it creates an extra >> copy of the Shape object, so it's going to allocate a bit more memory than >> the current implementation. Not sure if there's a better way to update the >> Shape in-place. >> >> Take care, >> >> Daniel >> >> --- >> >> [1] Oracle internal review ID 9077538, no public bug ID yet >> [2] >> https://github.com/openjdk/jdk/compare/master...gredler:jdk:draw-string-bug >> [3] >> import java.awt.Color; >> import java.awt.Font; >> import java.awt.Graphics2D; >> import java.awt.geom.AffineTransform; >> import java.awt.image.BufferedImage; >> import java.io.File; >> >> import javax.imageio.ImageIO; >> >> /** >> * <p>The "TEST 2" text does not draw. NOTE: Reducing the AffineTransform >> scale >> * from 12 to 10 seems to allow it to draw?!? >> * >> * <p>Oracle internal review ID : 9077538 >> */ >> public class DrawStringTest { >> >> public static void main(String[] args) throws Exception { >> >> AffineTransform at = AffineTransform.getRotateInstance(3 * >> Math.PI / 2); >> at.scale(12, 12); >> >> Font font1 = new Font("SansSerif", Font.PLAIN, 10); >> Font font2 = font1.deriveFont(at); >> >> BufferedImage image = new BufferedImage(500, 500, >> BufferedImage.TYPE_BYTE_GRAY); >> Graphics2D g2d = image.createGraphics(); >> g2d.setColor(Color.WHITE); >> g2d.fillRect(0, 0, image.getWidth(), image.getHeight()); >> g2d.setColor(Color.BLACK); >> g2d.setFont(font1); >> g2d.drawString("TEST 1", 200, 400); // draws text >> g2d.setFont(font2); >> g2d.drawString("TEST 2", 200, 400); // does not draw text >> g2d.dispose(); >> >> ImageIO.write(image, "png", new File("test.png")); >> } >> >> } >> >>
