[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