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.stepa...@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.










Reply via email to