[OpenJDK 2D-Dev] [9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page size set using PageFormat
Prasanta Sadhukhan
prasanta.sadhukhan at oracle.com
Mon Mar 27 06:52:31 UTC 2017
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/464c51cf/attachment.html>
More information about the 2d-dev
mailing list