Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 172 additions & 0 deletions android/src/main/java/com/swmansion/rnscreens/Screen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,19 @@ package com.swmansion.rnscreens

import android.annotation.SuppressLint
import android.content.pm.ActivityInfo
import android.graphics.Bitmap
import android.graphics.Paint
import android.graphics.Rect
import android.graphics.drawable.BitmapDrawable
import android.os.Build
import android.os.Handler
import android.os.HandlerThread
import android.os.Looper
import android.os.Parcelable
import android.util.Log
import android.util.SparseArray
import android.view.MotionEvent
import android.view.PixelCopy
import android.view.View
import android.view.ViewGroup
import android.view.WindowManager
Expand Down Expand Up @@ -34,6 +43,8 @@ import com.swmansion.rnscreens.events.SheetDetentChangedEvent
import com.swmansion.rnscreens.ext.asScreenStackFragment
import com.swmansion.rnscreens.ext.parentAsViewGroup
import com.swmansion.rnscreens.gamma.common.FragmentProviding
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit

@SuppressLint("ViewConstructor") // Only we construct this view, it is never inflated.
class Screen(
Expand Down Expand Up @@ -459,12 +470,155 @@ class Screen(

fun startRemovalTransition() {
if (!isBeingRemoved) {
captureScreenSnapshotForRemoval()
isBeingRemoved = true
startTransitionRecursive(this)
}
}

// --- Screen-level removal snapshot -------------------------------------------------------

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implementation opinion anchor — This screen-level opt-in is the right boundary for the removal snapshot. I would preserve this design while fixing the edge-swipe lifecycle seams described in the review body.

// The exit animation renders this screen while surface-backed content (e.g. a mid-fling
// WebView) may stop producing pixels, which flashes the window background through. When a
// screen opts in via the `snapshotOnRemoval` prop, freeze its last presented frame into its
// ViewOverlay for the duration of the removal transition so the pop animates real pixels.
// Opt-in only: the capture reads the composited window, which is only correct for opaque,
// full-window screens (e.g. the Reader) — never enable it for translucent/partial screens.

/** Set from JS via the `snapshotOnRemoval` prop; default off. */
var snapshotOnRemoval: Boolean = false

private var removalSnapshotDrawable: BitmapDrawable? = null

// One capture attempt per removal: a snapshot is only valid if taken before the removal
// transaction starts animating. If the pre-transaction attempt fails (timeout etc.), a later

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Convention audit: minor divergence — The timeout wording no longer matches the current async fallback.

The 17ms wait expiring is not a failed attempt: the same PixelCopy remains live and may install from its posted continuation. Please distinguish actual failure from asynchronous completion, e.g. “If the attempt fails or completes asynchronously, later entry points must not issue another capture.”

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f5a1e8a — the one-attempt comment now distinguishes outright failure from asynchronous completion, per your wording.

// entry point must NOT retry — it could capture an already-translated, mid-exit window.
//
// Attempt state is owned by the MAIN thread and claimed only at actual capture time, so the

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Convention audit: minor divergence — Make the one-attempt comment match when the attempt is consumed.

The code sets removalSnapshotAttempted before attachment, size, activity, and window prerequisites, while this comment says it is claimed only at actual capture time. Document that entering the main-thread routine consumes the attempt and a prerequisite failure is final, or move the assignment after prerequisites if retry is intended.

// synchronous pre-transaction call in ScreenStack.onUpdate() always wins over queued off-main
// requests: a queued request that runs after it no-ops on the claimed attempt.
private var removalSnapshotAttempted: Boolean = false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Architectural seam: this single boolean currently conflates a queued request, an actual capture attempt, and membership in a particular removal lifecycle. I would replace it with main-thread-owned attempt state plus a removal generation/token; that makes the pre-transaction guarantee explicit rather than timing-dependent.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 16b8ea4 per the blocking finding: scheduled/attempted/lifecycle are now distinct — attempt state is main-thread-owned and claimed at capture time, and queued requests carry a removal-generation token. The pre-transaction guarantee is structural rather than timing-dependent.


// Incremented (on main) whenever the snapshot lifecycle resets. An off-main request carries
// the generation it was posted under and no-ops if its removal has already been cleaned up,
// so a delayed post can never install an overlay into a later lifecycle.
@Volatile
private var removalSnapshotGeneration: Int = 0

fun captureScreenSnapshotForRemoval() {
if (!snapshotOnRemoval) return
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) return
if (Looper.myLooper() == Looper.getMainLooper()) {
captureScreenSnapshotOnMain()
} else {
val requestGeneration = removalSnapshotGeneration
post {
if (requestGeneration == removalSnapshotGeneration) {
captureScreenSnapshotOnMain()
}
}
}
}

private fun captureScreenSnapshotOnMain() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Convention audit: gap — this adds a new lifecycle/thread-race state machine without an automated regression case.

The timeout/callback ownership handoff, cancellation, detach, repeated removal, API <26 behavior, and Paper/Fabric ordering are the load-bearing cases. Add a focused Android test seam around the capture result/timeout and at least an app regression case that exercises removal and cancellation; the existing stack test fixture is at apps/src/tests/TestScreenStack/index.tsx.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged as a real gap. Two constraints shaped what I did instead: the fork's Android tree has no unit-test source sets or test dependencies, and the load-bearing behavior (PixelCopy timing vs presented frames) is only observable on a real compositor. The lifecycle cases named here — timeout, cancellation, detach, repeated removal, both entry-point orderings — are covered by on-device suites that ran against each commit (interrupted-transition ×6, rapid double-back, 15-cycle removal memory soak, and a per-frame dual-probe video scan of mid-fling pops that caught a real regression in the async variant and drove 9ff816c). I'd take the TestScreenStack fixture + Robolectric seam as a follow-up PR rather than coupling test infrastructure to this change.

// Recheck the opt-in: a posted capture can outlive a prop flip on a recycled screen.
if (!snapshotOnRemoval) return
if (removalSnapshotAttempted) return
removalSnapshotAttempted = true
if (!isAttachedToWindow || width <= 0 || height <= 0) return
val activity = reactContext.currentActivity ?: return
val window = activity.window
val decor = window.decorView

val location = IntArray(2)
getLocationInWindow(location)
val srcRect = Rect(location[0], location[1], location[0] + width, location[1] + height)
// Only capture a fully on-window screen; a clipped rect would be stretched over the
// screen's full bounds when displayed.
if (!srcRect.intersect(0, 0, decor.width, decor.height)) return
if (srcRect.width() != width || srcRect.height() != height) return

val bitmap =
try {
Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888)
} catch (e: OutOfMemoryError) {
Log.w(TAG, "removal snapshot allocation failed", e)
return
}

