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

[Prasanta Sadhukhan]

Gentle Reminder to review..

Regards
Prasanta
On 7/20/2016 9:48 PM, Prasanta Sadhukhan wrote:
> 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/20160808/595800d6/attachment.html>