perf(android): avoid redundant traversal and transform allocations - #682
OskarEichler wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The changes are localized performance refactors that preserve ordering/semantics, and the updated transform logic is still correctly scoped by the caller’s save/restoreToCount.
Pull request overview
Improves Android capture performance in ViewShot by reducing redundant allocations and work while preserving existing leaf traversal order and transform application semantics.
Changes:
- Refactors
getAllChildrento accumulate leaf views into a single output list (avoids per-subtree list allocations). - Optimizes ancestor transform application by iterating the ancestor list in reverse index order (avoids list reversing/copying and removes unused matrix construction and per-ancestor canvas saves).
File summaries
| File | Description |
|---|---|
| android/src/main/java/fr/greweb/reactnativeviewshot/ViewShot.java | Reduces allocations and unnecessary Canvas/Matrix work during leaf traversal and overlay transform application. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@OskarEichler thanks for the PR. I merged #676 earlier that conflicts with this, can you look at this, maybe your work also addressed that earlier work in a more generic way |
|
Merged current upstream master and resolved the conflict while preserving the pivot-aware Canvas.scale fix from #676. This PR does not replace that rendering fix: it removes the unused Matrix calculation, redundant ancestor-list copying/reversal, and per-node save calls (both call sites already save/restore around the full transformation). Leaf traversal order remains unchanged. Reviewed the resulting diff and both save/restore call sites; git diff --check passes. I have not run an Android device rendering test locally. |
Fixes
Collect leaves into one output list, retain one ancestor array list, iterate it in reverse order without copying, and remove unused matrix construction and per-ancestor canvas saves. The caller already saves/restores the complete overlay transform.
Compatibility / observable changes
Leaf ordering and parent-to-child translate/rotate/scale operations are unchanged, including root and detached-view cases. This does not implement the separate clipping/pivot corrections in open PRs #656 and #676.
Verification
Compiled actual Java methods verify unchanged leaf and transform ordering with one list allocation and no unused matrix or canvas saves. Existing ancestor and child collection tests pass.
This patch was applied independently to upstream commit
6acbec50a5e3cab7d668711d757c52cfdc791c76and passed its targeted external actual-source diagnostics. Across the focused patches, 119 such checks pass. Java/Objective-C diagnostics use controlled bridge/platform doubles or owned filesystem fixtures; they are not substitutes for physical-device rendering tests.Combined branch checks:
The separate iOS cleanup patch intentionally makes the existing test named
testReleaseCapture_currentlyDeletesPrefixOnlyImposterDirectories_KNOWN_LOOSE_GUARDfail because it asserts the unsafe old behavior. That patch is kept in a separate draft PR. No checked-in test/spec or snapshot files were added, modified, regenerated or disabled.Windows/Expo native builds, physical-device video/PixelCopy output, and full Detox suites were not run. The full unchanged Playwright suite was run against both baseline and patched builds: both have 9 passes and the same 3 failures. Two Linux-reference screenshot mismatches have byte-identical baseline/patched actual images on macOS. The CORS fixture hard-codes port 3000, occupied by an unrelated local backend; the isolated example uses another port. No snapshots or assertions were changed. React Doctor also reports existing example suggestions and a React-18 ref-cleanup warning; the latter was checked against actual React 18 legacy-ref and React 19 cleanup behavior. No rules were suppressed.
Scope
android/src/main/java/fr/greweb/reactnativeviewshot/ViewShot.javaUnrelated audit changes are submitted separately.