Seems OK .. modulo Brian's off-list question about TIFF
-phil.
On 9/20/16, 10:57 PM, Jayathirth D V wrote:
Hi Prasanta,
Thanks for your review.
When we are writing embedded image we have internal WriteProgressAdapter in
which we have dummy listener functions which are doing nothing.
So there is no need to check for abortRequested() at 1340
processImageProgress(percentageDone); in BMP.
Regarding GIF change I want to keep abortRequested()checks right after
writeRows() or writeRowsOpt() for readability and debugging purpose. Only in
last else case we can merge the request which will look different from all
other if conditions.
I have modified test case to remove not needed catch clause. Please find
updated webrev for review:
http://cr.openjdk.java.net/~jdv/8164931/webrev.02/
Thanks,
Jay
-----Original Message-----
From: Prasanta Sadhukhan
Sent: Wednesday, September 21, 2016 11:00 AM
To: Jayathirth D V; Sergey Bylokhov; Philip Race; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8164931 : Verify if writer.abort()
works properly for all writers in IIOWriteProgressListener.
Is it not required to call abortRequested() after
1340 processImageProgress(percentageDone);
in BMP
In GIF 1044 if (abortRequested()) {
1045 return;
1046 } and 1054 if (abortRequested()) {
1055 return;
1056 }
can be merged and called after if-else block!!
In the testcase, I believe we do not need to have try-catch-finally block, just
a try-finally block should do as we are not doing anyprocessing in catch block.
Regards
Prasanta
On 9/16/2016 5:26 PM, Jayathirth D V wrote:
Hi Sergey,
Thanks for your review.
abortRequested in insert() should be at the end of the function only.
It has come by mistake while I was creating webrev as I was testing other
things.
Please find the updated webrev with abortRequested() check at the end of
insert() function for review:
http://cr.openjdk.java.net/~jdv/8164931/webrev.01/
Regards,
Jay
-----Original Message-----
From: Sergey Bylokhov
Sent: Friday, September 16, 2016 4:36 PM
To: Jayathirth D V; Philip Race; Prasanta Sadhukhan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8164931 : Verify if writer.abort()
works properly for all writers in IIOWriteProgressListener.
Looks fine, expect the changes in TIFFImageWriter.java. I am not sure
I understand why abortRequested was moved from the end of insert();
On 16.09.16 12:04, Jayathirth D V wrote:
Hi,
Please review the following fix in JDK9 at your convenience:
This issue is similar to
https://bugs.openjdk.java.net/browse/JDK-4924727 where we made
changes to all ImageReader plugins.
Bug : https://bugs.openjdk.java.net/browse/JDK-8164931
Webrev : http://cr.openjdk.java.net/~jdv/8164931/webrev.00/
Issue : Verify that when we issue ImageWriter.abort() in
IIOWriteProgressListener callbacks whether writing is aborted properly.
Root cause : In many writer plugins we are not checking for
abortRequested() right after IIOWriteProgressListener callbacks. In
which case writing may continue until we check for abortRequested().
Also in some writers we are not calling clearAbortRequest() before
every
write() call.
Solution : Check for abortRequested() after every
IIOWriteProgressListener callbacks and before every write() call we
should have clearAbortRequest() called.
In case of JPEG clearAbortRequest() is overridden in JPEGImageWriter
and it clears native abort flag also so there is no change in
JPEGImageWriter. WBMP changes will be done in JDK-8164930 as
checkSampleModel() is failing for WBMP.
Thanks,
Jay
--
Best regards, Sergey.
