Control flickers between old and new when assigning a new graph about graphshape HOT 9 CLOSED

Unfortunately, I don't have time right now. I might have time later.

from graphshape.

I managed to get some time to look for it. I found a way to prevent flickering by setting AnimationLength = TimeSpan.Zero, but it removed all animations.

I looked over all the uses of IsAnimationEnabled and CanAnimate and I found that the check for CanAnimate was missing in the RunCreationTransition and RunDestructionTransition methods as it was in RunMoveAnimation. But that wasn't enough, so I try to temporarily remove the animations when the Graph property is changed and it no longer flickers while keeping the animations.

from graphshape.

Hello,

There is a IsAnimationEnabled flag on the GraphLayout control, but it seems to not really prevent flickering.
This would certainly need more in depth investigations. Did you have some time to dig into it or not?

from graphshape.

Glad to know you found a "workaround" but indeed still remain stange that everything related to animation does not really have significant impact on them. I suspect that it was kind of not really used feature before, ot was just not disturbing in those use cases.

When I will have some time, I will try to check in applications at work if I observe the same behavior or not. And if not, then determining the reason why.

from graphshape.

@Shprrr I checked it a bit more and indeed there is an issue with the CanAnimate that was not checked in both RunCreationTransitionand RunDestructionTransition. So we must check it as you added in the PR #7.

Here are some more details on the animations.
The CanAnimate is a readonly property that gather several other information in order to determine if we should run animation or not. So indeed it can be used to control them directly but we have to deal with what is part of the animation conditions.
The reason why in GraphLayout control setting AnimationLength avoid animations to run is because it's part of CanAnimate condition. But the first check done is to verify if the IsAnimationEnabled flag is set to true, which by default is the case. So we set it to false and partially inlcude changes from you PR we get the right behavior that is to say disable animation on demand. The point was only due to missing checks.

Note also that if you don't disable animation with the flag, both run methods will trigger CreationTransition or DestructionTransition which respectively are default setup to a FadeInTransition and a FadeOutTransition. But note that you can change those properties over time to set them to null for example which has for consequence to not run any animation too. Here they are not disabled but not run because not "set".

Here are some possible setup of the control:

<!-- Animations set but fully disabled -->
<GraphLayout
    Graph="{Binding Path=Graph, Mode=OneWay}"
    IsAnimationEnabled="False" />

Or

<!-- Animations enabled but not set -->
<GraphLayout
    Graph="{Binding Path=Graph, Mode=OneWay}"
    CreationTransition="{x:Null}"
    DestructionTransition="{x:Null}" />

Note that both scenarios can be mixed, and will produce the same visual result in the end.

But here is my final point, the change you propose also add a forced animation disabling on graph change, while with part of your change we can conserve full control over animations without forced behavior and fix the behavior you observed. I think we need to keep control. So I propose to update in a way that we don't include this forced behavior, OR at least make it optional. My feeling is that for now we have hardcoded animations that are certainly not the best one but I don't really want to block users that set custom ones because of default ones being faulty.

What do you think of that?

from graphshape.

I tried both examples without the animation disable patch and it wasn't the same result. With IsAnimationEnabled set to false, there was an animation (the same one with the patch) when I would have expected to instantly change to the new graph. With transitions set to null, it was a folding animation instead of the zoom animation with the patch. So it makes me think that if someone sets his own animation, he would have to take in consideration of the animation that is always there no matter what because I would also expect to see an instant change between graphs with transition to null.

I also tried both examples with the disable patch and there was no change. The animation was still going on.

From what I understand from these tests, there are at least two animations that shouldn't happen in creation and destruction. The disable patch keeps one of them but it also removes the good creation and destruction transition. Ideally, we need to have only the creation and destruction animations in this case.

Maybe instead of the disable patch, it should have no default creation and destruction transition until the wrong animation is found, so the default behavior is correct ? Or maybe the disable patch is only there when there is the default transitions ? ex: graphLayout.FreezeAnimations = graphLayout.CreationTransition is FadeInTransition && graphLayout.DestructionTransition is FadeOutTransition; That way when transitions are set to null, the patch isn't working and it makes the same folding animation. But this is again only a temporary fix until we find the remaining wrong animation.

from graphshape.

So I managed to test several scenarios and managed to know why there is a remaining animation. It's because of the MoveAnimation. If we setup the graph control with any of those configurations and with the change I suggest for the PR we end with the same behavior when switching graph which is no animation except the zoom effect when zoom control is in "Fill" mode and no animation at all if static.

Config 1

<GraphLayout
    MoveAnimation="{x:Null}"
    CreationTransition="{x:Null}"
    DestructionTransition="{x:Null}" />

Config 2

<GraphLayout
    IsAnimationEnabled="False" />

Config 3

<GraphLayout
    AnimationLength="0" />

Of course if you want to cut animations then the IsAnimationEnabled flag is the way to go.

So now we should have identified all the animations involved in there. The point is that when you switch graph you have all animtations that are run which explain the flickering. But we now know to to control them.

Here is what I used for testing: 6c1e2c8

from graphshape.

I tried to play around with all three animations a bit more and I saw that when I only had the move animation that it didn't flicker. So I checked the fade transition further to see that by default the animation was playing twice. That's why it was flickering, it was playing the fade in and fade out twice each.

I updated the PR to set to 1 by default and removed the workaround.

from graphshape.

@Shprrr Sorry for the delay I was quite busy :-(
I looked at the last commit and it seems indeed a good catch!

I merged the PR and published a new package with the fix.

from graphshape.