From 582f5fa6eb7dc00333bffd4c4db254fda20b059f Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Fri, 7 Aug 2020 11:33:26 +0200 Subject: [PATCH 1/9] dont use replace transaction anymore. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That way we dont “unload” and “load” fragments. This fixes black screens and slow transitions with opengl or cameras # Conflicts: # packages/core/ui/frame/fragment.transitions.android.ts # packages/core/ui/frame/frame-common.ts # packages/core/ui/frame/index.android.ts --- .../ui/frame/fragment.transitions.android.ts | 3 +- packages/core/ui/frame/frame-common.ts | 4 --- packages/core/ui/frame/index.android.ts | 30 +++++++++++++++++-- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/packages/core/ui/frame/fragment.transitions.android.ts b/packages/core/ui/frame/fragment.transitions.android.ts index 83fe2dd1a..cea73620d 100644 --- a/packages/core/ui/frame/fragment.transitions.android.ts +++ b/packages/core/ui/frame/fragment.transitions.android.ts @@ -143,13 +143,12 @@ export function _setAndroidFragmentTransitions(animated: boolean, navigationTran if (currentFragmentNeedsDifferentAnimation) { setupCurrentFragmentFadeTransition(navigationTransition, currentEntry); } - } else if (name === 'explode') { + } else if (name === 'explode') { setupNewFragmentExplodeTransition(navigationTransition, newEntry); if (currentFragmentNeedsDifferentAnimation) { setupCurrentFragmentExplodeTransition(navigationTransition, currentEntry); } } else if (name.indexOf('flip') === 0) { - navigationTransition = { duration: 3000, curve: null }; const direction = name.substr('flip'.length) || 'right'; //Extract the direction from the string const flipTransition = new FlipTransition(direction, navigationTransition.duration, navigationTransition.curve); diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index e89038db2..f8b98e87e 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -296,10 +296,6 @@ export class FrameBase extends CustomLayoutView { private raiseCurrentPageNavigatedEvents(isBack: boolean) { const page = this.currentPage; if (page) { - if (page.isLoaded) { - // Forward navigation does not remove page from frame so we raise unloaded manually. - page.callUnloaded(); - } page.onNavigatedFrom(isBack); } } diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 91cd68408..40d1af199 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -324,6 +324,8 @@ export class Frame extends FrameBase { // If we had real navigation process queue. this._processNavigationQueue(entry.resolvedPage); + + } else { // Otherwise currentPage was recreated so this wasn't real navigation. // Continue with next item in the queue. @@ -436,7 +438,7 @@ export class Frame extends FrameBase { //transaction.setTransition(androidx.fragment.app.FragmentTransaction.TRANSIT_FRAGMENT_OPEN); } - transaction.replace(this.containerViewId, newFragment, newFragmentTag); + transaction.add(this.containerViewId, newFragment, newFragmentTag); transaction.commitAllowingStateLoss(); } @@ -458,8 +460,30 @@ export class Frame extends FrameBase { _reverseTransitions(backstackEntry, this._currentEntry); - transaction.replace(this.containerViewId, backstackEntry.fragment, backstackEntry.fragmentTag); - + const currentIndex =this.backStack.length; + const gotBackToIndex = this.backStack.indexOf(backstackEntry); + + for (let index = gotBackToIndex + 1; index < currentIndex; index++) { + transaction.remove(this.backStack[index].fragment); + } + if (this._currentEntry !== backstackEntry) { + // if we are going back we need to store where we are backing to + // so that we can set the current entry + if ((this._currentEntry as any).exitTransitionListener) { + (this._currentEntry as any).exitTransitionListener.backEntry = backstackEntry; + } + if ((this._currentEntry as any).returnTransitionListener) { + (this._currentEntry as any).returnTransitionListener.backEntry = backstackEntry; + } + if ((this._currentEntry as any).enterTransitionListener) { + (this._currentEntry as any).enterTransitionListener.backEntry = backstackEntry; + } + if ((this._currentEntry as any).reenterTransitionListener) { + (this._currentEntry as any).reenterTransitionListener.backEntry = backstackEntry; + } + transaction.remove((this._currentEntry).fragment); + + } transaction.commitAllowingStateLoss(); } From b127f12bbbf7d8f6f97b5d06f621377d3205a509 Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Thu, 27 Aug 2020 09:25:01 +0200 Subject: [PATCH 2/9] another try at fixing transitions --- .../ui/frame/fragment.transitions.android.ts | 18 +++++++++---- packages/core/ui/frame/index.android.ts | 26 ++++++++----------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/packages/core/ui/frame/fragment.transitions.android.ts b/packages/core/ui/frame/fragment.transitions.android.ts index cea73620d..8840733b9 100644 --- a/packages/core/ui/frame/fragment.transitions.android.ts +++ b/packages/core/ui/frame/fragment.transitions.android.ts @@ -26,6 +26,7 @@ let AnimationListener: android.animation.Animator.AnimatorListener; interface ExpandedTransitionListener extends androidx.transition.Transition.TransitionListener { entry: ExpandedEntry; + backEntry?: BackstackEntry; transition: androidx.transition.Transition; } @@ -223,6 +224,7 @@ function getAnimationListener(): android.animation.Animator.AnimatorListener { onAnimationStart(animator: ExpandedAnimator): void { const entry = animator.entry; + const backEntry = animator.backEntry; addToWaitingQueue(entry); if (Trace.isEnabled()) { Trace.write(`START ${animator.transitionType} for ${entry.fragmentTag}`, Trace.categories.Transition); @@ -236,10 +238,13 @@ function getAnimationListener(): android.animation.Animator.AnimatorListener { } onAnimationEnd(animator: ExpandedAnimator): void { + const entry = animator.entry; + const backEntry = animator.backEntry; if (Trace.isEnabled()) { - Trace.write(`END ${animator.transitionType} for ${animator.entry.fragmentTag}`, Trace.categories.Transition); + Trace.write(`END ${animator.transitionType} for ${entry.fragmentTag} backEntry:${backEntry ? backEntry.fragmentTag : 'none'}`, Trace.categories.Transition); } - transitionOrAnimationCompleted(animator.entry, animator.backEntry); + transitionOrAnimationCompleted(entry, backEntry); + animator.backEntry = null; } onAnimationCancel(animator: ExpandedAnimator): void { @@ -345,10 +350,12 @@ function getTransitionListener(entry: ExpandedEntry, transition: androidx.transi onTransitionEnd(transition: androidx.transition.Transition): void { const entry = this.entry; + const backEntry = this.backEntry; if (Trace.isEnabled()) { - Trace.write(`END ${toShortString(transition)} transition for ${entry.fragmentTag}`, Trace.categories.Transition); + Trace.write(`END ${toShortString(transition)} transition for ${entry.fragmentTag} backEntry:${backEntry ? backEntry.fragmentTag : 'none'}`, Trace.categories.Transition); } - transitionOrAnimationCompleted(entry, this.backEntry); + transitionOrAnimationCompleted(entry, backEntry); + this.backEntry = null; } onTransitionResume(transition: androidx.transition.Transition): void { @@ -497,7 +504,7 @@ function setEnterTransition(navigationTransition: NavigationTransition, entry: E fragment.setEnterTransition(transition); } -function setExitTransition(navigationTransition: NavigationTransition, entry: ExpandedEntry, transition: androidx.transition.Transition): void { +function xsetExitTransition(navigationTransition: NavigationTransition, entry: ExpandedEntry, transition: androidx.transition.Transition): void { setUpNativeTransition(navigationTransition, transition); const listener = addNativeTransitionListener(entry, transition); @@ -660,6 +667,7 @@ function transitionOrAnimationCompleted(entry: ExpandedEntry, backEntry: Backsta if (!entries) { return; } + console.log('transitionOrAnimationCompleted', frameId, backEntry && backEntry.fragmentTag, waitingQueue.size, entries.size, completedEntries.size ); entries.delete(entry); if (entries.size === 0) { diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 40d1af199..ef82d6743 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -19,6 +19,7 @@ import { Builder } from '../builder'; import { CSSUtils } from '../../css/system-classes'; import { Device } from '../../platform'; import { profile } from '../../profiling'; +import { ExpandedEntry } from './fragment.transitions.android'; export * from './frame-common'; @@ -463,26 +464,21 @@ export class Frame extends FrameBase { const currentIndex =this.backStack.length; const gotBackToIndex = this.backStack.indexOf(backstackEntry); - for (let index = gotBackToIndex + 1; index < currentIndex; index++) { - transaction.remove(this.backStack[index].fragment); - } + // the order is important so that the transition listener called be + // the one from the current entry we are going back from if (this._currentEntry !== backstackEntry) { + const entry = this._currentEntry as ExpandedEntry; // if we are going back we need to store where we are backing to // so that we can set the current entry - if ((this._currentEntry as any).exitTransitionListener) { - (this._currentEntry as any).exitTransitionListener.backEntry = backstackEntry; + // it only needs to be done on the return transition + if (entry.returnTransitionListener) { + entry.returnTransitionListener.backEntry = backstackEntry; } - if ((this._currentEntry as any).returnTransitionListener) { - (this._currentEntry as any).returnTransitionListener.backEntry = backstackEntry; - } - if ((this._currentEntry as any).enterTransitionListener) { - (this._currentEntry as any).enterTransitionListener.backEntry = backstackEntry; - } - if ((this._currentEntry as any).reenterTransitionListener) { - (this._currentEntry as any).reenterTransitionListener.backEntry = backstackEntry; - } - transaction.remove((this._currentEntry).fragment); + transaction.remove((this._currentEntry).fragment); + } + for (let index = gotBackToIndex + 1; index < currentIndex; index++) { + transaction.remove(this.backStack[index].fragment); } transaction.commitAllowingStateLoss(); } From c1e710a614470333b5ec0244cab514579136b388 Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Mon, 24 Aug 2020 11:07:29 +0200 Subject: [PATCH 3/9] use replace when navigating with clearHistory --- packages/core/ui/frame/index.android.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index ef82d6743..6143a2416 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -439,7 +439,11 @@ export class Frame extends FrameBase { //transaction.setTransition(androidx.fragment.app.FragmentTransaction.TRANSIT_FRAGMENT_OPEN); } - transaction.add(this.containerViewId, newFragment, newFragmentTag); + if (clearHistory) { + transaction.replace(this.containerViewId, newFragment, newFragmentTag); + } else { + transaction.add(this.containerViewId, newFragment, newFragmentTag); + } transaction.commitAllowingStateLoss(); } From 392e2c8d59551c4183538234d8c63a3d747e72e1 Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Mon, 24 Aug 2020 11:14:36 +0200 Subject: [PATCH 4/9] still unload page if navigation is replace or is back or clearHistory is used --- packages/core/ui/frame/frame-common.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index f8b98e87e..6f4a5c526 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -259,7 +259,7 @@ export class FrameBase extends CustomLayoutView { public _updateBackstack(entry: BackstackEntry, navigationType: NavigationType): void { const isBack = navigationType === NavigationType.back; const isReplace = navigationType === NavigationType.replace; - this.raiseCurrentPageNavigatedEvents(isBack); + this.raiseCurrentPageNavigatedEvents(isBack, isReplace, entry.entry.clearHistory); const current = this._currentEntry; // Do nothing for Hot Module Replacement @@ -293,9 +293,14 @@ export class FrameBase extends CustomLayoutView { return false; } - private raiseCurrentPageNavigatedEvents(isBack: boolean) { + private raiseCurrentPageNavigatedEvents(isBack: boolean, isReplace: boolean, clearHistory: boolean) { const page = this.currentPage; if (page) { + if ((isBack || isReplace || clearHistory) && page.isLoaded) { + console.log('unloading current page'); + // Forward navigation does not remove page from frame so we raise unloaded manually. + page.callUnloaded(); + } page.onNavigatedFrom(isBack); } } From 9a5d99e241a0453a347d76fcf1aab00eb591f2ad Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Mon, 24 Aug 2020 11:14:50 +0200 Subject: [PATCH 5/9] also use replace when replacing frame page --- packages/core/ui/frame/index.android.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 6143a2416..0c876dc06 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -439,7 +439,7 @@ export class Frame extends FrameBase { //transaction.setTransition(androidx.fragment.app.FragmentTransaction.TRANSIT_FRAGMENT_OPEN); } - if (clearHistory) { + if (clearHistory || isReplace) { transaction.replace(this.containerViewId, newFragment, newFragmentTag); } else { transaction.add(this.containerViewId, newFragment, newFragmentTag); From d41195e3f97ffc1e71a2625c9dc1a53993efd2f7 Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Mon, 24 Aug 2020 11:25:02 +0200 Subject: [PATCH 6/9] in fact this does not seem to be necessary anymore --- packages/core/ui/frame/frame-common.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index 6f4a5c526..f8b98e87e 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -259,7 +259,7 @@ export class FrameBase extends CustomLayoutView { public _updateBackstack(entry: BackstackEntry, navigationType: NavigationType): void { const isBack = navigationType === NavigationType.back; const isReplace = navigationType === NavigationType.replace; - this.raiseCurrentPageNavigatedEvents(isBack, isReplace, entry.entry.clearHistory); + this.raiseCurrentPageNavigatedEvents(isBack); const current = this._currentEntry; // Do nothing for Hot Module Replacement @@ -293,14 +293,9 @@ export class FrameBase extends CustomLayoutView { return false; } - private raiseCurrentPageNavigatedEvents(isBack: boolean, isReplace: boolean, clearHistory: boolean) { + private raiseCurrentPageNavigatedEvents(isBack: boolean) { const page = this.currentPage; if (page) { - if ((isBack || isReplace || clearHistory) && page.isLoaded) { - console.log('unloading current page'); - // Forward navigation does not remove page from frame so we raise unloaded manually. - page.callUnloaded(); - } page.onNavigatedFrom(isBack); } } From be15327dccd37d87715a2b52b06d15eff1dcdd42 Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Mon, 24 Aug 2020 17:02:42 +0200 Subject: [PATCH 7/9] typo fix --- packages/core/ui/frame/fragment.transitions.android.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/ui/frame/fragment.transitions.android.ts b/packages/core/ui/frame/fragment.transitions.android.ts index 8840733b9..76cf44924 100644 --- a/packages/core/ui/frame/fragment.transitions.android.ts +++ b/packages/core/ui/frame/fragment.transitions.android.ts @@ -504,7 +504,7 @@ function setEnterTransition(navigationTransition: NavigationTransition, entry: E fragment.setEnterTransition(transition); } -function xsetExitTransition(navigationTransition: NavigationTransition, entry: ExpandedEntry, transition: androidx.transition.Transition): void { +function setExitTransition(navigationTransition: NavigationTransition, entry: ExpandedEntry, transition: androidx.transition.Transition): void { setUpNativeTransition(navigationTransition, transition); const listener = addNativeTransitionListener(entry, transition); From 5fcfb753509c9ca72c1a805afba3eb42e8f2b56b Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Thu, 27 Aug 2020 09:07:25 +0200 Subject: [PATCH 8/9] android fragment transitions fully working now --- packages/core/ui/frame/frame-common.ts | 2 +- packages/core/ui/frame/index.android.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/core/ui/frame/frame-common.ts b/packages/core/ui/frame/frame-common.ts index f8b98e87e..f52530aa3 100644 --- a/packages/core/ui/frame/frame-common.ts +++ b/packages/core/ui/frame/frame-common.ts @@ -259,8 +259,8 @@ export class FrameBase extends CustomLayoutView { public _updateBackstack(entry: BackstackEntry, navigationType: NavigationType): void { const isBack = navigationType === NavigationType.back; const isReplace = navigationType === NavigationType.replace; - this.raiseCurrentPageNavigatedEvents(isBack); const current = this._currentEntry; + this.raiseCurrentPageNavigatedEvents(isBack); // Do nothing for Hot Module Replacement if (isBack) { diff --git a/packages/core/ui/frame/index.android.ts b/packages/core/ui/frame/index.android.ts index 0c876dc06..6cc4af7ff 100644 --- a/packages/core/ui/frame/index.android.ts +++ b/packages/core/ui/frame/index.android.ts @@ -444,6 +444,9 @@ export class Frame extends FrameBase { } else { transaction.add(this.containerViewId, newFragment, newFragmentTag); } + if (this._currentEntry && this._currentEntry.entry.backstackVisible === false) { + transaction.remove(this._currentEntry.fragment); + } transaction.commitAllowingStateLoss(); } @@ -466,7 +469,7 @@ export class Frame extends FrameBase { _reverseTransitions(backstackEntry, this._currentEntry); const currentIndex =this.backStack.length; - const gotBackToIndex = this.backStack.indexOf(backstackEntry); + const goBackToIndex = this.backStack.indexOf(backstackEntry); // the order is important so that the transition listener called be // the one from the current entry we are going back from @@ -481,9 +484,10 @@ export class Frame extends FrameBase { transaction.remove((this._currentEntry).fragment); } - for (let index = gotBackToIndex + 1; index < currentIndex; index++) { + for (let index = goBackToIndex + 1; index < currentIndex; index++) { transaction.remove(this.backStack[index].fragment); } + transaction.commitAllowingStateLoss(); } From 53c8234b23457b144785acd9b47df0dfc4b8395a Mon Sep 17 00:00:00 2001 From: Martin Guillon Date: Thu, 27 Aug 2020 09:08:05 +0200 Subject: [PATCH 9/9] test fix for new navigation event order --- apps/automated/app/ui/page/page-tests-common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/automated/app/ui/page/page-tests-common.ts b/apps/automated/app/ui/page/page-tests-common.ts index 74ef94543..08ee90412 100644 --- a/apps/automated/app/ui/page/page-tests-common.ts +++ b/apps/automated/app/ui/page/page-tests-common.ts @@ -181,7 +181,7 @@ function _test_PageNavigation_EventSequence(withTransition: boolean) { helper.navigateWithEntry(navigationEntry); helper.goBack(); - const expectedEventSequence = ['navigatingTo', 'loaded', 'navigatedTo', 'navigatingFrom', 'unloaded', 'navigatedFrom']; + const expectedEventSequence = ['navigatingTo', 'loaded', 'navigatedTo', 'navigatingFrom', 'navigatedFrom', 'unloaded']; TKUnit.arrayAssert(eventSequence, expectedEventSequence, 'Actual event sequence is not equal to expected. Actual: ' + eventSequence + '; Expected: ' + expectedEventSequence); }