[OpenJDK 2D-Dev] [9] RFR JDK-8161264: RasterPrinterJob.updateAttributesWithPageFormat does redundant check

[Prasanta Sadhukhan]

No, rest of it is needed as if iw/ih is -ve, then if we do not have this 
lines

if (iw <= 0) iw = (float)(page.getPaper().getWidth()/DPI) - (ix*2);
if (ih <= 0) ih = (float)(page.getPaper().getHeight()/DPI) - (iy*2);

then MediaPrintable constructor will throw IAE ((ignoring valid ix/iy)  
and we end up with Java default 1" margin without even trying to rectify 
iw/ih.

So, what we did was to TRY to rectify the -eve iw/ih to proper value 
first time
and even after that, if iw/ih is -ve then this webrev says that no need 
to reset to 0 as having iw/ih as -ve OR 0 will result in IAE.

Regards
Prasanta
On 7/20/2016 8:40 PM, Philip Race wrote:
> By that logic all the rest of it is unneeded too ... and
> the original bug was not a bug or the fix was not quite right.
> This is not adding up to me.
>
> -phil
>
> On 7/13/16, 1:53 AM, Prasanta Sadhukhan wrote:
>> Hi All,
>>
>> Please review a code cleanup for a redundant code introduced in the 
>> below 6601097 fix via webrev.03.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8161264
>> webrev: http://cr.openjdk.java.net/~psadhukhan/8161264/webrev.00/
>>
>> This code is redundant because irrespective of iw/ih being *negative* 
>> or *0*, MediaPrintableArea constructor called just after @line 692 
>> will throw IAE which is consumed
>> and thereafter this invalid mpa value is not propagated anymore and 
>> we will fallback to Java default paper margin of 1".
>> So, there is no need of checking for iw/ih for negative and set it to 0 .
>>
>> Regards
>> Prasanta
>> On 6/22/2016 9:07 PM, Philip Race wrote:
>>> +1 .. they won't get any output with a zero-width imageable area
>>> but this is last ditch fix up of excessively bad values.
>>>
>>> -phil.
>>>
>>> On 6/16/16, 11:32 PM, prasanta sadhukhan wrote:
>>>>
>>>> Hi Phil,
>>>>
>>>> Ok. I have added a check for this case in which it will fall back 
>>>> to default values since
>>>> if ix/iy is too large then we probably will not get anything to 
>>>> print inside printable area if we have to leave same margin on the 
>>>> right/bottom of the paper.
>>>> validatePaper() does not check for ix/iy too large case.
>>>>
>>>> Modified webrev
>>>> http://cr.openjdk.java.net/~psadhukhan/6601097/webrev.03/
>>>>
>>>> Regards
>>>> Prasanta
>>>> On 6/16/2016 5:11 AM, Philip Race wrote:
>>>>> I did say so long as the "ix/iy" are also valid. Which means not 
>>>>> just positive but that they
>>>>> are not too large. Consider
>>>>> if (iw <= 0) iw = (float)(page.getPaper().getWidth()/DPI) - (ix*2);
>>>>>
>>>>> if we have ix = 500 and iw = -20 for a paper with width 800 this 
>>>>> will result
>>>>> in iw = 800 - (500*2) = -200 ..
>>>>>
>>>>> -phil.
>>>>>
>>>>> On 6/8/16, 4:42 AM, prasanta sadhukhan wrote:
>>>>>> Hi Phil,
>>>>>>
>>>>>> As discussed offline, regarding mpa modification in both 
>>>>>> setAttributes() and updateAttributesWithPageFormat, I found that
>>>>>> updateAttributesWithPageFormat() will be called during 
>>>>>> pageDialog() and setPrintable()
>>>>>> whereas setAttributes() will be called during print() and 
>>>>>> setAttributes() called validatePaper() to validate imageable 
>>>>>> values, so in that regard, setAttributes() has final say in 
>>>>>> validating and updating invalid mpa values.
>>>>>>
>>>>>> Regarding bug, I found that if we have -ve width/height, 
>>>>>> MediaPrintable constructor throws IAE if width/height is -ve so 
>>>>>> mpa values set by user will not be added to pageAttributes (even 
>>>>>> if there was valid x,y mpa values)
>>>>>> therefore we fallback to Java default paper size and so we will 
>>>>>> get mpa values as ix=72 iy=72 iw=451 ih=697 in validatePaper()
>>>>>> so to avoid IAE and to use user-set valid values, I have modified 
>>>>>> the code to constrain iw/ih with requested ix/iy as you suggested.
>>>>>>
>>>>>> Please find the modified webrev:
>>>>>> http://cr.openjdk.java.net/~psadhukhan/6601097/webrev.02/
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>> On 5/31/2016 10:12 PM, Phil Race wrote:
>>>>>>> Well ... few printers can print on the entire paper. Photo 
>>>>>>> printers are
>>>>>>> the ones that can do this. So Paper dimension minus the 
>>>>>>> "hardware margins"
>>>>>>> are the maximum imageable width.
>>>>>>> And then supposing imageable x/y is some perfectly reasonable 
>>>>>>> value like 1" each
>>>>>>> then you've set iw/ih such that even a printer with zero 
>>>>>>> hardware margins has
>>>>>>> an imageable area that goes off the bottom and right off the paper.
>>>>>>>
>>>>>>> More reasonable would be to constrain iw/ih such that they work 
>>>>>>> with the
>>>>>>> requested ix/iy - assuming they are also valid.
>>>>>>>
>>>>>>> If all else fails then just using the "default" set of values as 
>>>>>>> if the application
>>>>>>> had not set any values would be better.
>>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>> On 05/26/2016 03:26 AM, prasanta sadhukhan wrote:
>>>>>>>> Hi Phil,
>>>>>>>>
>>>>>>>> I got it rectified.
>>>>>>>>
>>>>>>>> Please find the modified webrev
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/6601097/webrev.01/
>>>>>>>>
>>>>>>>> Regarding using entire width/height pf paper, I thought since 
>>>>>>>> imageable width/height is invalid we should make the entire 
>>>>>>>> paper as the imageable area.For invalid x,y we were making it 
>>>>>>>> to paper's top/left.
>>>>>>>> Else what option do we have, should we calculate 
>>>>>>>> width[height]=abs(image[width][height]) instead?
>>>>>>>>
>>>>>>>> Regards
>>>>>>>> Prasanta
>>>>>>>> On 5/25/2016 10:07 PM, Philip Race wrote:
>>>>>>>>> It seems to me that you are using the wrong units.
>>>>>>>>> You have not divided by DPI to get inches.
>>>>>>>>>
>>>>>>>>> Also I am not sure that the *entire* width/height of the paper 
>>>>>>>>> is what you want here
>>>>>>>>> but that is secondary to the first issue
>>>>>>>>>
>>>>>>>>> -phil
>>>>>>>>>
>>>>>>>>> On 5/19/16, 2:59 AM, prasanta sadhukhan wrote:
>>>>>>>>>> Hi All,
>>>>>>>>>>
>>>>>>>>>> Please review a fix for jdk9 which is a continuation of the 
>>>>>>>>>> fix of JDK-6543815.
>>>>>>>>>>
>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6601097
>>>>>>>>>> webrev: 
>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/6601097/webrev.00/
>>>>>>>>>>
>>>>>>>>>> 6543815 fix resets the x,y to 0 if they are negative before 
>>>>>>>>>> creating a MediaPrintableArea and the platform replaces it 
>>>>>>>>>> with hardware margins when printing.
>>>>>>>>>> This works only if x/y is negative.
>>>>>>>>>> But, If either width/height is negative alongwith x or y, 
>>>>>>>>>> then the margin is set to the java def 1 inch margin and not 
>>>>>>>>>> hardware margins.
>>>>>>>>>>
>>>>>>>>>> This is because width/height -ve results in IAE in 
>>>>>>>>>> MediaPrintableArea constructor and so values are ignored.
>>>>>>>>>> Added a check for -ve width/height to make sure width/height 
>>>>>>>>>> are set to minimum paper width/height.
>>>>>>>>>>
>>>>>>>>>> Regards
>>>>>>>>>> Prasanta
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20160720/a9308ba2/attachment.html>