[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: RFR: 6176679: Application freezes when copying an animated gif image to the system clipboard [v1
From:       Rajat Mahajan <rmahajan () openjdk ! org>
Date:       2023-04-30 21:36:22
Message-ID: DMz-FmTLIH7bVpGQd6R4GrYhjSGr2_cvYkxuuQs2hec=.791bb738-8e4f-433d-9542-b684dee24e6e () github ! com
[Download RAW message or body]

On Thu, 20 Apr 2023 19:26:52 GMT, Rajat Mahajan <rmahajan@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:javalists@cbfiddle.com) on \
> [client-libs-dev](mailto:client-libs-dev@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


[prev in list] [next in list] [prev in thread] [next in thread]