// The copy is requested before the removal transaction is created, so it reads the
// pre-transition presented frame. The main thread waits at most ONE frame period for it
// (measured completion is 6-16ms, so the typical install is synchronous — which matters:
// the overlay must be up before the next frame can present the app's own teardown blank,
// or one black frame slips through). If the copy is slower, the main thread moves on and
// the completion posts the install instead: worst case is one frame of jank or one late

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Convention audit: minor divergence — “Never a dropped snapshot” overstates the fallback guarantee.

installOrDropRemovalSnapshot intentionally drops/recycles on copy failure, generation change, or detach. A precise claim is that the code never abandons an in-flight successful snapshot solely because the synchronous wait elapsed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f5a1e8a — the claim is now precise: an in-flight successful copy is never abandoned merely because the synchronous wait elapsed; the continuation still drops on copy failure, generation change, or detach.

// frame, never a long stall and never a dropped snapshot.
//
// The install/drop decision runs exactly once, always on the main thread (`handled` is
// only touched there): the synchronous path claims it when the copy beats the bounded
// wait; otherwise the posted continuation claims it. Install only if this removal is
// still live (generation) and the screen attached; otherwise the bitmap is dropped.
val requestGeneration = removalSnapshotGeneration
val handled = booleanArrayOf(false)
val latch = CountDownLatch(1)
val pixelCopyResult = intArrayOf(PixelCopy.ERROR_UNKNOWN)
try {
PixelCopy.request(window, srcRect, bitmap, { copyResult ->
pixelCopyResult[0] = copyResult
latch.countDown()
mainHandler.post {
if (!handled[0]) {
handled[0] = true
installOrDropRemovalSnapshot(copyResult, bitmap, requestGeneration)
}
}
}, snapshotHandler)
} catch (e: IllegalArgumentException) {
bitmap.recycle()
return
}

val completed =
try {
latch.await(SNAPSHOT_SYNC_WAIT_MS, TimeUnit.MILLISECONDS)
} catch (e: InterruptedException) {
Thread.currentThread().interrupt()
false
}
if (completed && !handled[0]) {
handled[0] = true
installOrDropRemovalSnapshot(pixelCopyResult[0], bitmap, requestGeneration)
}
}

