Refactor TransitioningContentControl

[grokys]

What does the pull request do?

As described in #11167, TransitioningContentControl had two problems:

1. Transitions were flickering
2. The content wasn't actually transitioned between the old and new content: the old content transitioned out and then the new content transitioned in

Turns out that although there remains a problem which caused 1, fixing 2 actually fixes 1 in this case.

This PR fixes 2 by making TransitioningContentControl host two ContentPresenters and transitioning between them.

Breaking changes

Any custom templates for TransitioningContentControl will need to be updated to host two ContentPresenters. If that's not done, the control will still work, but no transitions will take place.

Fixed Issues

Fixes #11167

[Mrxx99]

I tested this PR and it still flickers (GIF may take some time to load):

Also the new behavior of starting both animations at the same time is different as to what is shown in the docs: https://docs.avaloniaui.net/docs/controls/transitioningcontentcontrol
I think both have their use cases, maybe it should be configurable with a property like e.g TransitionConcurrently="True"

[llfab]

I tested this PR and it still flickers (GIF may take some time to load):

Also the new behavior of starting both animations at the same time is different as to what is shown in the docs: https://docs.avaloniaui.net/docs/controls/transitioningcontentcontrol I think both have their use cases, maybe it should be configurable with a property like e.g TransitionConcurrently="True"

