[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-6601097:Margins are not reset to hardware margins when width/height
From: Jayathirth D V <jayathirth.d.v () oracle ! com>
Date: 2016-06-23 11:04:33
Message-ID: 0864dc39-d107-468e-b24e-cceac4e38bdf () default
[Download RAW message or body]
Hi Prasanta,
Changes are working fine.
Please convert multiple single line comments to multi-line comments both in code \
change in test case. In test case there are some places where lines are ending with \
more than 80 characters.
No need for review. Please change it before check-in.
Thanks,
Jay
From: Philip Race
Sent: Wednesday, June 22, 2016 9:08 PM
To: prasanta sadhukhan
Cc: 2d-dev@openjdk.java.net
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-6601097:Margins are not reset to hardware \
margins when width/height is 0 or -ve alongwith x, y
+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
HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.03/"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:
HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.02/"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
HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.01/"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: HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.00/"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 xmlns:v="urn:schemas-microsoft-com:vml" \
xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" \
xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type \
content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 \
(filtered medium)"><style><!-- /* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;
color:black;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p
{mso-style-priority:99;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:12.0pt;
font-family:"Times New Roman",serif;
color:black;}
pre
{mso-style-priority:99;
mso-style-link:"HTML Preformatted Char";
margin:0in;
margin-bottom:.0001pt;
font-size:10.0pt;
font-family:"Courier New";
color:black;}
span.HTMLPreformattedChar
{mso-style-name:"HTML Preformatted Char";
mso-style-priority:99;
mso-style-link:"HTML Preformatted";
font-family:Consolas;
color:black;}
span.new
{mso-style-name:new;}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body bgcolor=white lang=EN-US link=blue \
vlink=purple><div class=WordSection1><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Hi \
Prasanta,<o:p></o:p></span></p><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Changes are \
working fine.<o:p></o:p></span></p><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Please \
convert multiple single line comments to multi-line comments both in code change in \
test case. In test case there are some places where lines are ending with more than \
80 characters.<o:p></o:p></span></p><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>No need for \
review. Please change it before check-in.<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Thanks,<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Jay<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><div><div \
style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p \
class=MsoNormal><b><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext'>From:</span></b><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext'> Philip \
Race <br><b>Sent:</b> Wednesday, June 22, 2016 9:08 PM<br><b>To:</b> prasanta \
sadhukhan<br><b>Cc:</b> 2d-dev@openjdk.java.net<br><b>Subject:</b> Re: [OpenJDK \
2D-Dev] [9] RFR JDK-6601097:Margins are not reset to hardware margins when \
width/height is 0 or -ve alongwith x, y<o:p></o:p></span></p></div></div><p \
class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>+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: <o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p>Hi \
Phil,<o:p></o:p></p><p class=MsoNormal>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 \
href="http://cr.openjdk.java.net/%7Epsadhukhan/6601097/webrev.03/">http://cr.openjdk.j \
ava.net/~psadhukhan/6601097/webrev.03/</a><br><br>Regards<br>Prasanta<o:p></o:p></p><div><p \
class=MsoNormal>On 6/16/2016 5:11 AM, Philip Race \
wrote:<o:p></o:p></p></div><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal>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<o:p></o:p></p><pre><span class=new>if (iw <= \
0) iw = (float)(page.getPaper().getWidth()/DPI) - (ix*2);</span><o:p></o:p></pre><p \
class=MsoNormal><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: <o:p></o:p></p><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal>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 \
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><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p \
class=MsoNormal>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><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p \
class=MsoNormal>Hi Phil, <br><br>I got it rectified. <br><br>Please find the modified \
webrev <br><br><a 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><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p \
class=MsoNormal>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><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p \
class=MsoNormal style='margin-bottom:12.0pt'>Hi All, <br><br>Please review a fix for \
jdk9 which is a continuation of the fix of JDK-6543815. <br><br>Bug: <a \
href="https://bugs.openjdk.java.net/browse/JDK-6601097">https://bugs.openjdk.java.net/browse/JDK-6601097</a> \
<br>webrev: <a 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><o:p></o:p></p></blockquote></blockquote><p \
class=MsoNormal><o:p> </o:p></p></blockquote><p \
class=MsoNormal><o:p> </o:p></p></blockquote><p \
class=MsoNormal><o:p> </o:p></p></blockquote></blockquote><p \
class=MsoNormal><o:p> </o:p></p></blockquote></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic