On 11/21/2014 9:20 PM, Phil Race wrote:
This seems to me to be asking about something I covered already.

>The latter one appears 'correct' in this case since applying it second
>fixes the output but I don't have enough information  to know why the
>values differ.

But you have the test case and I don't ..

Did you try any of what I suggested ?

The CPrinterJob.getPageFormat() returns right selected printer format. The problem is that RasterPrinterJob.attributeToPageFormat() method creates a default page with right page format and overrides page size/imageable area after that to predefined ones. This happens only because the CPrinterJob.printLoop() native method calls javaPrinterJobToNSPrintInfo(...) method after javaPageFormatToNSPrintInfo(...).

The JCK has a set of JTable print tests. I run them using the predefined page size/imageable area and with the selected printer settings. The page number is properly printed when the selected printer settings are used.

   I have updated the fix to preserve the selected printer page size:
     http://cr.openjdk.java.net/~alexsch/8044444/webrev.01/

  Thanks,
  Alexandr.



-phil.

On 11/18/14 8:17 AM, Alexander Scherbatiy wrote:


  Hi Phil,

Before the 8011069 fix RasterPrinterJob.getPageFormatFromAttributes() method returns null for null attributes
 and native page size for ImageableArea has been used.
After the 8011069 fix the attributes are not null and updateAttributesWithPageFormat() method rewrites
 the ImageableArea size to the default constants.

The question is which ImageableArea size is correct? If there should be used default values then the 8044444 is not an issue as all works as expected. If it is necessary to use native size then I can update the fix to do that.

  Thanks,
  Alexandr.


On 10/30/2014 11:10 PM, Phil Race wrote:
When we reach this code everything in the job is already configured by a combination of initial settings and user updates and and we just need to read
the settings and pass it on to the native NSPrintInfo.
So surely switching the order should not matter unless one of these
is using the 'wrong' PageFormat ?

-----------------
the body of the method called here :-

javaPrinterJobToNSPrintInfo(env, jthis, pageable, printInfo);

gets its PageFormat as follows :-

static JNF_MEMBER_CACHE(jm_getPageFormat, sjc_CPrinterJob, "getPageFormatFromAttributes", "()Ljava/awt/print/PageFormat;");
....

jobject page = JNFCallObjectMethod(env, srcPrinterJob, jm_getPageFormat);
    if (page != NULL) {
        javaPageFormatToNSPrintInfo(env, NULL, page, dst);
    }


So this uses the result of making a call to RasterPrinterJob.getPageFormatFromAttributes()

   protected PageFormat getPageFormatFromAttributes() {
       if (attributes == null) {
            return null;
        }
return attributeToPageFormat(getPrintService(), this.attributes);
   }

-----------------------------

whereas

javaPageFormatToNSPrintInfo(env, jthis, page, printInfo);

is using a PageFormat obtained as follows :-

static JNF_MEMBER_CACHE(jm_getPageFormat, sjc_CPrinterJob, "getPageFormat", "(I)Ljava/awt/print/PageFormat;");

jobject page = JNFCallObjectMethod(env, jthis, jm_getPageFormat, 0);

This is CPrinterJob.getPageFormat() { .. }


Although its not easily apparent what the returned values are in each of these cases
it does seem they must be different.

The latter one appears 'correct' in this case since applying it second
fixes the output but I don't have enough information  to know why the
values differ.

Looking at the fix for 8011069, it avoided an NPE by always
creating an 'attributes' map, albeit an empty one.
This can change the result of calling getPageFormatFromAttributes from
'null' to a PageFormat built from an empty attribute set.
If the no-args native printDialog() and the no-args print() call is used this will be empty.

So the method will indeed build - at that moment - a page format built from
default values.

Now. If we *do* use the printDialog(PrintRequestAttributeSet) and
print(PrintRequestAttributeSet) methods, then it may well be that this
method is the one that should be called.

And I think we were previously only in this block of code if that were the case by virtue of the block being guarded by "if (page != NULL)", which means there is an attributeset, which previously meant one of those "with args"
methods had been used.

So I wonder/suspect if the switching of the order will introduce the equivalent
problem in that 'with args' case.

As you can tell just looking at the webrev its nigh on impossible to tell
for sure and you'd probably need to play around with testing changing
paper size and orientation in native and cross-platform dialogs to test it.

You could start by seeing if the test 'passes' simply by switching to
'with args' before & after your fix - ensuring that the same paper sizes
are being used. I am not sure what the default settings were that were
created for the empty attribute set vs the ones that are used when you
fixed this. You'll have to tell me that.

Perhaps what is needed is a unified call to get the PageFormat which
can figure out whether to use the attributes or not. And you could
check if the call to CPrinterJob.getPageFormat() already performs that ..

-phil.


On 10/28/2014 01:03 PM, Alexander Scherbatiy wrote:

 Hello,

 Could you review the fix?

 Thanks,
 Alexandr.

On 7/15/2014 3:28 PM, Alexander Scherbatiy wrote:

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8044444
  webrev: http://cr.openjdk.java.net/~alexsch/8044444/webrev.00

Native method printLoop from CPrinterJob calls javaPrinterJobToNSPrintInfo(...) method after javaPageFormatToNSPrintInfo(...). Both methods set the page size. The initial page size is set in defaultPage(PageFormat) method. After the fix 8011069 the printDialog() initializes attributes which leads that new page size is created in the attributeToPageFormat(PrintService, PrintRequestAttributeSet) method.

The fix changes order of the javaPrinterJobToNSPrintInfo(...) javaPageFormatToNSPrintInfo(...) call so initial page size is set at the end.

  There is the JCK test that covers the issue.

Thanks,
Alexandr.






Reply via email to