[OpenJDK 2D-Dev] [9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page size set using PageFormat
Anton Litvinov
anton.litvinov at oracle.com
Mon Mar 27 16:35:10 UTC 2017
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.
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?
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/20170327/f5e3f8e4/attachment.html>
More information about the 2d-dev
mailing list