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

[Philip Race]

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/86dc2136/attachment.html>