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
