[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR: 8145776: [TEST] add a test checking multipage tiff creation
From: Alexander Stepanov <alexander.v.stepanov () oracle ! com>
Date: 2016-01-18 10:20:55
Message-ID: 569CBC87.80709 () oracle ! com
[Download RAW message or body]
Thanks!
On 1/18/2016 10:46 AM, Semyon Sadetsky wrote:
> Looks good.
>
> --Semyon
>
> On 1/11/2016 2:49 PM, Alexander Stepanov wrote:
>> Hello Semyon,
>>
>> Thank you, that eliminates the question with the internal package.
>> Probably Brian meant the same (but I just didn't understand this at
>> first).
>>
>> Please review the updated webrev:
>> http://cr.openjdk.java.net/~avstepan/8145776/webrev.02/
>>
>> Thanks,
>> Alexander
>>
>> On 1/11/2016 11:59 AM, Semyon Sadetsky wrote:
>>> Hello Alexander,
>>>
>>> Why don't use the reader method analogue for writer? Then the test
>>> could use the public API which is preferable.
>>>
>>> Just a cosmetic note. Lines should be wrapped at 80 position.
>>>
>>> --Semyon
>>>
>>> On 12/29/2015 3:44 PM, Alexander Stepanov wrote:
>>>> Hello Brian,
>>>>
>>>> Thank you for the notes, please see the updated webrev:
>>>> http://cr.openjdk.java.net/~yan/8145776/webrev.01/
>>>>
>>>> 1., 3. - fixed
>>>> WRT 2.: this import is necessary to use TIFFImageWriter,
>>>> TIFFImageWriterSpi. As it follows from
>>>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/07ae3247e988, the
>>>> majority of new classes (excepting tag sets) were added to
>>>> com.sun.imageio.plugins.tiff. Do you mean these classes would be
>>>> moved to javax/imageio soon? (please note also a @module tag added
>>>> for compatibility with modular java).
>>>>
>>>> Thanks,
>>>> Alexander
>>>>
>>>> On 12/29/2015 2:12 AM, Brian Burkhalter wrote:
>>>>> Hello Alexander,
>>>>>
>>>>> A few comments.
>>>>>
>>>>> 1) Lines 53,59: By convention it is better to put constants in
>>>>> upper case, e.g., NUM_IMAGES and BLACK_SIZE.
>>>>>
>>>>> 2) Line 45, 102: The test references the internal package
>>>>> com.sun.imageio.plugins.tiff. In general it is better to use the
>>>>> public packages.
>>>>>
>>>>> 3) Line 69: If the test fails, it would be good to know the random
>>>>> seed. It would also be good to be able to set the seed when
>>>>> running the test. The approach taken in core libraries may be seen
>>>>> in test/java/math/BigInteger/BigIntegerTest.java. The
>>>>> jdk.testlibrary.RandomFactory class is used to create the Random
>>>>> instance and to obtain any seed provided via Java properties. This
>>>>> requires the test tags “@library /lib/testlibrary” and “@build
>>>>> jdk.testlibrary.*” and it is also good to add “@key randomness.”
>>>>>
>>>>> Regards,
>>>>>
>>>>> Brian
>>>>>
>>>>> On Dec 25, 2015, at 5:10 AM, Alexander Stepanov
>>>>> <alexander.v.stepanov@oracle.com> wrote:
>>>>>
>>>>>> Could you please review the following fix
>>>>>> http://cr.openjdk.java.net/~yan/8145776/webrev.00/
>>>>>> <http://cr.openjdk.java.net/%7Eyan/8145776/webrev.00/>
>>>>>> for
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8145776
>>>>>>
>>>>>> Just a single test added checking the correctness of multi-page
>>>>>> TIFF image creation.
>>>>>
>>>>
>>>
>>
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Thanks!<br>
<br>
<div class="moz-cite-prefix">On 1/18/2016 10:46 AM, Semyon Sadetsky
wrote:<br>
</div>
<blockquote cite="mid:569C985D.4080500@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Looks good.<br>
<br>
--Semyon<br>
<br>
<div class="moz-cite-prefix">On 1/11/2016 2:49 PM, Alexander
Stepanov wrote:<br>
</div>
<blockquote cite="mid:569396CD.5070205@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Hello Semyon,<br>
<br>
Thank you, that eliminates the question with the internal
package. Probably Brian meant the same (but I just didn't
understand this at first).<br>
<br>
Please review the updated webrev:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eavstepan/8145776/webrev.02/">http://cr.openjdk.java.net/~avstepan/8145776/webrev.02/</a><br>
<br>
Thanks,<br>
Alexander<br>
<br>
<div class="moz-cite-prefix">On 1/11/2016 11:59 AM, Semyon
Sadetsky wrote:<br>
</div>
<blockquote cite="mid:56936EE5.70002@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Hello Alexander,<br>
<br>
Why don't use the reader method analogue for writer? Then the
test could use the public API which is preferable.<br>
<br>
Just a cosmetic note. Lines should be wrapped at 80 position.<br>
<br>
--Semyon<br>
<br>
<div class="moz-cite-prefix">On 12/29/2015 3:44 PM, Alexander
Stepanov wrote:<br>
</div>
<blockquote cite="mid:56828032.3040304@oracle.com" type="cite">
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
Hello Brian,<br>
<br>
Thank you for the notes, please see the updated webrev:<br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Eyan/8145776/webrev.01/">http://cr.openjdk.java.net/~yan/8145776/webrev.01/</a><br>
<br>
1., 3. - fixed<br>
WRT 2.: this import is necessary to use TIFFImageWriter,
TIFFImageWriterSpi. As it follows from <a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/07ae3247e988"><a \
class="moz-txt-link-freetext" \
href="http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/07ae3247e988">http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/07ae3247e988</a></a>,
the majority of new classes (excepting tag sets) were added
to com.sun.imageio.plugins.tiff. Do you mean these classes
would be moved to javax/imageio soon? (please note also a
@module tag added for compatibility with modular java).<br>
<br>
Thanks,<br>
Alexander<br>
<br>
<div class="moz-cite-prefix">On 12/29/2015 2:12 AM, Brian
Burkhalter wrote:<br>
</div>
<blockquote
cite="mid:AB0EB362-6C43-4DBD-BE9A-D5D96CC478D2@oracle.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
Hello Alexander,
<div><br>
</div>
<div>A few comments.</div>
<div><br>
</div>
<div>1) Lines 53,59: By convention it is better to put
constants in upper case, e.g., NUM_IMAGES and
BLACK_SIZE.</div>
<div><br>
</div>
<div>2) Line 45, 102: The test references the internal
package com.sun.imageio.plugins.tiff. In general it is
better to use the public packages.</div>
<div><br>
</div>
<div>3) Line 69: If the test fails, it would be good to
know the random seed. It would also be good to be able
to set the seed when running the test. The approach
taken in core libraries may be seen in
test/java/math/BigInteger/BigIntegerTest.java. The
jdk.testlibrary.RandomFactory class is used to create
the Random instance and to obtain any seed provided via
Java properties. This requires the test tags “@library
/lib/testlibrary” and “@build jdk.testlibrary.*” and it
is also good to add “@key randomness.” </div>
<div><br>
</div>
<div>Regards,</div>
<div><br>
</div>
<div>Brian</div>
<div><br>
<div>
<div>On Dec 25, 2015, at 5:10 AM, Alexander Stepanov
<<a moz-do-not-send="true"
class="moz-txt-link-abbreviated"
\
href="mailto:alexander.v.stepanov@oracle.com">alexander.v.stepanov@oracle.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite"><span style="font-family:
Helvetica; font-size: 12px; font-style: normal;
font-variant: normal; font-weight: normal;
letter-spacing: normal; line-height: normal;
orphans: auto; text-align: start; text-indent:
0px; text-transform: none; white-space: normal;
widows: auto; word-spacing: 0px;
-webkit-text-stroke-width: 0px; float: none;
display: inline !important;">Could you please
review the following fix</span><br
style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: 0px;">
<a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Eyan/8145776/webrev.00/"
style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: \
0px;">http://cr.openjdk.java.net/~yan/8145776/webrev.00/</a><br style="font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: 0px;">
<span style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: 0px; float: none;
display: inline !important;">for</span><br
style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: 0px;">
<a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8145776"
style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: \
0px;">https://bugs.openjdk.java.net/browse/JDK-8145776</a><br style="font-family: \
Helvetica; font-size: 12px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: 0px;">
<br style="font-family: Helvetica; font-size: 12px;
font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: 0px;">
<span style="font-family: Helvetica; font-size:
12px; font-style: normal; font-variant: normal;
font-weight: normal; letter-spacing: normal;
line-height: normal; orphans: auto; text-align:
start; text-indent: 0px; text-transform: none;
white-space: normal; widows: auto; word-spacing:
0px; -webkit-text-stroke-width: 0px; float: none;
display: inline !important;">Just a single test
added checking the correctness of multi-page TIFF
image creation.</span></blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</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