private fun installOrDropRemovalSnapshot(
copyResult: Int,
bitmap: Bitmap,
requestGeneration: Int,
) {
if (copyResult == PixelCopy.SUCCESS &&
requestGeneration == removalSnapshotGeneration &&
isAttachedToWindow
) {
val drawable = BitmapDrawable(resources, bitmap)
drawable.setBounds(0, 0, width, height)
removalSnapshotDrawable = drawable
overlay.add(drawable)
invalidate()
} else {
bitmap.recycle()
}
}

fun releaseRemovalSnapshot() {
removalSnapshotGeneration++
removalSnapshotDrawable?.let { overlay.remove(it) }
removalSnapshotDrawable = null
removalSnapshotAttempted = false
}
// --- end screen-level removal snapshot ----------------------------------------------------

fun endRemovalTransition() {
releaseRemovalSnapshot()
if (!isBeingRemoved) {
return
}
Expand Down Expand Up @@ -563,6 +717,11 @@ class Screen(
}
}

override fun onDetachedFromWindow() {
releaseRemovalSnapshot()
super.onDetachedFromWindow()
}

private fun dispatchSheetDetentChanged(
detentIndex: Int,
isStable: Boolean,
Expand Down Expand Up @@ -650,6 +809,19 @@ class Screen(
* This value describes value in sheet detents array that will be treated as `fitToContents` option.
*/
const val SHEET_FIT_TO_CONTENTS = -1.0

// One 60Hz frame period: the most the main thread will wait for a PixelCopy before
// handing the install to the async continuation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Convention audit: minor divergence — 17ms is not one frame period on all supported displays.

It is approximately one 60Hz frame and exceeds one frame at 90/120Hz. Calling this a “17ms bounded wait” keeps the comment accurate without implying a device-independent frame budget.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in f5a1e8a — described as a bounded wait of ~one frame at 60Hz (more than one at 90/120Hz), not a device-independent frame period.

private const val SNAPSHOT_SYNC_WAIT_MS = 17L

// Shared thread PixelCopy callbacks land on; one idle thread for the process lifetime.
private val snapshotHandler: Handler by lazy {
Handler(HandlerThread("RNSScreenSnapshot").also { it.start() }.looper)
}

// Main-thread continuation for snapshot installs. A plain handler (not View.post) so the
// decision always runs even if the view detaches while the copy is in flight.
private val mainHandler: Handler by lazy { Handler(Looper.getMainLooper()) }
}
}

Expand Down
7 changes: 7 additions & 0 deletions android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,13 @@ class ScreenStack(
}
}

if (!shouldUseOpenAnimation && topScreenWillChange) {
// Close animation: the current top screen is outgoing. Freeze its last presented
// frame before the removal transaction starts animating it (covers dismissal paths
// where Fabric's removeViewAt — and thus startRemovalTransition — runs later).
topScreenWrapper?.screen?.captureScreenSnapshotForRemoval()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the right architectural seam: capture the explicitly opted-in outgoing top screen immediately before constructing the close transaction. The important invariant is that this remains the only point at which a new snapshot attempt can succeed; a later fallback attempt after failure can capture the transition itself.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The invariant is now enforced in code (5879b16): a per-removal attempted flag means no entry point can start a second capture attempt after the first — whether it succeeded or failed — until the removal's cleanup resets it.

}

