<AWT Dev> [9] Review request for 8044444 The output's 'Page-n' footer does not show completely.
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Wed Dec 3 14:08:18 UTC 2014
On 12/1/2014 8:28 PM, Phil Race wrote:
> Hmm .. it looks as if this breaks the case when the Swing dialog is
> used, doesn't it ?
>
> I think this update needs to be accompanied by regression tests that
> show that
> this kind of page set up using native & swing dialogs both work.
> We can't easily use the JCK tests for this.
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/8044444/webrev.02
- The native imageable area is set to the page if it is not defined
in the set of attributes for the printer job.
- The manual test that checks printing with/without print dialog and
with/without media printable area properties is added.
Thanks,
Alexandr.
>
> -phil.
>
> On 11/27/14 7:45 AM, Alexander Scherbatiy wrote:
>> 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.
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the awt-dev
mailing list