Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Animations are not drawn #376

Open
daell82 opened this issue Feb 13, 2024 · 25 comments · May be fixed by #445
Open

Animations are not drawn #376

daell82 opened this issue Feb 13, 2024 · 25 comments · May be fixed by #445

Comments

@daell82
Copy link

daell82 commented Feb 13, 2024

Hello,

after looking into the flow example again, I've noticed that the animations for the graph layout are not drawn anymore. I was able to track it down to the changes introduced in #127. This also affects the animations for palette drawers and animated zoom.

Currently, for me this is reproducible on Windows. On Linux I am not sure whether this was ever supported.

@ptziegler
Copy link
Contributor

Can you perhaps post a screenshot of what you mean?

I've reverted the change locally, but I couldn't see any differences. Though that might partially be due to me using Linux, which you've already hinted at.

@daell82
Copy link
Author

daell82 commented Feb 18, 2024

Yes, for the recording I've duplicated the drawer in the palette, otherwise it is not collapsible.

When the canvas.update() is omitted, the animations are still processed for their delay but not drawn, which also feels a little bit "laggy".

animations.mp4

@ptziegler
Copy link
Contributor

ptziegler commented Feb 18, 2024

Ah, so that's what you mean. Thanks for the example!

Hm, it doesn't look like those animations are played at all on Linux, regardless of whether the update method is called...
I have to dig up my old Windows laptop to see what's going on here.

@daell82
Copy link
Author

daell82 commented Feb 18, 2024

No problem.

On the other hand, I was not able to reproduce the flickering issues mentioned in #127 on Windows. Maybe it could be a quick solution, if the canvas.update() is only performed on that platform.

@azoitl
Copy link
Contributor

azoitl commented Feb 18, 2024

I also noticed that with the improvements of SWT we sometimes see improvements in Draw2d too.

@Phillipus can you please comment here as well?

@Phillipus
Copy link
Contributor

Phillipus commented Feb 18, 2024

I don't think the changes introduced in #127 have anything to do with animations. What I found was that SWT needs time to call Display.getDefault().readAndDispatch() to update the animation. See https://github.com/archimatetool/archi/blob/master/org.eclipse.draw2d/src/org/eclipse/draw2d/Animation.java

@Phillipus
Copy link
Contributor

Phillipus commented Feb 18, 2024

Actually, it seems that reverting #127 allows animation to work on Windows, but animation is not working on Mac and Linux which is why I modified the Animation class. So, even if #127 is reverted it won't fix the animation problem on all platforms.

@Phillipus
Copy link
Contributor

You could try #377 to see if that works.

@daell82
Copy link
Author

daell82 commented Feb 18, 2024

That change fixes the palette animations. 👍

For the flow-example the graph layout uses it's own static animation constructs. When I add Display.getDefault().readAndDispatch(); to the static boolean playbackState(IFigure container) method in GraphAnimation.java@146 before the last return statement, it is animated as well.

Update: I am using the animatable zoom from GMF in my application, which is not animated with the fix.

@Phillipus
Copy link
Contributor

As of now, I see these options:

  1. Revert Fix drag operation unwanted artifacts #127 which will allow animations to work on Windows but not Mac or Linux. This will have the side-effect of re-introducing unwanted artifacts on a Figure with some drag operations (bits of feedback are only partially erased). We see these artifacts in Archi on Windows if that fix is not applied.
  2. Apply something like Fix Animation not working #377 which I think will fix animations on all platforms.

Update: I am using the animatable zoom from GMF in my application, which is not animated with the fix.

Is it animated when reverting #127?

@daell82
Copy link
Author

daell82 commented Feb 19, 2024

Yes, it does, but having animations on Windows and flickering without animations on other platforms is somewhat undesirable..

I've opened a PR #378 which puts the Display.readAndDispatch into the DeferredUpdateManager.performUpdate. This shows animations on Windows and on Linux. But I am not that deep into the SWT-drawing mechanisms, therefore I can't estimate, whether this introduces other side effects, The flickering issues didn't appear.

When looking through the repository, the UpdateManager.performUpdate() is mainly used during animations and a few other things like smooth scrolling.

@Phillipus
Copy link
Contributor

which puts the Display.readAndDispatch into the DeferredUpdateManager.performUpdate

I thought about that but as the problem is related to animation I put it in the Animation class.

@daell82
Copy link
Author

daell82 commented Feb 19, 2024

I guess, that's a little bit complex 😄

It got my attention when when I noticed that the animations on Windows were not drawn after #127 and when checking it on Linux, they didn't appear even with the revert. Therefore my guess was, that this wasn't supported even before. With the fix only in the Animation class, it does not cover all features (e.g. the animated zoom from GMF and the graph layout from the flow example, probably even more) as they do their own logic. Since #127 is a fix for non-Windows platforms dealing with a Windows-issue, I wouldn't revert it back.

@Phillipus
Copy link
Contributor

Therefore my guess was, that this wasn't supported even before.