I tend to agree. You could argue about what a TCC should do. I had been pretty happy with the old function also because you were able to also transition from/to null-content (skipping the null-content transition) by creating your own CrossFade (https://github.com/llfab/Samples/tree/main/CrossFade). It would be great to have both (transition-out then transition-in OR transition both at the same time) but for keeping the semantic compatibility I would give the old behavior priority.

[grokys]

I tested this PR and it still flickers

Strange, I'm definitely not seeing that here; I'm guessing it's a timing issue. Sounds like the problem with page transitions still needs to be solved correctly but for that I need to wait for @kekekeks incoming changes.

Also the new behavior of starting both animations at the same time is different as to what is shown in the docs

Yeah that's because the docs still show the old TransitioningContentControl which was broken.

I think both have their use cases, maybe it should be configurable with a property like e.g TransitionConcurrently="True"

If such a thing is needed IMO that setting should be put in the transition itself.

[llfab]

I tested this PR and it still flickers

Strange, I'm definitely not seeing that here; I'm guessing it's a timing issue. Sounds like the problem with page transitions still needs to be solved correctly but for that I need to wait for @kekekeks incoming changes.

Also the new behavior of starting both animations at the same time is different as to what is shown in the docs

Yeah that's because the docs still show the old TransitioningContentControl which was broken.

I think both have their use cases, maybe it should be configurable with a property like e.g TransitionConcurrently="True"

If such a thing is needed IMO that setting should be put in the transition itself.

That would be a way, yes. However, the old CrossFade had both transitions concurrently using await Task.WhenAll(tasks); but the Start() function of CrossFade is called twice from the TCC. Once with to == null and once with from == null. Hence, it may not be so easy to just do it via the transition (like CrossFade) because it strongly depends on how the transition(s) is/are called from the TCC. Right?

[grokys]

Hence, it may not be so easy to just do it via the transition (like CrossFade) because it strongly depends on how the transition(s) is/are called from the TCC. Right?

Sorry, I don't understand the question. The intention of a page transition was always to transition from one page to another, the page transition itself can decide to do that in any way it likes. It can transition one page out and then the other one in, it can decide not to transition on some condition. I don't think it "strongly depends on how the transitions are called" because the transition should always called in one way: "from this control, to this control".

The original implementation of TransitioningContentControl was just broken IMO and wouldn't have made it past review if I'd seen it in time because it precisely doesn't give you this control.

[grokys]

Actually, regarding handling null: one change that we might want to make in this PR is passing null as to instead of an empty content presenter when the content is null.

[llfab]

Hence, it may not be so easy to just do it via the transition (like CrossFade) because it strongly depends on how the transition(s) is/are called from the TCC. Right?

Sorry, I don't understand the question. The intention of a page transition was always to transition from one page to another, the page transition itself can decide to do that in any way it likes. It can transition one page out and then the other one in, it can decide not to transition on some condition. I don't think it "strongly depends on how the transitions are called" because the transition should always called in one way: "from this control, to this control".

The original implementation of TransitioningContentControl was just broken IMO and wouldn't have made it past review if I'd seen it in time because it precisely doesn't give you this control.

That sounds good. If it's possible to give the control to the transition, that would be the best solution. I just had the impression that the "old" way had been there for a reason, having been surprised myself when I saw that Start() was always called twice. Now I understand that it should have been different in the first place. Then I appreciate very much that the flickering actually appeared so this became a focus area. Thanks for having such a detailed look at it.

[llfab]

Actually, regarding handling null: one change that we might want to make in this PR is passing null as to instead of an empty content presenter when the content is null.

Nice. One problem I had when creating above mentioned custom CrossFade was, that I needed to use a dummy animation (1 msec) for the null content otherwise the final state was wrong. So yes, addressing from/to null would be much appreciated. Also it makes sense because that seems not like just an edge case.

[llfab]

I also tested using control catalog:

• CrossFade seem ok. No flickering
• Slide (horizontally and vertically): when starting the transition the next image seems to appear for 1 frame in the background which you only notice if the next image is larger than the previous (and only if you increase the window size of control catalog)
• Custom transition seems to have the exact same problem
• Once switched once to Custom transition and later back to Cross Fade or Slide, the fade out is not shown ever again. The from image disappears completely and the new image is transitioned in. This can only be fixed by restarting the app.

Hope that helps.

Windows 10, Nvidia Quadro T2000

[grokys]

one change that we might want to make in this PR is passing null as to instead of an empty content presenter when the content is null

Actually this isn't going to be possible without some nasty hacks as the "current" content presenter will exist and have null as its content by the time the transition starts, so we do need to transition to this control. This case can be detected though by checking if the to control is a ContentPresenter will a null Content.

[llfab]

one change that we might want to make in this PR is passing null as to instead of an empty content presenter when the content is null

Actually this isn't going to be possible without some nasty hacks as the "current" content presenter will exist and have null as its content by the time the transition starts, so we do need to transition to this control.

Not a problem so much, as long as one can find out in a custom transition (similar to my custom CrossFade) that there is null-content. For myself the workaround using a 1 msec dummy animation then is good enough.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0034612-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

[grokys]

the workaround using a 1 msec dummy animation then is good enough.

That workaround shouldn't be necessary, I'll make sure the TransitioningContentControl handles a no-op transition.

[grokys]

Slide (horizontally and vertically): when starting the transition the next image seems to appear for 1 frame in the background which you only notice if the next image is larger than the previous (and only if you increase the window size of control catalog)
Custom transition seems to have the exact same problem

Yeah I think this is a problem with the renderer and animations being out of sync. @kekekeks is working on a fix for this.

Once switched once to Custom transition and later back to Cross Fade or Slide, the fade out is not shown ever again. The from image disappears completely and the new image is transitioned in. This can only be fixed by restarting the app.

This was caused by the custom transition setting FillMode.Forward which means that after the transition had finished, the from control had a render transform with a Y scaling of 0. Fixed.

[avaloniaui-team]

You can test this PR using the following package version. 11.0.999-cibuild0034653-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

[maxkatz6]

todo: test if reactiveui navigation still works

[grokys]

todo: test if reactiveui navigation still works

@maxkatz6 there's a test app in ReactiveUIDemo - I've tested this PR there and it appears to work fine. Are there other scenarios to test?

[maxkatz6]

@grokys no, ReactiveUIDemo should be enough. Thanks.

[laolarou726]

@maxkatz6 @grokys
It seems the new TCC will cause an exception when switching between 2 pages is too quick or the animation speed is too slow.

[billhenn]

Yeah I think this is a problem with the renderer and animations being out of sync. @kekekeks is working on a fix for this.

I just tested out a PR build made last night for another issue and tried TransitioningContentControl to see if the flickering was resolved. It seemed better than a couple weeks ago, but I did see the same thing as @llfab where a horizontal slide transition shows the "to" content briefly for about a frame prior to the transition starting.

@kekekeks - Are you still working on a fix for that?

[grokys]

@laolarou726 not quite sure what you mean, can we get a repro? If not, at least a full stacktrace might help.