[OpenJDK 2D-Dev] RFR: 8145776: [TEST] add a test checking multipage tiff creation
Alexander Stepanov
alexander.v.stepanov at oracle.com
Tue Jan 12 15:18:58 UTC 2016
In such a case (canWriteSequence() && canWriteRasters()), I guess, we
return to the sole TIFF :)
On 1/12/2016 6:14 PM, Sergey Bylokhov wrote:
> 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.
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>
More information about the 2d-dev
mailing list