Animations used to work on Linux and Mac up to about 2017 or 2018. Our app, Archi, has been around since 2010 so we used to have animations on all platforms.

@Phillipus
Copy link
Contributor

Since #127 is a fix for non-Windows platforms dealing with a Windows-issue, I wouldn't revert it back.

Actually, it only affects Windows so an OS check could be applied there. However, it would still break animation on Windows.

@daell82
Copy link
Author

daell82 commented Feb 19, 2024

Ok, that is strange.

I just checked the master branch on a Linux system. There are no animations drawn, neither in the palette, nor in the flow-example graph layout.

@Phillipus
Copy link
Contributor

Ok, that is strange.

I just checked the master branch on a Linux system. There are no animations drawn, neither in the palette, nor in the flow-example graph layout.

Do you mean with the Display.readAndDispatch patch? It's working for me on Linux X11. Maybe you use Wayland?

@daell82
Copy link
Author

daell82 commented Feb 19, 2024

With the current master branch, without any modifications, there are no animations. With Display.readAndDispatch in the Animation class, the palette gets animated. Instead when putting it into the DeferredUpdateManager everything seems ok.

I'm testing with OpenSuse in Hyper-V, it says X11.

@Phillipus
Copy link
Contributor

To be clear:

Animations used to work on Linux and Mac up to about 2017 or 2018.

@daell82
Copy link
Author

daell82 commented Feb 19, 2024

Could that be related to this issue:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=481200 ?

@Phillipus
Copy link
Contributor

Could that be related to this issue: https://bugs.eclipse.org/bugs/show_bug.cgi?id=481200 ?

That's my bug report.

There's no way of determining what caused animations to stop working on Mac and Linux. I think probably that changes to the OS were the cause, not SWT.

@daell82
Copy link
Author

daell82 commented Feb 19, 2024

Oh, I didn't see that 🙈

If I could, I would test it on OSX. But I can delegate it tomorrow if appreciated.

@Phillipus
Copy link
Contributor

To be clear, Animations do work on Linux and Mac with the Display.readAndDispatch workaround. As I said earlier, it's what we use in our RCP app.

@ptziegler
Copy link
Contributor

From what I can tell, the issue is caused by the following contract of the UpdateManager no longer being satisfied:

/**
 * Forces an update to occur. Update managers will perform updates
 * automatically, but may do so asynchronously. Calling this method forces a
 * synchronous update.
 */
public abstract void performUpdate();

This issue only shows up when the FigureCanvas is created using the SWT.DOUBLE_BUFFERED flag because then, the NativeGraphicsSource is used internally.

However, because the call to update() has been removed with #127, this update is now done asychronously. For example, the redraw() method (which is still executed) is using RedrawWindow on Windows.

And according to the documentation:

The following flags control when repainting occurs. RedrawWindow will not repaint unless one of these flags is specified:

  • RDW_ERASENOW
  • RDW_UPDATENOW

Though reverting #127 is not the solution as it only works on Windows, and because it paints too much (as shown by the artifacts of the originating bug).

I tried out a few things and this is what I learned:

  1. The "original" behavior by using canvas.update().
WithUpdate.webm
  1. Instead of returning null, the getGraphics() method returns a proper Graphics object.
WithGraphics.webm
  1. Instead of calling canvas.update(), I now call Display.readAndDispatch() directly after the call to redraw().
WithReadAndDispatch.webm

Both solutions 2) and 3) work, but 2) causes a lot of flickering. I'm not overly excited with 3), as there is no guarantee that the event that is executed is exactly the redraw event. But given that it worked in Archi for years, I think that' the best bet.

ptziegler added a commit to ptziegler/gef-classic that referenced this issue May 17, 2024
When the FigureCanvas is created with SWT.DOUBLE_BUFFERED, animations
are not painted correctly. This is because an instance of
NativeGraphicsSource is used internally, which doesn't paint
synchronously.

Because the animation is done inside the UI thread, this paint operation
is only processed after the animation is done.

Resolves eclipse#376
ptziegler added a commit to ptziegler/gef-classic that referenced this issue May 19, 2024
When the FigureCanvas is created with SWT.DOUBLE_BUFFERED, animations
are not painted correctly. This is because an instance of
NativeGraphicsSource is used internally, which doesn't paint
synchronously.

Because the animation is done inside the UI thread, this paint operation
is only processed after the animation is done.

Resolves eclipse#376
@daell82
Copy link
Author

daell82 commented May 26, 2024

Thank you for also deep diving into this.

I can verify that the third solution seems to fix all issues I observed.

ptziegler added a commit to ptziegler/gef-classic that referenced this issue May 27, 2024
When the FigureCanvas is created with SWT.DOUBLE_BUFFERED, animations
are not painted correctly. This is because an instance of
NativeGraphicsSource is used internally, which doesn't paint
synchronously.

Because the animation is done inside the UI thread, this paint operation
is only processed after the animation is done.

Resolves eclipse#376
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants