RFR: 8344637: Fix manual test java/awt/print/PrinterJob/PrintTextTest.java on Linux and Windows [v2]

Daniel Gredler duke at openjdk.org
Wed Nov 20 19:00:16 UTC 2024


On Wed, 20 Nov 2024 17:48:26 GMT, Harshitha Onkar <honkar at openjdk.org> wrote:

>> @honkar-jdk OK, I think this PR is ready for another review on Windows. The issue you highlighted in your last comment was caused by a partial fix to the JDK-8132890 regression (which had obviously removed the font rotation component, but had less-obviously also removed the device transform rotation component).
>> 
>> The goal is to completely fix this test on Windows and Linux, so I've updated the problem list to flag only macOS as problematic. Will we need a separate bug in the issue tracker, since this PR will not fix the macOS parts of JDK-8148334? I'm not sure whether PRs are allowed to partially fix issues (when they represent more than one bug), or whether they are expected to fully fix whatever issue they reference.
>> 
>> @prrace This is related to two bugs that you worked on in the past: JDK-8132890 and JDK-8029204. It has been a few years, but it would be good to get your review on this as well, if you have the time.
>> 
>> In particular, it feels like we stopped using most of the device transform in some parts of the JDK-8132890 changes (e.g. glyph position calculation), and now we're re-adding the rotation component, and I wonder if there are other parts of the device transform that we are missing, and/or whether it would be easier to go back to using the device transform in the glyph position calculation. (The code seems to work as-is, I'm just wondering if it's more complex than is necessary.)
>> 
>> Summary of changes:
>> 
>> * PathGraphics: Do not apply font transform translation component twice (results in incorrect text start point)
>> * WPathGraphics: Do not apply font transform translation component twice (results in incorrect text start point)
>> * WPathGraphics: Include both font and device rotation when calculating glyph positions
>> * WPathGraphics: Fix direction of y scale
>
> @gredler 
> 
>> Will we need a separate bug in the issue tracker, since this PR will not fix the macOS parts of JDK-8148334? 
> 
> That's correct. I think it is better to create a new JBS bug and use the new JBS ID for this PR than using [JDK-8148334](https://bugs.openjdk.org/browse/JDK-8148334). I can create a new JBS issue on your behalf.
> Since this test spans across multiple pages testing various glyph transformations it is better to keep [JDK-8148334](https://bugs.openjdk.org/browse/JDK-8148334) in open state and reference it while problemlisting the test.
> 
>> I'm not sure whether PRs are allowed to partially fix issues (when they represent more than one bug), or whether they are expected to fully fix whatever issue they reference.
> 
> A PR can have fix that focuses on specific issue and separate PRs can be created to address other issues. So in this case it seems reasonable to have a PR for Windows and Linux fix and create a separate one for macOS.

@honkar-jdk I just got access to JBS today, so that's not a problem. I've created JDK-8344637 to track just the Linux and Windows fixes in this PR (scoping out macOS for now). Have you had a chance to review on Windows?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21980#issuecomment-2489330915


More information about the client-libs-dev mailing list