[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] RFR JDK-6789262: getPageFormat doesn't apply PrintRequestAttributeSet speci
From:       Phil Race <philip.race () oracle ! com>
Date:       2016-06-30 21:59:03
Message-ID: 57759627.5010203 () oracle ! com
[Download RAW message or body]

Shouldn't these all be >= and <= ?
Else (eg) for X==1 in both cases you would return false when it should 
be true ..

f ((mpa.getX(units) > mediaPrintables[i].getX(units)) &&


-phil.


On 06/30/2016 03:31 AM, Prasanta Sadhukhan wrote:
> Hi Phil,
>
> I modified the method to check for "fit" rather than perfect match as 
> you suggested.
> Please find the modified webrev
> http://cr.openjdk.java.net/~psadhukhan/6789262/webrev.01/
>
> Regards
> Prasanta
> On 6/29/2016 11:29 PM, Philip Race wrote:
>> This is actually a little subtle.
>>
>> The check you are deleting has been there since 2000 ..
>> and it can't just be removed. But it need adjusting.
>> This is the subtlety. Most of the attribute checks
>> are yes/no. I think for this one we need to look at
>> the actual dimensions and return true if it is "fits"
>> So isSupportedMediaPrintableArea needs to do more than "equals"
>> I am not 100% sure that won't surprise some other code but
>> as far as I can think right now it seems the right thing to do.
>>
>> -phil.
>>
>> On 6/27/16, 4:27 AM, Prasanta Sadhukhan wrote:
>>> Hi All,
>>>
>>> Please review a fix for an issue where it was found that setting 
>>> left margin of 2" is ignored and when we query 
>>> PrinterJob.job.getPageFormat(PrintRequestAttributeSet)
>>> we get the default paper mediapritable area of x=1" instead of the 
>>> set 2".
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6789262
>>> webrev: http://cr.openjdk.java.net/~psadhukhan/6789262/webrev.00/
>>>
>>> This is because
>>> it was found that getPageFormat() calls 
>>> Win32PrintService#isAttributeValueSupported() where we call
>>> isSupportedMediaPrintableArea()
>>> to check if the mediaprintable area is matching with the media size 
>>> that has been cached. It gets the paperID by calling findPaperID() 
>>> and stores the mpa of that paper in MediaPrintableArea[]
>>> I see that my printer supports 43 mediasize names (like iso_a4, 
>>> iso_a5, na_letter etc) and none of the 43 mediaPrintables equals the 
>>> mpa value of (50.4, 25.4, 133.8, 246.2)mm
>>>
>>> ---------------isSupportedMediaPrintableArea()---------
>>> if (mediaPrintables != null) {
>>>             for (int i=0; i<mediaPrintables.length; i++) {
>>>                 if (mpa.equals(mediaPrintables[i])) {
>>>                     return true;
>>>                 }
>>>             }
>>>         }
>>> ----------------------
>>> So, it seems this check isSupportedMediaPrintableArea() is 
>>> preventing valid mpa values to be propagated to RasterPrinterJob to 
>>> be used for printing.
>>>
>>> I see in linux and mac, 
>>> *IPPPrintService*#isAttributeValueSupported() is called which does 
>>> not have this isSupportedMediaPrintableArea() check and there this 
>>> left margin of 2" is not ignored and stored properly.
>>>
>>> So, the proposed fix do away with this 
>>> isSupportedMediaPrintableArea() call from 
>>> Win32PrintService#isAttributeValueSupported.
>>>
>>> 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">
    <div class="moz-cite-prefix">Shouldn't these all be &gt;= and &lt;=
      ? <br>
      Else (eg) for X==1 in both cases you would return false when it
      should be true ..<br>
      <pre><span class="changed">f ((mpa.getX(units) &gt; \
mediaPrintables[i].getX(units)) &amp;&amp;</span></pre>  <br>
      -phil.<br>
      <br>
      <br>
      On 06/30/2016 03:31 AM, Prasanta Sadhukhan wrote:<br>
    </div>
    <blockquote cite="mid:5774F4EB.7050806@oracle.com" type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      Hi Phil,<br>
      <br>
      I modified the method to check for "fit" rather than perfect match
      as you suggested.<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/6789262/webrev.01/">http://cr.openjdk.java.net/~psadhukhan/6789262/webrev.01/</a><br>
  <br>
      Regards<br>
      Prasanta<br>
      <div class="moz-cite-prefix">On 6/29/2016 11:29 PM, Philip Race
        wrote:<br>
      </div>
      <blockquote cite="mid:57740C8B.10609@oracle.com" type="cite">
        <meta content="text/html; charset=UTF-8"
          http-equiv="Content-Type">
        This is actually a little subtle.<br>
        <br>
        The check you are deleting has been there since 2000 ..<br>
        and it can't just be removed. But it need adjusting.<br>
        This is the subtlety. Most of the attribute checks<br>
        are yes/no. I think for this one we need to look at<br>
        the actual dimensions and return true if it is "fits"<br>
        So isSupportedMediaPrintableArea needs to do more than "equals"<br>
        I am not 100% sure that won't surprise some other code but<br>
        as far as I can think right now it seems the right thing to do.<br>
        <br>
        -phil.<br>
        <br>
        On 6/27/16, 4:27 AM, Prasanta Sadhukhan wrote:
        <blockquote cite="mid:57710D96.8010808@oracle.com" type="cite">
          <meta http-equiv="content-type" content="text/html;
            charset=UTF-8">
          Hi All,<br>
          <br>
          Please review a fix for an issue where it was found that
          setting left margin of 2" is ignored and when we query
          PrinterJob.job.getPageFormat(PrintRequestAttributeSet)<br>
          we get the default paper mediapritable area of x=1" instead of
          the set 2".<br>
          <br>
          Bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
            href="https://bugs.openjdk.java.net/browse/JDK-6789262">https://bugs.openjdk.java.net/browse/JDK-6789262</a><br>
  webrev: <a moz-do-not-send="true"
            class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Epsadhukhan/6789262/webrev.00/">http://cr.openjdk.java.net/~psadhukhan/6789262/webrev.00/</a><br>
  <br>
          This is because<br>
          it was found that getPageFormat() calls
          Win32PrintService#isAttributeValueSupported() where we call<br>
          <pre><span class="removed">isSupportedMediaPrintableArea() </span></pre>
          <pre><span class="removed"></span></pre>
          <span class="removed">to check if the mediaprintable area is
            matching with the media size that has been cached. It gets
            the paperID by calling findPaperID() and stores the mpa of
            that paper in MediaPrintableArea[]<br>
            I see that my printer supports 43 mediasize names (like
            iso_a4, iso_a5, na_letter etc) and none of the 43 </span><span
            class="removed"><span class="removed">mediaPrintables </span>equals



            the mpa value of (50.4, 25.4, 133.8, 246.2)mm<br>
            <br>
          </span>---------------isSupportedMediaPrintableArea()---------<br>
          <span class="removed">if (mediaPrintables != null) {</span><br>
          <span class="removed">                       for (int i=0;
            i&lt;mediaPrintables.length; i++) {</span><br>
          <span class="removed">                               if
            (mpa.equals(mediaPrintables[i])) {</span><br>
          <span class="removed">                                       return \
                true;</span><br>
          <span class="removed">                               }</span><br>
          <span class="removed">                       }</span><br>
          <span class="removed">               }<br>
            ----------------------</span><span class="removed"><br>
            So, it seems this check isSupportedMediaPrintableArea() is
            preventing valid mpa values to be propagated to
            RasterPrinterJob to be used for printing.<br>
            <br>
            I see in linux and mac, <b>IPPPrintService</b>#</span><span
            class="removed">isAttributeValueSupported() is called which
            does not have this </span><span \
class="removed">isSupportedMediaPrintableArea</span><span  class="removed"></span>() \
check and there this left margin  of 2" is not ignored and stored properly.<br>
          <br>
          So, the proposed fix do away with this <span \
class="removed">isSupportedMediaPrintableArea()



          </span>call from Win32PrintService#isAttributeValueSupported.<br>
          <br>
          Regards<br>
          Prasanta<br>
          <span class="removed"><br>
          </span><br>
          <pre><span class="removed"></span></pre>
          <br>
          <br>
        </blockquote>
      </blockquote>
      <br>
    </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