[OpenJDK 2D-Dev] [9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page size set using PageFormat

Prasanta Sadhukhan prasanta.sadhukhan at oracle.com
Tue Mar 28 09:46:32 UTC 2017


Hi Anton,


On 3/27/2017 10:05 PM, Anton Litvinov wrote:
> Hello Prasanta,
>
> Indeed it is Mac specific, as it was written in my previous e-mail, I 
> verified this fact on Windows OS and Linux OS by your request.
>
> Yes, I am fully sure that, when the bug occurs, the paper size of the 
> printed page is wrong, as it is stated in the bug, and I am fully sure 
> that with the applied any of 2 versions of the created fix, the paper 
> size of the printed page becomes correct and as expected. Otherwise, I 
> would not send the fix for review. The instruction, by following which 
> the bug can be reproduced, is written in "STEPS TO FOLLOW TO REPRODUCE 
> THE PROBLEM :" section of the description of the bug in JBS. This bug 
> is raised by the user, who owns a support contract and requests for 
> resolution of this bug, this can be seen in proper comments of the bug 
> record.
>
> The automated regression test is not possible for this bug, since it 
> is necessary to verify visually that the document is physically 
> printed on the paper of the expected size. Otherwise, I would send the 
> 1st version of the fix with the automated test already, it is not the 
> first bug in JDK on which I have been working. By your request the 
> regression test, even though it is manual, was created and added to 
> the 2nd version of the fix.
>
> Yes, it is correct, the implemented by the test, and the test case 
> method "Printable.print(Graphics, PageFormat, int)" receives the 
> correct instance of "PageFormat" in runtime in scenario described in 
> this bug, because, as you already described, it is extracted using the 
> expression "pageable.getPageFormat(pageIndex)" in the Java method 
> "sun.lwawt.macosx.CPrinterJob.getPageformatPrintablePeekgraphics(int)" 
> called from "PrinterView.m::rectForPage:(NSInteger)" and further 
> transferred in this method to the Java method 
> "sun.lwawt.macosx.CPrinterJob.printAndGetPageFormatArea".
>
> The wrong paper size which is returned from the method 
> "RasterPrinterJob.getPageFormatFromAttributes()" and which is set to 
> certain fields of the Objective-C  object "NSPrintInfo" in the 
> function "CPrinterJob.m::javaPaperToNSPrintInfo" through the next call 
> sequence
>
> CPrinterJob.m::printLoop() --> 
> CPrinterJob.m::javaPrinterJobToNSPrintInfo() --> 
> CPrinterJob.m::javaPageFormatToNSPrintInfo() --> 
> CPrinterJob.m::javaPaperToNSPrintInfo()
>
> further influences physical size of all pages printed by 1 printer job.
>
> Thus, I consider that firstly "PageFormat" returned from 
> "RasterPrinterJob.getPageFormatFromAttributes()" is wrong, and 
> secondly setting paper size from this wrong "PageFormat" to proper 
> fields of "NSPrintInfo" object is also incorrect and is a root cause 
> of this bug.
OK. So, pageformat values set to native NSPrintInfo is different (wrong) 
compared to what is being passed to user.
>
> Implementation of Java Print Server API surely contains many issues, 
> and only working on this bug I already encountered 2 additional 
> different issues, which are described in one of my comments in this 
> bug record in JBS. However, I prefer to resolve issues separately and 
> to resolve this particular bug also separately from other issues which 
> we can indefinitely find while looking at the code and debugging it, 
> also because it is necessary to deliver the fix for this bug to 
> "jdk8u-dev" repository finally, while JDK 9 is currently in RDP 2 
> phase at which large fixes affecting more platforms or large code 
> scope would hardly be accepted by Group and Area leads or the release 
> team.
>
> I would like to bring also your attention again to the fact, which was 
> mentioned in my previous e-mail, that I already ran all manual and 
> automatic "jtreg" regression tests related to printing from open and 
> closed parts of JDK on JDK 9 without the fix and with the fix, what is 
> 198 tests.
>
> Do you find anything incorrect in the 2nd version of the fix? Will you 
> approve the 2nd version of the fix?
>
I think there might be a (probable) issue with this fix.  PageFormat of 
1st page only is used to get the information.

jobject page = JNFCallObjectMethod(env, srcPrinterJob, jm_getPageFormat, 
(jint)0);

What if user has set different pageformat to different page like this 
below? Will it not lose the 2nd page pageformat? I guess in 
windows/linux, we obtain pageformat for each pageindex

PrinterJob job = PrinterJob.getPrinterJob();
// Create a landscape pageformat
PageFormat pfl = job.defaultPage();
pfl.setOrientation(PageFormat.LANDSCAPE);
//Setup a book
Book bk = new Book();
bk.append(new xPrintable(), pfl); // 1st page landscape , can be diff. 
pagesize too
bk.append(new xxPrintable(), job.defaultPage()); //2nd page portrait
job.setPageable(bk);

Regards
Prasanta
> Thank you,
> Anton
>
> On 3/27/2017 9:52 AM, Prasanta Sadhukhan wrote:
>>
>> Hi Anton,
>>
>> Ok. So, it seems it mac specific. But, are you sure wrong page size 
>> is used in mac as is claimed in the bug?
>> I thought we could use automated regression test instead of manual by 
>> checking pageformat value in print() as compared to what user passes 
>> in setPrintable().
>>
>> Then, I see in print() method in testcase, the "PageFormat" argument 
>> passed has same values as it is passed in setPrintable() in main() 
>> even without your fix.
>>
>> If I reverse trace from print() method present in testcase, I see it 
>> is called from CPrinterJob.java#printAndGetPageFormatArea()
>> which is called from PrinterView.m#rectForPage. And before calling 
>> printAndGetPageFormatArea() it calls 
>> getPageformatPrintablePeekgraphics() which calls 
>> pageable.getPageFormat(pageIndex), so it should behave same as 
>> windows. Am I missing something?
>>
>> Regards
>> Prasanta
>> On 3/27/2017 3:32 AM, Anton Litvinov wrote:
>>> Hello Prasanta,
>>>
>>> I verified that the bug is not reproducible using JDK 9 compiled 
>>> from the latest version of "jdk9/client" forest neither on Windows, 
>>> nor on Linux platform, therefore, yes, this bug is only Mac 
>>> specific. Debugging showed that on Windows platform the behavior is 
>>> exactly like you described, on Windows 
>>> "RasterPrinterJob.print(PrintRequestAttributeSet)" method is 
>>> responsible for printing the documents and in this method 
>>> "RasterPrinterJob.printPage(Pageable, int)" is called for each page 
>>> separately, and in this "printPage" method "PageFormat" instance 
>>> used for printing of the page is extracted from the the transferred 
>>> as the method argument instance of "Pageable" by the expression 
>>> "origPage = document.getPageFormat(pageIndex);".
>>>
>>> The new version of the fix was created. Could you please review the 
>>> second version of the fix.
>>>
>>> Webrev (the 2nd version): 
>>> http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.01
>>>
>>> In the second version of the fix:
>>> 1. Calling for "RasterPrinterJob.getPageFormatFromAttributes()" was 
>>> substituted for "sun.lwawt.macosx.CPrinterJob.getPageFormat(int)" in 
>>> the native method "CPrinterJob.m#javaPrinterJobToNSPrintInfo".
>>> 2. The method "RasterPrinterJob.getPageFormatFromAttributes()" was 
>>> removed, since it is not called from any other code in "jdk" repository.
>>> 3. The manual regression test was created.
>>>
>>> Also on OS X I executed all manual and automatic "jtreg" regression 
>>> tests related to printing from the listed below directories and the 
>>> corresponding directories in the closed repositories using both JDK 
>>> 9 compiled without and with the fix, and I verified that no new test 
>>> failed on JDK 9 with the fix.
>>>
>>> The directories with the regression tests from open repositories:
>>> - "jdk/test/java/awt/print"
>>> - "jdk/test/javax/print"
>>>
>>> Thank you,
>>> Anton
>>>
>>> On 3/22/2017 7:42 AM, Prasanta Sadhukhan wrote:
>>>>
>>>> Hi Anton,
>>>>
>>>> I thought about your point and have a question: does this issue not 
>>>> reproduce in non-mac platform?
>>>> I thought it probably would so suggested modifying setAttributes() 
>>>> . But, if it is only reproduced in mac, we need to find out why 
>>>> despite this setAttributes() flaw, how this is working in non-mac 
>>>> platform?
>>>>
>>>> It probably might work in windows/linux because in 
>>>> RasterPrinterJob.print(attr) method, after it calls 
>>>> setAttributes(), it calls printPage() where it gets the original 
>>>> PageFormat
>>>> by calling getPageFormat(pageIndex) and gets the orientation, 
>>>> imageablearea
>>>> and not by constructing pageFormat from attributes as it is done in 
>>>> mac.
>>>> So, probably, in mac also, we need to do away with 
>>>> getPageFormatFromAttributes() call  and call 
>>>> getPageFormat(pageIndex) from 
>>>> CPrinterJob.m#javaPrinterJobToNSPrintInfo().
>>>>
>>>> Regards
>>>> Prasanta
>>>> On 3/21/2017 8:15 PM, Anton Litvinov wrote:
>>>>> Hello Prasanta,
>>>>>
>>>>> Thank you for review of this fix. I have tried to apply the 
>>>>> approach which you suggested and it allowed to resolve the issue. 
>>>>> In this case I agree that it would be more correct to resolve the 
>>>>> issue in 
>>>>> "RasterPrinterJob.setAttributes(PrintRequestAttributeSet)" method. 
>>>>> In the first version of the fix I decided to change the method 
>>>>> "RasterPrinterJob.getPageFormatFromAttributes()", because it is 
>>>>> invoked only from 1 place in JDK code and this place is located in 
>>>>> OS X specific code 
>>>>> "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m", 
>>>>> so that would automatically minimize the affected by the fix 
>>>>> platforms to OS X only.
>>>>>
>>>>> Starting to work on implementation of the second version of the 
>>>>> fix including the regression test.
>>>>>
>>>>> Thank you,
>>>>> Anton
>>>>>
>>>>> On 3/21/2017 12:52 PM, Prasanta Sadhukhan wrote:
>>>>>>
>>>>>> I think the real problem is not in 
>>>>>> RasterPrinterJob.getPageFormatFromAttributes().
>>>>>>
>>>>>> One can see that, when we call RasterPrinterJob.setPrintable(), 
>>>>>> we call updatePageAttributes() which in turn calls 
>>>>>> updateAttributesWithPageFormat() where orientation, media and 
>>>>>> mediaprintablearea "attributes" get populated/cached from the 
>>>>>> "pageformat" supplied by the user.
>>>>>>
>>>>>> But, when PrinterJob.print(PrintRequestAttributeSet) is called, 
>>>>>> it calls setAttributes(attributes) where "attributes" is what is 
>>>>>> populated by the user. It does not contain orientation,media and 
>>>>>> mediaprintablearea attributes for this bug, so setAttributes sets 
>>>>>> cached attribute with this user-set attribute instance
>>>>>> />>else {//
>>>>>> //>>            // for AWT where pageable is not an instance of 
>>>>>> OpenBook,//
>>>>>> //>>            // we need to save paper info//
>>>>>> // >>           this.attributes = attributes;//
>>>>>> // >>       }//
>>>>>> /
>>>>>>
>>>>>> //thereby losing the attributes it has cached earlier from user 
>>>>>> pageformat. I think we should check if this.attributes is not 
>>>>>> null, then retain those attributes unless explicitly set by the 
>>>>>> user in user-defined attributes.
>>>>>>
>>>>>> But, this code is used by windows,linux,mac so it needs to be 
>>>>>> tested against all those platforms to ensure we are not 
>>>>>> regressing anything.
>>>>>>
>>>>>> Also, I think user should call validatePage(PageFormat) in 
>>>>>> application to check if the settings passed is compatible with 
>>>>>> the printer or not, get compatible/adjusted pageformat and call 
>>>>>> setPrintable() with that "adjusted" pageformat. If user pass 
>>>>>> 0,0,fullpaperwidth,fullpaperheight as imageablearea, then he 
>>>>>> should not expect this setting to be used since printer will have 
>>>>>> its own hardware limitation (and use some offset printing)
>>>>>>
>>>>>> BTW, a regression testcase (even if it is manual) should be 
>>>>>> created with the fix.
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>> On 3/17/2017 10:59 PM, Anton Litvinov wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> Could you please review the following fix for the bug.
>>>>>>>
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8167102
>>>>>>> Webrev: 
>>>>>>> http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.00
>>>>>>>
>>>>>>> The bug consists in the fact that, if the user's application 
>>>>>>> specifies the required page size (media size) through 
>>>>>>> "java.awt.print.PrinterJob" API particularly via 
>>>>>>> "java.awt.print.PageFormat" instance supplied to the method 
>>>>>>> "PrinterJob.setPrintable(Printable, PageFormat)" and calls 
>>>>>>> "PrinterJob.print(PrintRequestAttributeSet)" with 
>>>>>>> "javax.print.attribute.PrintRequestAttributeSet" containing at 
>>>>>>> least 1 any "PrintRequestAttribute", then the page or pages will 
>>>>>>> be printed with "PageFormat" describing the default page size of 
>>>>>>> the printer which is different from the expected and originally 
>>>>>>> set by the user's application "PageFormat".
>>>>>>>
>>>>>>> Debugging showed that the new "PageFormat" with unexpected media 
>>>>>>> size is created in the method 
>>>>>>> "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" which 
>>>>>>> is invoked from the native function 
>>>>>>> "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m::javaPrinterJobToNSPrintInfo". 
>>>>>>> The method "RasterPrinterJob.getPageFormatFromAttributes()" 
>>>>>>> returns a new "PageFormat" always, if the provided by the user 
>>>>>>> "PrintRequestAttributeSet" is not empty, and the default values 
>>>>>>> are set to particular instance variables of this "PageFormat", 
>>>>>>> if "PrintRequestAttributeSet" does not contain the searched 
>>>>>>> "OrientationRequested", "MediaSizeName", "MediaPrintableArea" 
>>>>>>> attributes.
>>>>>>>
>>>>>>> THE SOLUTION:
>>>>>>> Although it is clearly stated in Java Platform SE 8 API 
>>>>>>> Specification (documentation of the method 
>>>>>>> "PrinterJob.print(PrintRequestAttributeSet)")
>>>>>>> Specification URL: 
>>>>>>> http://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet-
>>>>>>>
>>>>>>> that media size, orientation and imageable area attributes 
>>>>>>> specified in "PrintRequestAttributeSet" supplied to the method 
>>>>>>> "PrinterJob.print" will be used for construction of a new 
>>>>>>> "PageFormat", which will be passed to "Printable" object, 
>>>>>>> instead of the original "PageFormat" instance set through 
>>>>>>> "PrinterJob.setPrintable" method, the observed in this issue 
>>>>>>> behavior is a bug, because in the bug test case neither media 
>>>>>>> size, nor orientation, nor imageable area attributes are 
>>>>>>> specified in "PrintRequestAttributeSet".
>>>>>>>
>>>>>>> The fix alters the method 
>>>>>>> "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" to 
>>>>>>> use corresponding values from "PageFormat" instance originally 
>>>>>>> set through "PrinterJob" API during construction of the new 
>>>>>>> "PageFormat", when it is found out that "OrientationRequested", 
>>>>>>> or "MediaSizeName", or "MediaPrintableArea" attribute is not 
>>>>>>> specified in "PrintRequestAttributeSet" supplied to 
>>>>>>> "PrinterJob.print" method.
>>>>>>>
>>>>>>> Thank you,
>>>>>>> Anton
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20170328/7dcd4016/attachment.html>


More information about the 2d-dev mailing list