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

[Alexander Stepanov]

Thanks!

On 1/14/2016 2:41 PM, Sergey Bylokhov wrote:
> On 14/01/16 13:41, Alexander Stepanov wrote:
>> Hello Sergey,
>>
>> In such a case I prefer to have a separate need-test bug to cover all
>> the formats, please see
>> https://bugs.openjdk.java.net/browse/JDK-8147082
>
> ok, thanks. looks fine.
>
>>
>> This test was planned to cover tiff functionality (in the context of JEP
>> 262).
>>
>> Thanks,
>> Alexander
>>
>> On 1/13/2016 8:10 PM, Sergey Bylokhov wrote:
>>> On 12/01/16 19:53, Alexander Stepanov wrote:
>>>> So it seems the test logic could be used "as is" for the tiff format
>>>> only (as intended), and I'd like to keep it without sizable changes.
>>>
>>> yes but in past we had a situations when such assumption was broken in
>>> some new writers/readers: JDK-8146887, JDK-8144245, JDK-8145240. And
>>> the bugs were uncovered because the tests verify all installed 
>>> providers.
>>>
>>>>
>>>> Regards,
>>>> Alexander
>>>>
>>>> On 1/12/2016 6:18 PM, Alexander Stepanov wrote:
>>>>> 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.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>
>
>