createTransaction().let { transaction ->
if (stackAnimation != null) {
transaction.setTweenAnimations(stackAnimation, shouldUseOpenAnimation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,14 @@ open class ScreenViewManager :
view.isGestureEnabled = gestureEnabled
}

@ReactProp(name = "snapshotOnRemoval", defaultBoolean = false)
override fun setSnapshotOnRemoval(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[HIGH] The new prop was not added to the checked-in Paper codegen. RNSScreenManagerInterface has no setSnapshotOnRemoval, so this override should fail Paper compilation, and RNSScreenManagerDelegate has no case that can dispatch the prop. yarn architectures-consistency-check currently fails on this exact mismatch.

Run yarn sync-architectures, commit the regenerated Paper interface and delegate, and verify the Paper Android build.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 5879b16 — ran yarn sync-architectures and committed the regenerated RNSScreenManagerInterface/RNSScreenManagerDelegate (the diff is exactly the setSnapshotOnRemoval method + delegate case). node scripts/codegen-check-consistency.js now exits 0.

view: Screen,
snapshotOnRemoval: Boolean,
) {
view.snapshotOnRemoval = snapshotOnRemoval
}

@ReactProp(name = "replaceAnimation")
override fun setReplaceAnimation(
view: Screen,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ class ScreenAnimationDelegate(
}
}

override fun onAnimationCancel(animation: Animator) = Unit
override fun onAnimationCancel(animation: Animator) {
// A cancelled exit can leave the screen attached and live; drop the frozen removal
// snapshot so the user never returns to a stale frame covering real content.
wrapper.screen.releaseRemovalSnapshot()
}

override fun onAnimationRepeat(animation: Animator) = Unit

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ public void setProperty(T view, String propName, @Nullable Object value) {
case "gestureEnabled":
mViewManager.setGestureEnabled(view, value == null ? true : (boolean) value);
break;
case "snapshotOnRemoval":
mViewManager.setSnapshotOnRemoval(view, value == null ? false : (boolean) value);
break;
case "statusBarColor":
mViewManager.setStatusBarColor(view, ColorPropConverter.getColor(value, view.getContext()));
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public interface RNSScreenManagerInterface<T extends View> {
void setHomeIndicatorHidden(T view, boolean value);
void setPreventNativeDismiss(T view, boolean value);
void setGestureEnabled(T view, boolean value);
void setSnapshotOnRemoval(T view, boolean value);
void setStatusBarColor(T view, @Nullable Integer value);
void setStatusBarHidden(T view, boolean value);
void setScreenOrientation(T view, @Nullable String value);
Expand Down
7 changes: 7 additions & 0 deletions lib/typescript/types.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions src/fabric/ScreenNativeComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export interface NativeProps extends ViewProps {
homeIndicatorHidden?: boolean;
preventNativeDismiss?: boolean;
gestureEnabled?: WithDefault<boolean, true>;
snapshotOnRemoval?: WithDefault<boolean, false>;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Convention audit: no precedent — This is the repository’s first screen-removal PixelCopy/ViewOverlay mechanism. The positive opt-in/default-false prop matches existing native-prop conventions and appropriately prevents normal stack screens from paying its cost. The generated Paper surfaces still need to be synchronized, as noted in the blocking review.

@artemlitch artemlitch Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

[MEDIUM] Regenerate the shipped Fabric declaration for this new native prop

snapshotOnRemoval is added to NativeProps here, but the exact head still ships lib/typescript/fabric/ScreenNativeComponent.d.ts without this member (and its source map is likewise unchanged). Direct consumers of the packaged Fabric native component therefore receive a stale TypeScript contract even though the runtime and public ScreenProps accept the prop. This repo normally commits the matching generated declaration/map alongside Fabric spec changes, so please run the normal build/codegen workflow and commit lib/typescript/fabric/ScreenNativeComponent.d.ts plus .d.ts.map.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in f5a1e8a — ran the normal bob build; the regeneration touched exactly the declarations/maps downstream of the spec change (lib/typescript/fabric/ScreenNativeComponent.d.ts + .map, lib/typescript/types.d.ts + .map, and the module/commonjs source maps), all committed.

statusBarColor?: ColorValue;
statusBarHidden?: boolean;
screenOrientation?: string;
Expand Down
9 changes: 9 additions & 0 deletions src/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,15 @@ export interface ScreenProps extends ViewProps {
* @platform ios
*/
gestureEnabled?: boolean;
/**
* When `true`, the screen freezes its last presented frame into a native overlay at the start
* of its removal transition, so the exit animation shows real pixels even if surface-backed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Convention audit: minor divergence — This public contract currently reads as an unconditional, start-of-transition freeze, but the implementation explicitly allows the 17 ms wait to elapse and installs from an async continuation; PixelCopy failure also drops the bitmap. Please describe this as a best-effort capture requested at removal start (and mirror the wording in the generated declaration via the normal build), so consumers do not rely on a guarantee the native path cannot make.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d085779 — the prop doc (src/types.tsx and the bob-regenerated shipped declarations) now describes a best-effort capture requested at removal start: asynchronous completion shortly after the transition begins is possible, and the capture is dropped on PixelCopy failure or when the view has left the window.

* content (e.g. a WebView) stops rendering mid-transition. Only enable for opaque, full-window
* screens — the capture reads the composited window. Defaults to `false`.
*
* @platform android
*/
snapshotOnRemoval?: boolean;
/**
* Use it to restrict the distance from the edges of screen in which the gesture should be recognized. To be used alongside `fullScreenSwipeEnabled`.
*
Expand Down