On Sun, 30 Apr 2023 21:34:53 GMT, Rajat Mahajan <[email protected]> wrote:

>> **Problem:**
>> 
>> On pressing the Copy button we keep waiting in AWT-EventQueue thread in 
>> reconstruct() function as we detect that we have missing information for the 
>> animated image since we are copying single frame at a time.
>> Due to this infinite wait the application freezes.
>> 
>> **Proposed Fix:**
>> 
>> There are conditions in the reconstruct() function that avoid entering the 
>> wait() code if we have some error reading the image , etc. So, I added the 
>> condition to avoid entering the wait() code if we are copying single frame 
>> at a time. This sounded logical to me since if we have incomplete 
>> information(single frame) about the animated image we shouldn’t keep 
>> waiting, as it leads to infinite wait. 
>> After this change I see the GIF image being correctly copied and animated.
>> 
>> 
>> **Testing:**
>> 
>> Added a test for this (bug6176679.java) and tested locally on my Windows 
>> machine and on mach5.
>
> Rajat Mahajan has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - Update ImageRepresentation.java
>    
>    code cleanup
>  - Update ImageRepresentation.java
>    
>    code cleanup
>  -  update fix to work for both cases where image is displayed and not 
> displayed

> _Mailing list message from [Alan Snyder](mailto:[email protected]) on 
> [client-libs-dev](mailto:[email protected]):_
> 
> Your changes look reasonable.
> 
> I have a suggestion. Unless IDEA is lying to me, all uses of reconstruct() 
> pass ALLBITS as the parameter.
> 
> The net effect is to wait for any of the three terminating conditions, 
> ALLBITS, FRAMEBITS, and ERROR.
> 
> My suggestion would be to eliminate the parameter, add ALLBITS to the test in 
> the loop, and rename the method waitForTermination() or something like that.
> 
> (The existing method can hang if ALLBITS is not true in the parameter, which 
> makes the method unnecessarily fragile.)
> 
> > I am not sure what you mean by native image. Could you please clarify ?
> 
> I think I was mistaken. I tried again and was able to run your test.
> 
> -------------- next part -------------- An HTML attachment was scrubbed... 
> URL: 
> <https://mail.openjdk.org/pipermail/client-libs-dev/attachments/20230430/3e4de822/attachment-0001.htm>

Thanks Alan for this suggestion. I have created a new JBS bug 
[JDK-8307183](https://bugs.openjdk.org/browse/JDK-8307183) for refactoring this 
code as I thought it is not in scope of this bug and would do it as a follow up 
change to this one.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/13414#issuecomment-1530148379

Reply via email to