On Wed, 20 Nov 2024 19:07:28 GMT, Daniel Gredler <dgred...@openjdk.org> wrote:
>> This PR fixes the issue identified in JDK-8148334 in screenshots >> `Page8_landscape.JPG` and `Page8_portrait.JPG`. >> >> It does not address `mac_Page1.png` or `mac_Page8.png`, which I'm not even >> sure are still issues (I have no access to a Mac). >> >> The method in question, `PathGraphics.printedSimpleGlyphVector(...)` is >> quite complex, with many special cases being handled in different ways. In >> this specific scenario (page 8 of `PrintTextTest`), all special case checks >> fail, and we fall through all the way to the final handling block, which >> draws the individual characters one by one. It looks like the problem is >> that the font transform translation is applied twice, once via the glyph >> positions, and again by `drawString(...)` via the font. The proposed fix is >> to provide `drawString(...)` a font without any translation transform. >> >> Testing looks good on Linux, but needs to be done on Mac and Windows. > > Daniel Gredler has updated the pull request incrementally with one additional > commit since the last revision: > > Add additional issue ID to PrintTextTest bug list Tested on windows, Pg#8 looks good. I did not notice any regressions on other pages with the fix. src/java.desktop/share/classes/sun/print/PathGraphics.java line 962: > 960: } > 961: } > 962: The fix to PathGraphics.java looks good. A new font var is used to copy over all the transforms except translation from the original font object and use the newly created font object in drawString(). The translation transform is skipped since the drawString takes care of x and y positions. src/java.desktop/windows/classes/sun/awt/windows/WPathGraphics.java line 873: > 871: AffineTransform.getScaleInstance(scaleFactorX, > scaleFactorY); > 872: advanceTransform.rotate(devangle); // radians > 873: advanceTransform.rotate(iangle * Math.PI / 1800.0); // 1/10 > degrees -> radians Fix to WPathGraphics.java is a little more intricate and certainly requires a more expert review from @prrace. Was devangle fix added for the landscape issue observed on Pg#8 ? A earlier comment mentions that device space rotation must not be added. advanceTransform.rotate(devangle); // radians advanceTransform.rotate(iangle * Math.PI / 1800.0); // 1/10 degrees -> radians ------------- PR Review: https://git.openjdk.org/jdk/pull/21980#pullrequestreview-2462961434 PR Review Comment: https://git.openjdk.org/jdk/pull/21980#discussion_r1859398722 PR Review Comment: https://git.openjdk.org/jdk/pull/21980#discussion_r1859413062
