Skip to content

Commit

Permalink
refact(iOS, Fabric): take snapshot in unmountChildComponent:index: (s…
Browse files Browse the repository at this point in the history
…oftware-mansion#2261)

## Description

> [!note] 
This PR applies to iOS only. 

Ok, so this PR is related to software-mansion#2247 & to get broader context I highly
recommend to read [this
comment](software-mansion#2247 (review))
at the very minimum.

### Issue context

On Fabric during JS initialised Screen dismissal (view removing in
general) children are unmounted before their parents, thus when
dismissing screen from screen stack & starting a dismiss transition all
Screen content is already removed & we're animating only a blank screen
resulting in visual glitch.

### Current approaches

Right now we're utilising `RCTMountingTransactionObserving` protocol,
filter all mounting operations *before* they are applied and if screen
dismissal is to be done, we take a snapshot of to-be-removed-screen.

### Alternative approaches

software-mansion#2134 sets mounting coordinator delegate and effectively does the same
as the current approach, however it can also be applied to Android.

### Proposed approach

On iOS we can utilise the platform & how it works. Namely the fact of
unmounting child view does not impact the hardware buffer, nor bitmap
layer immediately, thus we can take the snapshot simply in `- [RNSScreen
unmountChildComponentView: index:]` and the children will still be
visible.

This approach is safe and reliable, because:

##### 10k feet explanation

Drawing is not performed immediately after an update to UIKit model
(such as removing a view), the system handles all operations pending on
main queue and just after that it schedules drawing. We're removing the
views & making snapshot in the middle of block execution on the main
thread, thus the drawing can't happen and just-unmounted-views will be
visible on the snapshot.

##### More detailed explanation

1. the main thread run loop of Cocoa application drains the main queue
till it's empty
[[1]](https://opensource.apple.com/source/CF/CF-1153.18/CFRunLoop.c.auto.html)
[[2]](https://fabernovel.github.io/2021-01-04/uikit-rendering-part-3)
[[5]](https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/RunLoopManagement/RunLoopManagement.html#//apple_ref/doc/uid/10000057i-CH16-SW1)
2. CoreAnimation framework integrates with the main run loop by
registering an observer and listening for `kCFRunLoopBeforeWaiting`
event (so after the main queue is drained & run loop is to become idle
due to no more pending tasks).
[[2]](https://fabernovel.github.io/2021-01-04/uikit-rendering-part-3)
3. CoreAnimation is responsible for applying all transactions from the
last loop pass & sending them to render server (this happens on main
thread), which in turn finally leads up to the changes being applied,
drawn & displayed (this happens on different threads).
[[2]](https://fabernovel.github.io/2021-01-04/uikit-rendering-part-3)
[[3]](https://developer.apple.com/library/archive/documentation/WindowsViews/Conceptual/ViewPG_iPhoneOS/WindowsandViews/WindowsandViews.html#//apple_ref/doc/uid/TP40009503-CH2-SW1)
4. [We know that the RN's mounting stage will be executed on main
thread](https://github.com/facebook/react-native/blob/91ecd7eb5330f5d725f5587744713064d614a6b3/packages/react-native/React/Fabric/Mounting/RCTMountingManager.mm#L258),
because UIKit is thread-safe only in selected parts and requires calling
from the main thread.
5. Single RN transaction is a complete diff between ["rendered tree" &
"next
tree"](https://reactnative.dev/architecture/render-pipeline#phase-2-commit-1)
and is performed [atomically &
synchronously](https://github.com/facebook/react-native/blob/91ecd7eb5330f5d725f5587744713064d614a6b3/packages/react-native/ReactCommon/react/renderer/mounting/TelemetryController.cpp#L18-L51)
on [main
thread](https://github.com/facebook/react-native/blob/91ecd7eb5330f5d725f5587744713064d614a6b3/packages/react-native/React/Fabric/Mounting/RCTMountingManager.mm#L258),
thus whole batch of updates will be finished before drawing instructions
will be send to render server.

#### Reference:


[[1]](https://opensource.apple.com/source/CF/CF-1153.18/CFRunLoop.c.auto.html)
(Look for `__CFRunLoopDoBlocks(...)` & `__CFRunLoopRun(...)` functions)

Important thing to notice if `__CFRunLoopDoBlocks` is that it locks the
`rl` (run loop) lock, takes & copies reference to the list of the blocks
to execute, clears the original list of blocks and releases the `rl`
lock. Thus only the "already scheduled" blocks are executed in the
single pass of this function. It is called multiple times in the single
pass of the run loop, but I haven't dug deeper, it should be enough for
our use case that we have guarantee that all the blocks are drained.

[[2]](https://fabernovel.github.io/2021-01-04/uikit-rendering-part-3)
(Blog post on rendering in UIKit)


[[3]](https://developer.apple.com/library/archive/documentation/WindowsViews/Conceptual/ViewPG_iPhoneOS/WindowsandViews/WindowsandViews.html#//apple_ref/doc/uid/TP40009503-CH2-SW1)
(Apple docs - The View Drawing Cycle section)

[[4]](https://bou.io/RunRunLoopRun.html) (Blog post on the run loop)


[[5]](https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/RunLoopManagement/RunLoopManagement.html#//apple_ref/doc/uid/10000057i-CH16-SW1)
(Apple docs on run loops)

## Changes

* Snapshot is not done in `unmountChildComponentView: index:` & only
when needed.
* Removed old mechanism
* Removed now unused implementation of `RCTMountingObserving` protocol

## Test code and steps to reproduce

Run any example on Fabric, push a screen, initiate go-back via JS (e.g.
by clicking a button with `navigation.goBack()` action), see that the
screen transitions correctly (the content is visible throughout
transition)

## Checklist

- [x] Ensured that CI passes
  • Loading branch information
kkafar authored and ja1ns committed Oct 9, 2024
1 parent e3183c6 commit dfeacf1
Showing 1 changed file with 3 additions and 18 deletions.
21 changes: 3 additions & 18 deletions ios/RNSScreenStack.mm
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,7 @@ @interface RNSScreenStackView () <
UINavigationControllerDelegate,
UIAdaptivePresentationControllerDelegate,
UIGestureRecognizerDelegate,
UIViewControllerTransitioningDelegate
#ifdef RCT_NEW_ARCH_ENABLED
,
RCTMountingTransactionObserving
#endif
>
UIViewControllerTransitioningDelegate>

@property (nonatomic) NSMutableArray<UIViewController *> *presentedModals;
@property (nonatomic) BOOL updatingModals;
Expand Down Expand Up @@ -1125,13 +1120,15 @@ - (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childCompone
- (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
{
RNSScreenView *screenChildComponent = (RNSScreenView *)childComponentView;

// We should only do a snapshot of a screen that is on the top.
// We also check `_presentedModals` since if you push 2 modals, second one is not a "child" of _controller.
// Also, when dissmised with a gesture, the screen already is not under the window, so we don't need to apply
// snapshot.
if (screenChildComponent.window != nil &&
((screenChildComponent == _controller.visibleViewController.view && _presentedModals.count < 2) ||
screenChildComponent == [_presentedModals.lastObject view])) {
[self takeSnapshot];
[screenChildComponent.controller setViewToSnapshot:_snapshot];
}

Expand Down Expand Up @@ -1166,18 +1163,6 @@ - (void)takeSnapshot
}
}

- (void)mountingTransactionWillMount:(react::MountingTransaction const &)transaction
withSurfaceTelemetry:(react::SurfaceTelemetry const &)surfaceTelemetry
{
for (auto &mutation : transaction.getMutations()) {
if (mutation.type == react::ShadowViewMutation::Type::Remove && mutation.parentShadowView.componentName != nil &&
strcmp(mutation.parentShadowView.componentName, "RNSScreenStack") == 0) {
[self takeSnapshot];
return;
}
}
}

- (void)prepareForRecycle
{
[super prepareForRecycle];
Expand Down

0 comments on commit dfeacf1

Please sign in to comment.