<AWT Dev> [9] Review request for 8044444 The output's 'Page-n' footer does not show completely.

[Phil Race]

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 ?

-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.
>>>>
>>>
>>
>