[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
(https://bugs.openjdk.java.net/browse/JDK-8177781).
Thank you,
Anton
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