Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refact(iOS, Fabric): take snapshot in unmountChildComponent:index: #2261

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Jul 22, 2024

Description

Note

This PR applies to iOS only.

Ok, so this PR is related to #2247 & to get broader context I highly recommend to read this comment 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

#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] [2] [5]
  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]
  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] [3]
  4. We know that the RN's mounting stage will be executed on main thread, 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" and is performed atomically & synchronously on main thread, thus whole batch of updates will be finished before drawing instructions will be send to render server.

Reference:

[1] (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] (Blog post on rendering in UIKit)

[3] (Apple docs - The View Drawing Cycle section)

[4] (Blog post on the run loop)

[5] (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

  • Ensured that CI passes

@kkafar kkafar changed the title refact(iOS, Fabric): take snapshot in unmountChildComponent:index: refact(iOS, Fabric): take snapshot in unmountChildComponent:index: Jul 23, 2024
@kkafar kkafar requested review from tboba, WoLewicki and alduzy July 23, 2024 13:26
Copy link
Member

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kkafar kkafar merged commit 137c9e2 into main Jul 23, 2024
5 checks passed
@kkafar kkafar deleted the @kkafar/refactor-snapshots-on-ios branch July 23, 2024 15:36
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
…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
WoLewicki added a commit that referenced this pull request Nov 14, 2024
## Description

Based on
Expensify/App#49937 (comment) and
the comments above, PR fixes the unnecessary checks for creating
snapshot on the view on dismiss. It was refactored already in
#2134 and
#2261 (👏 to
@kkafar). Those checks do nothing now since each screen is responsible
for making its own snapshot. Having those checks can only lead to
problems.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants