On Tue, 1 Nov 2022 03:06:16 GMT, Phil Race <p...@openjdk.org> wrote:

> The analysis of the problem sounds fine but then it all gets confusing to me 
> when I come to the fix.
> 
> So first you capture the intended orientation in the switch but do not set it 
> to the printer context. 

The captured orientation is set to `dstPrintInfo` after code for enlarging the 
paper size in the end of the fix:

 [dstPrintInfo setOrientation:orientation];


> And I am lost as to why you do this NSPaperOrientation orientation = 
> [dstPrintInfo orientation];
> but promptly over-write the result in the switch statement.

Yes. The default value is not necessary for orientation in this case. I will 
update the code to `NSPaperOrientation orientation;`

> Then we get to this code :

    NSSize size = [dstPrintInfo paperSize];
    // Enlarge height only for cases 1 and 4.
    if ([dstPrintInfo orientation] == NS_LANDSCAPE && size.width > size.height) 
{
        size.height = size.width + ((orientation == NS_PORTRAIT) ? 1 : 0);
        [dstPrintInfo setPaperSize: size];
    }

> So case 1 is intended orientation of PORTRAIT but (1) You are only checking 
> for landscape and
 
The `dstPrintInfo` orientation value is checked after the paper size is applied 
but before the requested java orientation is set. The `dstPrintInfo` 
orientation at this point solely depends on the `dstPrintInfo` paper size. For 
the case 1 the `dstPrintInfo` orientation is landscape (width is greater than 
height).

> (2) You are doing it on the current printer context, not what is intended. 
> I am probably missing something but I'd like you to point out what it is.

This is the idea of the proposed fix. The fix checks  if the `dstPrintInfo`  
page size is consistent with the requested orientation (for example, paper size 
with width > height does not work with portrait orientation) and enlarge the 
size for this case.

> Also that last "+ .." condition is odd and un-explained.

For the case 1. where width is greater than height and `dstPrintInfo` 
orientation is landscape (w>h) enlarging the paper size to [width x width] 
leaves the `dstPrintInfo` orientation as landscape and setting the requested 
portrait orientation leads that the rotated image is printed. Enlarging the 
paper size to [width x (width + 1)] updates the `dstPrintInfo` orientation to 
portrait which allows to print non rotated image after setting the requested 
portrait orientation.
The idea of the "+" sign is to enlarge the paper size to be consistent with the 
requested orientation which is set after that.
 
> But then we get to the bigger question(s) Are all the settings in sync / 
> agreement after this ? Is the clip correct ? What if the changed size now 
> matches a different paper ? - The ability to set paper size at all here is a 
> relic of when there was no way to find out the real paper sizes. Papers which 
> don't exist should not be used. Continuous feed is probably the only reason 
> for this and there we get into more complications.

The proposed fix suggests some intermediate solution between the right behavior 
(requested paper size and orientation are used) and the initial behavior where 
the default page size was always used (before JDK-8167102 and its dependent 
fixes).
It is not clear for me how the right behavior can be implemented with 
NSPrintInfo class which does not allow to have both a paper size with width is 
greater than height and a portrait orientation at the same time.

I agree that it would be better to find the solution which does not affect the 
requested paper size.

To discuss the issue in general I prepared the pure Objective-C code which uses 
Cocoa NSPrint and NSPrintInfo classes to print an image and illustrates the 
issue with the paper size and orientation: 
[main.m](https://bugs.openjdk.org/secure/attachment/101275/main.m).
The instruction to run the code on macOS is described in 
[JDK-8295737](https://bugs.openjdk.org/browse/JDK-8295737?focusedCommentId=14533578&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-14533578).

The code first sets the NSPrintInfo size to 300x100 and the orientation to 
portrait:

    [printInfo setPaperSize: imageSize];
    [printInfo setOrientation: NS_PORTRAIT]; 

The output is:

 print info: set image size [300.000000, 100.000000]
 print info orientation: [1] NS_LANDSCAPE, size: [300.000000, 100.000000]
 print info: set orientation NS_PORTRAIT
 print info orientation: [0] NS_PORTRAIT, size: [100.000000, 300.000000] 


The papers size is changed from the requested 300x100 to 100x300.

If the order of setting the paper size and orientation is manually reversed in 
the code:

    [printInfo setOrientation: NS_PORTRAIT];
    [printInfo setPaperSize: imageSize];

The output is:

 print info: set orientation NS_PORTRAIT
 print info orientation: [0] NS_PORTRAIT, size: [612.000000, 792.000000]
 print info: set image size [300.000000, 100.000000]
 print info orientation: [1] NS_LANDSCAPE, size: [300.000000, 100.000000] 

The resulting orientation is landscape.

What is not clear for me, what is the right way to set both 300x100 paper size 
and the portrait orientation in this case?

> And why can't we handle all this by just applying an appropriate transform 
> rather than hacking a non-existent size ?

This is interesting idea. Could you give more details about it?

What I see that the issue is not that a printed output is rotated in some way.
The issue is that setting the paper size (width is greater than height) to 
NSPrintInfo first and the Portrait orientation after that leads to the swapped 
paper size in NSPrintInfo. So the image is correctly placed on the page but the 
paper size has been updated because it is not consistent with the requested 
orientation.
Could transformations improve the situation?

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

PR: https://git.openjdk.org/jdk/pull/10808

Reply via email to