Animations are not drawn

[daell82]

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]

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]

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

https://github.com/eclipse/gef-classic/assets/139544371/3eb7f1d5-13d1-4df6-b6c9-32a4f9505603

[ptziegler]

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]

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]

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]

~I don't think the changes introduced in https://github.com/eclipse/gef-classic/pull/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]

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]

You could try https://github.com/eclipse/gef-classic/pull/377 to see if that works.

[daell82]

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]

As of now, I see these options:

1. Revert #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 #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]

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]

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]

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]

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]

Since https://github.com/eclipse/gef-classic/pull/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]

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]

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]

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]

To be clear:

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

[daell82]

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

[Phillipus]

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]

Oh, I didn't see that 🙈

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

[Phillipus]

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]

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

https://github.com/eclipse/gef-classic/assets/70652978/55604019-1142-4f7c-89c1-20d3c87a407c

2. Instead of returning null, the getGraphics() method returns a proper Graphics object.

https://github.com/eclipse/gef-classic/assets/70652978/93328682-add6-4d28-ade4-e6eeb6e741d4

3. Instead of calling canvas.update(), I now call Display.readAndDispatch() directly after the call to redraw().

https://github.com/eclipse/gef-classic/assets/70652978/0e34599c-c147-4ea4-9043-ca676d971140

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.

[daell82]

Thank you for also deep diving into this.

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

[ptziegler]

This one has been going on for several months now... @azoitl Should we include one of the fixes before the M1 and finally get this sorted out?

[azoitl]

@ptziegler yes we should. I'm still not perfectly sure which one. But I have a slight tendency to your solution as it is the more general solution. Am I right?

[ptziegler]

It's the only one that doesn't cause test failures 😅

[Phillipus]

But as the problem is only to do with animation changing NativeGraphicsSource seems a bit too much perhaps?

[azoitl]

Oh I was to fast and already merged. Should I revert, or should we give it a try? Early in the development cycle I tend to keep it.