-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Initial version of double masking #327
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
Conversation
* andrey/nonstandard-views: Attached dialog sample space use Canvas for certain types of windows consolidate in one view sample of adding floating views add floating button test: Add UI and logic to evaluate boolean flags (#305)
…ed-mask2 * andrey/do-not-send-duplicate-windows: Unit tests Volatile address feedback Fix feedback Recycle bitmap early fix size bug no message
* andrey/transformed-coordinates: address feedback cursor review
* main: doc: Add using ldMask in readme. (#311) chore: release main (#312) feat: take transformed coordinates, which are more precise in animation (#309) chore: release main (#307) fix(SEC-7530): update react-server-dom-webpack to 19.0.1 (#310) # Conflicts: # sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/capture/CaptureSource.kt # sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/masking/ComposeMaskTarget.kt # sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/masking/Mask.kt # sdk/@launchdarkly/observability-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/masking/NativeMaskTarget.kt
* main: feat: Limit accumulating canvas buffer (#322) chore: Fix existing RRwebGraphQLReplayLogExporterTest tests (#321) feat: sanitize URLs + semantic conventions for header attributes (#317) refactor: introduce granular ObservabilityOptions (#323) refactor: OY11-846 - Add Session Replay plugin (#313) chore: upgrade react-server-dom-webpack to 19.0.3 (#320) chore: release main (#319) feat: enhance Web Vitals telemetry with semantic attributes (#316) chore: release main (#318) fix: Android - Remove Disk Buffering (#315) chore: readme update with real examples (#314)
...y-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/capture/CaptureSource.kt
Outdated
Show resolved
Hide resolved
...ity-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/masking/MaskApplier.kt
Show resolved
Hide resolved
…bservability-sdk into andrey/fix-masking-offset * 'andrey/fix-masking-offset' of github.com:launchdarkly/observability-sdk: feat: Graphql client memory optimization (#325)
| maskIntRect.right = mask.rect.right.toInt() | ||
| maskIntRect.bottom = mask.rect.bottom.toInt() | ||
| canvas.drawRect(maskIntRect, paint) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Axis-aligned rectangle masks may use wrong coordinates
When mask.points forms an axis-aligned rectangle (no rotation), the code falls through to use mask.rect coordinates instead of constructing a rectangle from the points values. However, these coordinate systems can differ: mask.rect contains window-relative coordinates from getLocationInWindow(), while mask.points contains root-relative coordinates (screen position minus locationOnScreen(root)). These only match if the root view is at window position (0,0). For views in dialogs, popups, or non-standard window configurations where the root view has an offset within the window, the mask will be drawn at the wrong position. The old code always used points when available, avoiding this coordinate mismatch.
* main: docs: Update readme with options refactor (#326)
* main: feat: Gzip compression for Graphql request body (#328)
|
|
||
| if (afterMasksMap.count() == 0) { | ||
| return null | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Empty mask maps cause all captures to fail
The filteredBeforeMasksMap function returns null when afterMasksMap.count() == 0, even if both maps are empty (no masked elements). In CaptureSource.kt, this null return causes the capture to be discarded via the ?: return@withContext null pattern. This means screens without any masked elements will never produce a capture event, which is likely unintended behavior.
Additional Locations (1)
...y-android/lib/src/main/kotlin/com/launchdarkly/observability/replay/capture/CaptureSource.kt
Outdated
Show resolved
Hide resolved
| pts[i + 1] -= context.rootY | ||
| } | ||
|
|
||
| return pts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Compose masks use different coordinate system than Native masks
The removal of the rootX/rootY adjustment from ComposeMaskTarget.points() creates an inconsistency with NativeMaskTarget.points(), which still subtracts context.rootX and context.rootY from the coordinates. Now Compose masks return screen coordinates while Native masks return window-relative coordinates. When both types of masks are drawn on the same canvas, the Compose masks will be offset incorrectly.
| } | ||
|
|
||
| captured++ | ||
| captureResults[i] = captureResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Missing continue causes incorrect capture counting logic
When captureResult is null and i != 0, the code sets beforeMasks[i] = null but then falls through to increment captured++ and assign captureResults[i] = captureResult. A continue statement is missing after setting beforeMasks[i] = null. This causes captured to count all loop iterations rather than just successful captures, making the if (captured == 0) check at line 129-131 dead code since captured is always incremented.
Summary
Initial version double masking. Doesn't work perfect but masks should be more stable
Are there any deployment considerations?
Note
Implement double-pass masking in the capture pipeline using MaskApplier, collecting masks before/after a frame and drawing only stable masks; update demos and minor cleanups.
MaskApplierto draw masks and filter "before" masks against "after" masks for stability.CaptureSourceto:ComposeMaskTarget: stop subtracting root offsets in polygon points.NativeMaskTarget: reuse matrix with reset; add comment, keep offset correction.ldMasktoZIP CodeandCVV; remove headerldMask; minor layout tweaks.Written by Cursor Bugbot for commit a053d38. This will update automatically on new commits. Configure here.