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.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.
--
Best regards, Sergey.