[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8161264: RasterPrinterJob.updateAttributesWithPageFormat does redun
From: Prasanta Sadhukhan <prasanta.sadhukhan () oracle ! com>
Date: 2016-07-20 16:30:57
Message-ID: 578FA471.3090106 () oracle ! com
[Download RAW message or body]
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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
No, rest of it is needed as if iw/ih is -ve, then if we do not have
this lines<br>
<pre>if (iw <= 0) iw = (float)(page.getPaper().getWidth()/DPI) - (ix*2);
if (ih <= 0) ih = (float)(page.getPaper().getHeight()/DPI) - (iy*2);
</pre>
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.<br>
<br>
So, what we did was to TRY to rectify the -eve iw/ih to proper value
first time<br>
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.<br>
<br>
Regards<br>
Prasanta<br>
<div class="moz-cite-prefix">On 7/20/2016 8:40 PM, Philip Race
wrote:<br>
</div>
<blockquote cite="mid:578F9468.6020805@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
By that logic all the rest of it is unneeded too ... and<br>
the original bug was not a bug or the fix was not quite right.<br>
This is not adding up to me.<br>
<br>
-phil<br>
<br>
On 7/13/16, 1:53 AM, Prasanta Sadhukhan wrote:
<blockquote cite="mid:57860199.8010208@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hi All,<br>
<br>
Please review a code cleanup for a redundant code introduced in
the below <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.03/">6601097</a>
fix via webrev.03.<br>
<br>
Bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8161264">https://bugs.openjdk.java.net/browse/JDK-8161264</a><br>
webrev: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/8161264/webrev.00/">http://cr.openjdk.java.net/~psadhukhan/8161264/webrev.00/</a><br>
<br>
This code is redundant because irrespective of iw/ih being <b>negative</b>
or <b>0</b>, MediaPrintableArea constructor called just after
@line 692 will throw IAE which is consumed <br>
and thereafter this invalid mpa value is not propagated anymore
and we will fallback to Java default paper margin of 1".<br>
So, there is no need of checking for iw/ih for negative and set
it to 0 .<br>
<br>
Regards<br>
Prasanta<br>
<div class="moz-cite-prefix">On 6/22/2016 9:07 PM, Philip Race
wrote:<br>
</div>
<blockquote cite="mid:576AB0BD.7050506@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
+1 .. they won't get any output with a zero-width imageable
area<br>
but this is last ditch fix up of excessively bad values.<br>
<br>
-phil.<br>
<br>
On 6/16/16, 11:32 PM, prasanta sadhukhan wrote:
<blockquote
cite="mid:48d2a342-851a-0a79-1443-c7bf82958f7b@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<p>Hi Phil,<br>
</p>
Ok. I have added a check for this case in which it will fall
back to default values since <br>
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.<br>
validatePaper() does not check for ix/iy too large case.<br>
<br>
Modified webrev<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.03/">http://cr.openjdk.java.net/~psadhukhan/6601097/webrev.03/</a><br>
<br>
Regards<br>
Prasanta<br>
<div class="moz-cite-prefix">On 6/16/2016 5:11 AM, Philip
Race wrote:<br>
</div>
<blockquote cite="mid:5761E7A9.1010805@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
I did say so long as the "ix/iy" are also valid. Which
means not just positive but that they<br>
are not too large. Consider<br>
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
<pre><span class="new">if (iw <= 0) iw = \
(float)(page.getPaper().getWidth()/DPI) - (ix*2);</span></pre> <br>
if we have ix = 500 and iw = -20 for a paper with width
800 this will result<br>
in iw = 800 - (500*2) = -200 ..<br>
<br>
-phil.<br>
<br>
On 6/8/16, 4:42 AM, prasanta sadhukhan wrote:
<blockquote
cite="mid:0703d17a-9e36-94a2-93f8-80c56d0250f3@oracle.com"
type="cite">Hi Phil, <br>
<br>
As discussed offline, regarding mpa modification in both
setAttributes() and updateAttributesWithPageFormat, I
found that <br>
updateAttributesWithPageFormat() will be called during
pageDialog() and setPrintable() <br>
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.
<br>
<br>
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)
<br>
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() <br>
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. <br>
<br>
Please find the modified webrev: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.02/">http://cr.openjdk.java.net/~psadhukhan/6601097/webrev.02/</a>
<br>
<br>
Regards <br>
Prasanta <br>
On 5/31/2016 10:12 PM, Phil Race wrote: <br>
<blockquote type="cite">Well ... few printers can print
on the entire paper. Photo printers are <br>
the ones that can do this. So Paper dimension minus
the "hardware margins" <br>
are the maximum imageable width. <br>
And then supposing imageable x/y is some perfectly
reasonable value like 1" each <br>
then you've set iw/ih such that even a printer with
zero hardware margins has <br>
an imageable area that goes off the bottom and right
off the paper. <br>
<br>
More reasonable would be to constrain iw/ih such that
they work with the <br>
requested ix/iy - assuming they are also valid. <br>
<br>
If all else fails then just using the "default" set of
values as if the application <br>
had not set any values would be better. <br>
<br>
-phil. <br>
<br>
On 05/26/2016 03:26 AM, prasanta sadhukhan wrote: <br>
<blockquote type="cite">Hi Phil, <br>
<br>
I got it rectified. <br>
<br>
Please find the modified webrev <br>
<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
\
href="http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.01/">http://cr.openjdk.java.net/~psadhukhan/6601097/webrev.01/</a>
<br>
<br>
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. <br>
Else what option do we have, should we calculate
width[height]=abs(image[width][height]) instead? <br>
<br>
Regards <br>
Prasanta <br>
On 5/25/2016 10:07 PM, Philip Race wrote: <br>
<blockquote type="cite">It seems to me that you are
using the wrong units. <br>
You have not divided by DPI to get inches. <br>
<br>
Also I am not sure that the *entire* width/height
of the paper is what you want here <br>
but that is secondary to the first issue <br>
<br>
-phil <br>
<br>
On 5/19/16, 2:59 AM, prasanta sadhukhan wrote: <br>
<blockquote type="cite">Hi All, <br>
<br>
Please review a fix for jdk9 which is a
continuation of the fix of JDK-6543815. <br>
<br>
Bug: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
\
href="https://bugs.openjdk.java.net/browse/JDK-6601097">https://bugs.openjdk.java.net/browse/JDK-6601097</a>
<br>
webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
\
href="http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.00/">http://cr.openjdk.java.net/~psadhukhan/6601097/webrev.00/</a>
<br>
<br>
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. <br>
This works only if x/y is negative. <br>
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.
<br>
<br>
This is because width/height -ve results in IAE
in MediaPrintableArea constructor and so values
are ignored. <br>
Added a check for -ve width/height to make sure
width/height are set to minimum paper
width/height. <br>
<br>
Regards <br>
Prasanta <br>
<br>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic