<div dir="ltr"><div>Hi all,</div><div><br></div><div>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).<br></div><div><br></div><div>Thanks!</div><div><br></div><div>Daniel<br></div><div><br></div><div>[1] <a href="https://bugs.openjdk.org/browse/JDK-8339974">https://bugs.openjdk.org/browse/JDK-8339974</a></div><div>[2] <a href="https://github.com/openjdk/jdk/pull/20993">https://github.com/openjdk/jdk/pull/20993</a></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Sep 10, 2024 at 1:56 AM Daniel Gredler <<a href="mailto:djgredler@gmail.com">djgredler@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><div>Hi all,</div><div><br></div><div>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).</div><div><br></div><div>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.</div><div><br></div><div>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.<br></div><div><br></div><div>Two points I'd like some feedback on:</div><div></div><div>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?</div><div>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.<br></div><div><br></div><div>Take care,</div><div><br></div><div>Daniel<br></div><div><br></div><div>[1] <a href="https://github.com/openjdk/jdk/compare/master...gredler:jdk:draw-string-bug-2" target="_blank">https://github.com/openjdk/jdk/compare/master...gredler:jdk:draw-string-bug-2</a></div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Sep 5, 2024 at 8:30 PM Daniel Gredler <<a href="mailto:djgredler@gmail.com" target="_blank">djgredler@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi all,<br><br>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]<br><br>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.<br><br>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()?<br><br>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.<br><br>Take care,<br><br>Daniel<br><div><br></div><div>---<br></div><div><br></div>[1] Oracle internal review ID 9077538, no public bug ID yet<br>[2] <a href="https://github.com/openjdk/jdk/compare/master...gredler:jdk:draw-string-bug" target="_blank">https://github.com/openjdk/jdk/compare/master...gredler:jdk:draw-string-bug</a><br>[3]<br>import java.awt.Color;<br>import java.awt.Font;<br>import java.awt.Graphics2D;<br>import java.awt.geom.AffineTransform;<br>import java.awt.image.BufferedImage;<br>import java.io.File;<br><br>import javax.imageio.ImageIO;<br><br>/**<br> * <p>The "TEST 2" text does not draw. NOTE: Reducing the AffineTransform scale<br> * from 12 to 10 seems to allow it to draw?!?<br> *<br> * <p>Oracle internal review ID : 9077538<br> */<br>public class DrawStringTest {<br><br> public static void main(String[] args) throws Exception {<br><br> AffineTransform at = AffineTransform.getRotateInstance(3 * Math.PI / 2);<br> at.scale(12, 12);<br><br> Font font1 = new Font("SansSerif", Font.PLAIN, 10);<br> Font font2 = font1.deriveFont(at);<br><br> BufferedImage image = new BufferedImage(500, 500, BufferedImage.TYPE_BYTE_GRAY);<br> Graphics2D g2d = image.createGraphics();<br> g2d.setColor(Color.WHITE);<br> g2d.fillRect(0, 0, image.getWidth(), image.getHeight());<br> g2d.setColor(Color.BLACK);<br> g2d.setFont(font1);<br> g2d.drawString("TEST 1", 200, 400); // draws text<br> g2d.setFont(font2);<br> g2d.drawString("TEST 2", 200, 400); // does not draw text<br> g2d.dispose();<br><br> ImageIO.write(image, "png", new File("test.png"));<br> }<br><br><div>}</div><div><br></div></div>
</blockquote></div></div>
</blockquote></div>