On 9/21/2016 11:27 AM, 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:

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

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 
Please find the updated webrev with abortRequested() check at the end of 
insert() function for review:


-----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:

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



Best regards, Sergey.

Reply via email to