[OpenJDK 2D-Dev] RFR: 8145776: [TEST] add a test checking multipage tiff creation

[Sergey Bylokhov]

On 12/01/16 18:08, Alexander Stepanov wrote:
>  > applicable to all image types which canWriteSequence=true?
> I'm not sure. E.g., for "gif" canWriteSequence() returns "true", but
> UnsupportedOperationException is thrown when calling writeInsert() (I
> have to double-check).

canWriteRasters should be checked also.

>
> (moreover, not sure if the gif created in such a way could be called
> "multipage" in the same sense as tiff).
>
>  > "import javax.imageio.plugins.tiff.*" is unnecessary
> yes, it should be removed, thanks.
>
> Regards,
> Alexander
>
> On 1/12/2016 5:36 PM, Sergey Bylokhov wrote:
>> Since "import javax.imageio.plugins.tiff.*" is unnecessary in the
>> test, i guess it is applicable to all image types which
>> canWriteSequence=true?
>>
>> On 11/01/16 14:49, 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 at 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.
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>


-- 
Best regards, Sergey.