fix(replay): stop a platform view mask spilling past its visible bounds - #553
Conversation
A map, WebView, or camera preview reports its full, unclipped paint bounds, so a view trimmed by a `ClipRect` or a scroll viewport was masked over its whole size — the black box overran the view and covered the Flutter widgets below it, which then vanished from replay. Revealed views composited past the clip the same way on iOS. The mask rect is now intersected with every clip its ancestors apply, and a revealed view is clipped to that same region when it is composited rather than by shrinking the native request, which the platform matches the view by. Trimming the mask to the visible region exposed an older problem it had been hiding: the rect is measured a moment before the screenshot is rasterised, so a tree that moves in between leaves the mask slightly off, and the oversized rect used to absorb that slop. Measured on an Android emulator under continuous scroll, masked content became legible in 27.8% of frames with the trim alone, against 16.7% before it. The mask is therefore re-measured immediately before it is painted and widened to cover both positions, which returns the leak to baseline (18.6% vs 19.3%, p=0.90) without reintroducing the spill. Verified on an Android emulator with `example/lib/platform_view_spill_screen.dart`, which puts a sentinel banner directly below a clipped platform view in seven configurations: 3 of 7 sentinels were fully covered before the fix and 0 of 7 after, with the unclipped control byte-identical across both runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three follow-ups from the pre-PR review, all on paths this change introduced or edits. The capture-failure fallback in _compositeRevealedView painted its mask from the view's full frame, which the revealed path deliberately keeps because the native side matches the view by it. That is the same oversized mask this change exists to remove, still reachable whenever a native capture returns nothing: iOS has no capture path for a non-WKWebView, and Android's software-canvas fallback cannot read a SurfaceView. It now masks the visible region. describeApproximatePaintClip is documented as an approximation for the semantics phase and carries no guarantee of being a superset of the real paint clip; RenderViewportBase subtracts a sliver overlap correction that makes it smaller, so the walk uses the viewport's own bounds there. The walk also calls application code through CustomClipper.getApproximateClipRect, and a throw was propagating out to _addIfNew, which logged and added no rect at all, turning masking off for that view. A throwing clip is now skipped, keeping the wider bounds. Also drops the mask re-measure added earlier in this branch. Rect collection and the mask paint are separated by no await, so it re-read an identical render tree and could never observe motion, while the changeset claimed a protection it did not provide. The frame-late leak it was meant to cover is pre-existing and tracked separately. Verified on an Android emulator with forced capture failure and a throwing clipper: the sentinel below a revealed view went from fully covered to visible, and a masked view under a throwing clipper went from 123,600 exposed pixels to zero. The seven original spill cases re-ran byte-identical on Android and iOS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The walk was substituting a viewport's own bounds for describeApproximatePaintClip, on the grounds that RenderViewportBase subtracts a sliver overlap correction and so can report a clip smaller than the one it actually paints. That reasoning holds against Flutter's source, but a repro built on a pinned translucent SliverAppBar produced frames identical to the unpatched build, so nothing here was ever observed failing. Shipping a privacy change that cannot be demonstrated is worse than leaving the case open. The guard around the same call stays. That one is reproducible: a CustomClipper throwing from application code was letting the exception escape to _addIfNew, which added no rect at all and turned masking off for the view. On an Android emulator that leaked 123,600 pixels of otherwise-masked content, and none with the guard. Re-verified after the removal: 7/7 spill cases still pass on an Android emulator and an iOS simulator with pixel counts unchanged, and the capture failure and throwing clipper cases still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every committed expectation for clippedPaintBounds had origin (0, 0), so an implementation that dropped the visible rect's origin would have passed the whole suite while masking the wrong band of any scrolled platform view. None of them used a scroll view either, even though a viewport reaches describeApproximatePaintClip through a different framework implementation than a ClipRect does. Two cases now cover a view straddling each viewport edge, the second expecting a non-zero origin. The guard for a clipper that throws was verified on a device but pinned by nothing. RenderCustomClip asks getApproximateClipRect rather than getClip, so the obvious form of that test never reaches the walk and passes vacuously; the same is true of a control clipper that leaves getApproximateClipRect at its default, which returns the full box rather than the clip. Both are written so the throwing case and its control disagree. Also records why the composite clip disables antialiasing, that visibleRect is in the view's own coordinates, and that the walk depends on describeApproximatePaintClip over-approximating the real clip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ctive Mapping an ancestor's clip back into the view's coordinates goes through MatrixUtils.transformRect, which maps the four corners and takes their bounding box. That over-approximates for an affine matrix, which is what makes it safe for a mask, but it does not hold once the matrix carries perspective: a corner crossing the w = 0 plane makes the hull arbitrarily smaller than the real region, so the mask shrinks, and where the result misses the paint bounds entirely the view ends up with no mask at all. A ListWheelScrollView reaches this in ordinary use — it gives each child a perspective transform and reports a clip — so a masked platform view inside a CupertinoPicker lost its mask completely. Before this change the masked rect was the full paint bounds and the painter applied the same matrix as the content, so coverage was exact; the clip walk is what introduced the gap. An ancestor transform with a non-zero perspective row now keeps the unclipped bounds, matching how the walk already treats a clip it cannot map. The mask can still be wider than necessary there, never narrower. Also corrects a doc comment that claimed every framework implementation over-approximates the paint clip. RenderViewportBase does not: it subtracts a sliver overlap correction and reports what is semantically visible. Verified on an iPhone 14 Pro Max, iOS 26.6.1: the seven spill cases go from 3 passing on main to 7 with this change, and the unclipped control is identical across both runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A viewport answers describeApproximatePaintClip with what a viewer can see, which subtracts the overlap a pinned or floating sliver header sits over. It paints with its own bounds, so that band is still on screen whenever the header is translucent, and deriving the mask from the reported clip left it uncovered. Measured on a 300x300 viewport with an 80px pinned header, a platform view scrolled to 100 masked from y=100 instead of y=20 — exactly the header band missing, on content that was masked before this branch. The walk now uses the viewport's own bounds. Where a sliver reports no overlap the two are the same rect, so an ordinary ListView or SingleChildScrollView is unaffected: the existing scroll tests assert the same values, and the seven device cases return byte-identical pixel counts on an Android emulator. Also pins the third term of the projective guard. Rotating outside a perspective transform leaves the first two terms zero, and without storage[11] the mask for that subtree would shrink from 400px to 31px tall. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
posthog-flutter Compliance ReportDate: 2026-08-28 16:11:07 UTC ✅ All Tests Passed!45/45 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 16/16 tests passed View Details
|
Drops the comments that restated the line under them or repeated a neighbour, and shortens the rest. What is left states a constraint that is not recoverable from the code: which rect the native side matches on, why the composite clip cannot antialias, why a throwing clipper is skipped, why a viewport's reported clip is not the one it paints with, and why a projective transform cannot go through transformRect. The changeset carried the rationale as well as the change. Release notes are read by someone deciding whether to upgrade, so it is now one line describing what they will see; the rest lives in the pull request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 3 · PR risk: 0/10 |
|
…ports A CustomClipper answers getApproximateClipRect for the semantics layer, and it is allowed to report less than the region getClip actually clips to. The mask walk read that approximation, so a clipper reporting a 10x10 rect over a 100x100 clip left the difference uncovered — the view was on screen there and nothing masked it. The walk now reads getClip for the clip render objects that carry an app clipper, and falls back to the reported rect for everything else. A revealed view was also composited under a clip built from the bounding box of the transformed visible rect. Under a rotation or a skew that box is larger than the region itself, so native pixels could land outside the clip and over unrelated widgets. The canvas now carries the view's transform and clips in the view's own coordinates, which is exact for any affine transform and mirrors how the mask painter already works. Verified on an Android emulator and an iOS simulator: the seven spill cases stay at 7/7 with pixel counts unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…port report its own clip A clip render object with Clip.none paints its child in full, and describes no clip. Reading the clipper directly skipped that check, so a narrow clipper attached with Clip.none shrank the mask over content Flutter draws unclipped. The clipper is now only consulted when the node actually clips. Reverts extending a viewport's clip to its full bounds. The band an overlapping sliver header covers is excluded from what the viewport reports, and masking it paints black over the header itself, which is the common case for a pinned header and a visible regression in the replay. The narrower report is right whenever that header is opaque; a translucent one leaves the band visible and unmasked, which is recorded on clippedPaintBounds as a known limitation. Verified on an Android emulator and an iOS simulator: seven spill cases at 7/7 with pixel counts unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… two misplaced comments An app clipper can return a rect carrying NaN, and a NaN rect is not empty: it passes every emptiness check on the way to the mask, and the mask is then never drawn. In a debug build the draw asserts and the frame is dropped; in release the draw is skipped and the frame ships with the view unmasked. A clip that is not finite is now discarded, which keeps the wider bounds like the other guards. The comment block describing clippedPaintBounds sat above a private helper that was added between it and the function, so dartdoc attached the whole thing — including the pinned header limitation — to the helper, and the function it describes had no documentation at all. It also read as being about the helper to anyone scanning the file. The block now sits on the function, and says that under a translucent header a TextureBox contributes its own pixels rather than just leaving the band unmasked. A test comment still argued for the behaviour reverted in 034f096, directly contradicting the expectation thirty lines below it. Adds coverage for the ClipRRect, ClipPath and non-finite paths, none of which any test constructed, and offsets the clip inside the ancestor so a walk measuring the wrong node's transform no longer passes. Verified on an Android emulator and an iOS simulator: seven spill cases at 7/7 with pixel counts unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… cropped Native crops an axis-aligned region of the screen, so the bytes already hold the view's on-screen appearance. Clipping the composite moved the destination into the view's own space, which applied a rotation or flip a second time and landed a revealed view's pixels turned or mirrored. The clip stays in the view's space, where a rotated edge is exact; the draw goes back to device space. Pixel tests cover the turned, mirrored, clipped and singular cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven cases put a sentinel banner directly below a clipped platform view, so a replay frame missing the banner means the view's rect overran its visible region. Four more show a revealed view's four coloured quadrants under a transform, so a swapped corner means the native crop was composited in the view's own space and the view's rotation or flip was applied twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…gree clip A quarter turn maps an axis-aligned rect onto another axis-aligned rect, so the composite's clip could be swapped for its device-space hull with every test still green. At 45 degrees the hull is twice the area. Nothing covered the four lines that produce the fix either — the masked rect, the dropped fully-clipped view, the revealed view's visible region, and the capture-failure fallback. A seam over the collected rects and one over the fallback close all four; each was checked by mutating the line and watching a test fail. The composite's doc comment claimed native returns an axis-aligned screen crop. That holds for Android's PixelCopy. iOS snapshots a WKWebView in the view's own space, so a rotated or scaled revealed view composites unrotated there — the same on main as here, measured on a simulator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ormats `dart format` runs over the repo root in CI, and the example package resolves a newer language version than the SDK package, so it wraps differently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lockfile was swept into the formatting commit by a `flutter pub get` that `flutter analyze` ran in the example package. The local Flutter pins older SDK-vendored packages than the committed lock, so it read as a downgrade of matcher, meta, test_api and vector_math. Nothing in this PR needs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tokens are written inline in the case list; nothing read the map. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| transform: transform, | ||
| )); | ||
| } | ||
| } catch (e) { |
There was a problem hiding this comment.
Should this catch fail closed? Right now it adds the view to neither masked nor captured, so a throw anywhere in the walk drops the mask for that frame.
That's the opposite direction from the catch inside clippedPaintBounds, which skips a failing clip on purpose so the wider bounds survive.
It bites hardest for a TextureBox under privacy: mask: its pixels are already in the toImage() snapshot by the time this runs, so no mask entry means the live camera frame ships in the replay. Pushing ro.paintBounds into masked here when the policy isn't capture would widen the mask instead of removing it. Is there a reason not to?
There was a problem hiding this comment.
Good catch — you're right, and it's fixed in ab2d30f.
I wanted to be sure of your TextureBox premise before changing the failure direction, since this file asserts it in a comment but nobody had measured it. It holds. I built a throwaway app with a real texture producer on each platform (Android createSurfaceProducer() + a lockCanvas loop, iOS a FlutterTexture vending a CVPixelBuffer), painted a colour that cycles so a live frame is distinguishable from a stale one, wrapped it in a RepaintBoundary and read back toImage():
| texture centre | control | |
|---|---|---|
| Android, Impeller | (255,104,255) then (255,184,255) |
(0,255,0) |
| Android, Skia | (255,104,255) then (255,184,255) |
(0,255,0) |
| iOS 26.4 sim | (255,120,255) then (255,0,255) |
(0,255,0) |
Live texture pixels, not background, and the green channel advanced in step with the producer. The engine reason is that TextureLayer::Paint looks the texture up in the registry and draws it into the canvas (flow/layers/texture_layer.cc), and LayerTree::Flatten passes that registry into the offscreen paint context behind Scene::toImage — whereas PlatformViewLayer has no paint of its own and is an embedder-composited hole. So a masked TextureBox with no mask entry does ship a live camera frame.
On the fix direction: I went with dropping the frame rather than pushing ro.paintBounds into masked. The only thing that realistically throws in that block is ro.getTransformTo(ancestor) (object.dart:3696, "not in the same render tree"), and it's the first statement — so when we land in the catch we have no transform, and a mask rect without one is placed in the wrong coordinate space. Masking the wrong region would leave the real region exposed, which is the same leak with extra steps.
Dropping the frame is also what the widget mask walk two functions up already does when getMaskElements returns null, so the two paths now fail the same way. A revealed view still just skips — it has no mask to lose.
I deliberately did not narrow the drop to TextureBox only. Android TLHC platform views are texture-backed too and surface as PlatformViewRenderBox, and I have not confirmed whether their pixels reach toImage(), so treating every masked platform view the same avoids resting on an open question. The trigger needs the view and the container to be in different render trees, so frames should rarely drop in practice.
Regression test in platform_view_rects_test.dart measures against a detached RenderObject to force the real throw; mask policy returns null (frame dropped), capture policy still returns rects. Checked non-vacuous — reverting the catch to the old always-continue behaviour fails it. 334 tests pass.
…sured The collection walk swallowed a throw and moved on, so the view lost its mask for that frame while the frame still shipped. A texture-backed view's pixels are already in the toImage() snapshot — verified on Android under Impeller and Skia and on iOS, where TextureLayer::Paint draws into the Flutter canvas that PlatformViewLayer only leaves a hole in — so a camera or video frame reached the recording unmasked. The walk now reports failure and the capture drops the frame, matching what the widget mask walk already does. A revealed view still just skips: it has no mask to lose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
💡 Motivation and Context
Session replay masks a platform view — a map, WebView, or camera preview — by painting a black rectangle over it, because the OS draws those, not Flutter.
The rectangle was sized from the view's full bounds, ignoring that a
ClipRector a scroll viewport may only be showing part of it. So the mask ran past the view and covered whatever Flutter widgets sat outside that clip, and those widgets disappeared from replay. Revealed views (PostHogPlatformViewPrivacy.capture) composited past the clip the same way.The mask is now intersected with the ancestor clip chain, and a revealed view is clipped to the same region when it is composited. Masked regions are correspondingly smaller: content a clipped platform view never actually showed on screen is no longer covered.
Also fixed on the same path:
CustomClipperthat throws let the exception escape and the view lost its mask entirely.ListWheelScrollView, so anyCupertinoPicker) could shrink the mask or drop it, because mapping a rect by its four corners only over-approximates for affine matrices.ListView'scacheExtent) got a full-size black rectangle at a position entirely outside the viewport. It is now dropped.Scope
This clips platform-view masks. The widget-mask path (
PostHogMaskWidget,maskAllTexts,maskAllImages) still uses unclipped paint bounds and can still spill across a clip edge — same helper, different call sites, and it changes every mask on every screen, so it wants its own PR and its own device matrix.💚 How did you test it?
example/lib/platform_view_spill_screen.dartships in this PR and is reachable from the example app's menu, so the matrix below can be re-run. It has two families of case:Results
Sentinel cases report the banner's pixel count in the replay frame (0 = buried by the mask). Orientation cases report which quadrant sits in each corner, replay vs screen.
origin/main→ this PRmainmainEvery case the fix targets goes from 0 px to a fully visible banner, and the unclipped control (7) and the unclipped scrolled cases (2, 5) are unchanged to the pixel on both platforms.
The two platforms crop in different spaces
Worth knowing for anyone touching this code again, because it is not visible from the Dart side:
PixelCopycrops the screen. The returned pixels already show the view rotated, so they must go back into the device-space rect.WKWebViewin the view's own space. The returned pixels are always unrotated, so a rotated revealed view composites unrotated — onmainand here alike.An intermediate revision of this branch drew the crop in the view's own space. That cancelled out iOS's view-space snapshot (making iOS look better, 10/11) while double-rotating on Android (8/11 — case 9 replay
MYBRagainst screenYRMB). The orientation cases caught it and it is reverted.iOS cases 9–11 are therefore a pre-existing gap this PR neither touches nor regresses — the same result as
main. Tracking separately.Unit tests
333 pass. The clip walk is covered directly — a scrolled view with a non-zero origin, nested clips, a throwing clipper against a non-throwing control,
Clip.none, non-finite rects, perspective ancestors, a pinned sliver header, and full clip-away. The composite is covered with picture-recorder pixel tests, including a 45° turn (a quarter turn cannot tell the view-space clip apart from its device-space hull) and the capture-failure fallback.Every line that produces the fix was checked by mutating it and confirming a test fails: the masked rect, the fully-clipped drop, the revealed view's visible region, the fallback mask, and the view-space clip.
flutter analyzeanddart formatare clean on both the SDK and the example.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code drove seven review→triage→fix cycles against the shared SDK review criteria, plus the device matrices above.
Decisions worth flagging for a reviewer: