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

Anton Litvinov anton.litvinov at oracle.com
Wed Mar 29 17:53:20 UTC 2017

Hello Prasanta,

Thank you for approval of the second version of the fix. As it was 
agreed, I have filed a new bug with a corresponding test case for the 
separate issue which we discussed. The filed bug is 

Thank you,

On 3/28/2017 5:23 PM, Prasanta Sadhukhan wrote:
> On 3/28/2017 6:25 PM, Anton Litvinov wrote:
>> Hello Prasanta,
>> The correct "PageFormat" for each separate page is retrieved in the 
>> native function "PrinterView.m::rectForPage:(NSInteger)" by invoking 
>> the Java method "CPrinterJob.getPageformatPrintablePeekgraphics(int)" 
>> with the first expression specified below and getting the value of 
>> "PageFormat" from the returned array with the second expression 
>> specified below.
>> "jobjectArray objectArray = JNFCallObjectMethod(env, fPrinterJob, 
>> jm_getPageformatPrintablePeekgraphics, jPageNumber); // AWT_THREADING 
>> Safe (AWTRunLoopMode)"
>> "jobject pageFormat = (*env)->GetObjectArrayElement(env, objectArray, 
>> 0);"
>> But in that native method "PrinterView.m::rectForPage" only the page 
>> orientation field "mOrientation" of the retrieved "PageFormat" for 
>> each page is analyzed and is set to "NSPrintInfo" object through 
>> "NSPrintOperation". Thus for some reason at that method analysis of 
>> the paper size is ignored.
>> So obviously not taking into account individual paper size of the 
>> pages for the case, which you described, when the user's code 
>> specifies different "PageFormat" instances for different pages of the 
>> document, should take place in JDK, though I did not verify this 
>> practically. But this issue existed before my fix and is not a result 
>> of the fix.
>> I do not see anything bad in the hard coded "0" page number used in 
>> my fix, because we need to initialize "NSPrintInfo" with a valid page 
>> size and the page size of the first page of the document is the only 
>> correct or appropriate value for this moment, and because this 
>> approach already exists in "RasterPrinterJob.java" as the next 
>> expression.
>> "PageFormat pf = (PageFormat)pageable.getPageFormat(0).clone();"
>> From my point of view, the issue about disrespect of different paper 
>> sizes for different pages of the document is the issue which is 
>> different from the issue described in this bug (JDK-8167102) and 
>> should be fixed separately from my bug, because:
>> - In my bug "java.awt.print.Printable" interface is involved, while 
>> in this new issue "java.awt.print.Pageable" interface is the explicit 
>> requirement;
>> - In my bug calling "PrinterJob.print(PrintRequestAttributeSet)" with 
>> a non-empty "PrintRequestAttributeSet" is the obligatory and the key 
>> requirement, while in the new issue this condition is irrelevant.
>> - For my bug one regression test is necessary, while for the new 
>> issue the different regression test is necessary.
>> Therefore I suggest to file a separate bug for the discovered issue 
>> and to address it separately from this bug (JDK-8167102). Would you 
>> agree with this suggestion? Would you approve the second version of 
>> the fix for the bug JDK-8167102?
> So long the other issue is addressed (even separately), I am ok with 
> this fix.
> Regards
> Prasanta
>> Thank you,
>> Anton
>> On 3/28/2017 12:46 PM, Prasanta Sadhukhan wrote:
>>> 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/20170329/169df177/attachment.html>

More information about the 2d-dev mailing list