RFR: 8148334: The regression case: java/awt/print/PrinterJob/PrintTextTest.java failed as the printed pages do not match the shown pages [v2]

[Harshitha Onkar]

On Wed, 20 Nov 2024 13:15:29 GMT, Daniel Gredler <duke at openjdk.org> wrote:

>> @gredler Tested on windows with the latest fix. The portrait looks good but looks like the fix causes more issues in landscape mode. I'll require some time to go through the source code changes when I get a chance. Looks like you might be in the process of adding few more changes for Windows issue. Let me know once the PR is ready.
>> 
>> **Portrait**
>> 
>> <img width="1393" alt="Win_PortraitPg#8" src="https://github.com/user-attachments/assets/a6b48dfe-19f0-4739-825f-e50b0ae50128">
>> 
>> **Landscape**
>> 
>> <img width="1532" alt="Win_LandscapePg#8" src="https://github.com/user-attachments/assets/eeeb309f-fd36-4b8f-a77b-d53acc8a178b">
>
> @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). Let me know if you need any help with creating a new JBS issue. 
Since this test span 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 PR 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.

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

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