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

animations.mp4

[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 #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 #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 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]

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

WithUpdate.webm

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

WithGraphics.webm

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

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

[ptziegler]

I still believe that the issue stems from repaint operation triggered by performUpdate() being executed asynchronously, going directly against the documentation.

This issue is very noticeable with the animations, but it would generally happen whenever paint operations are done consecutively, as only the "last" repaint is processed.

Regarding the other proposals: I'm still not sure what's causing the Argument not valid exceptions in #377, but my current guess is that, given that the call readAndDispatch() might break the order in which certain UI tasks have to be executed.

The fact that we both had to guard against this case in the Animation class is an indicator for this.

[ptziegler]

Urgh... the test failures seem to be a result of this. Because the animations are drawn again, adding e.g. an edit part takes longer. The result is that the animation isn't finished by the time SWTBot checks the new state of the editor.

We probably should've done a rebase before merging. Given that M1 is due today, I would suggest rolling the commit back until this is sorted out.

[ptziegler]

Reopened because ee7f7ee was reverted.

[azoitl]

Then I'll start M1 now and we see how this can be fixed later.

[daell82]

Hello again,
Thank you again for your effort. I guess the assumption that the solution with readAndDispatch() breaks some events is correct. I've noticed this in my application on the PaletteScrollBar where the scroll-buttons use the ButtonModel#REPEAT_FIRING_BEHAVIOR which causes the Palette to become nearly unusable. In that case the Timer for firing events even remains active after the editor is closed. I've created recordings on Windows and Linux to visualize this with the FlowExample (by overloading PaletteDrawer):

linux-readAndDispatch.mp4

On Linux, there is rarely a dancing palette, but performance while animating afterwards decreases

windows-readAndDispatch.mp4

On Windows it's obvious

I assume that this issue is becoming more complex, since not all parts use the Animation class for animating their contents (e.g. the GraphAnimation from the flow example). I'm circumventing this currently by copying the PaletteScrollBar and using the ButtonModel#DEFAULT_FIRING_BEHAVIOR , since it's the only situation I've noticed this.

[ptziegler]

Thank you again for your effort. I guess the assumption that the solution with readAndDispatch() breaks some events is correct. I've noticed this in my application on the PaletteScrollBar where the scroll-buttons use the ButtonModel#REPEAT_FIRING_BEHAVIOR which causes the Palette to become nearly unusable.

To avoid any misunderstandings, which of the three proposed fixes are you using? Given that they are applied to different locations, each one may introduce their own set of unwanted side-effects.

[daell82]

I am using the third variant with canvas.getDisplay().readAndDispatch() in the NativeGraphicsSource. I have no other modifications in the repository (except overloading the PaletteDrawer from the FlowExample for the recordings).

[github-actions]

This issue is stale because it has been open for 90 days with no activity.