On Thu, 20 Apr 2023 19:26:52 GMT, Rajat Mahajan <rmaha...@openjdk.org> 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 one additional > commit since the last revision: > > Update CopyAnimatedGIFTest.java > _Mailing list message from [Alan Snyder](mailto:javali...@cbfiddle.com) on > [client-libs-dev](mailto:client-libs-...@mail.openjdk.org):_ > > I was hoping to be able to run your test program, but I cannot because on > macOS there is no support for transferring a native image. > > It also appears that I cannot simulate the transfer code because that code > depends upon the reconstruct method, which is not public. > > Just looking at the code, I think it is not correct. > > It looks to me that the ImageRepresentation.drawToBufImage method is called > only once, and it will do nothing unless ALLBITS or FRAMEBITS is true. I do > not see any loop that would call it multiple times. (If I am correct about > this, then only the first frame is drawn into the buffered image, which makes > much more sense than drawing every frame into the buffered image. One might > ask, however, what a user is trying to accomplish by copying an animated > image to the system clipboard?) > > So, it works if reconstruct() waits until ALLBITS or FRAMEBITS is true. But > your change does not test for FRAMEBITS in the loop, so it will only work if > FRAMEBITS is already true when reconstruct() is called. > > How can that happen? > > I think it works because you are displaying the image on the screen, and the > animation code is reading frames. > > I suggest trying the test without displaying the image. Thanks for your suggestions. I have modified the code so it works for both cases where image is and is not displayed on screen. I have modified the test so it tests for both these cases. > Also, your test would better if it retrieved the native image from the system > clipboard to verify that it was successfully transferred. You would need to > clear the system clipboard before running the test to be sure you are not > retrieving something from a previous test run. I am not sure what you mean by native image. Could you please clarify ? ------------- PR Comment: https://git.openjdk.org/jdk/pull/13414#issuecomment-1529141085
