Skip to content

Commit

Permalink
refactor: workaround capture issue (#2657)
Browse files Browse the repository at this point in the history
* refactor: remove ScreenshotStabilizer

* test: fixup native test

* workaround flutter image issue

* fixup

* try to fix ci
  • Loading branch information
vaind authored Feb 5, 2025
1 parent 8f62960 commit c6b7cb8
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 236 deletions.
36 changes: 2 additions & 34 deletions flutter/lib/src/event_processor/screenshot_event_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import '../screenshot/recorder.dart';
import '../screenshot/recorder_config.dart';
import 'package:flutter/widgets.dart' as widget;

import '../screenshot/stabilizer.dart';
import '../utils/debouncer.dart';

class ScreenshotEventProcessor implements EventProcessor {
Expand Down Expand Up @@ -126,37 +125,6 @@ class ScreenshotEventProcessor implements EventProcessor {
}

@internal
Future<Uint8List?> createScreenshot() async {
if (_options.experimental.privacyForScreenshots == null) {
return _recorder.capture((screenshot) =>
screenshot.pngData.then((v) => v.buffer.asUint8List()));
} else {
// If masking is enabled, we need to use [ScreenshotStabilizer].
final completer = Completer<Uint8List?>();
final stabilizer = ScreenshotStabilizer(
_recorder, _options,
(screenshot) async {
final pngData = await screenshot.pngData;
completer.complete(pngData.buffer.asUint8List());
},
// This limits the amount of time to take a stable masked screenshot.
maxTries: 5,
// We need to force the frame the frame or this could hang indefinitely.
frameSchedulingMode: FrameSchedulingMode.forced,
);
try {
unawaited(
stabilizer.capture(Duration.zero).onError(completer.completeError));
// DO NOT return completer.future directly - we need to dispose first.
return await completer.future.timeout(const Duration(seconds: 1),
onTimeout: () {
_options.logger(
SentryLevel.warning, 'Timed out taking a stable screenshot.');
return null;
});
} finally {
stabilizer.dispose();
}
}
}
Future<Uint8List?> createScreenshot() => _recorder.capture(
(screenshot) => screenshot.pngData.then((v) => v.buffer.asUint8List()));
}
28 changes: 10 additions & 18 deletions flutter/lib/src/native/cocoa/cocoa_replay_recorder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,24 @@ import '../../../sentry_flutter.dart';
import '../../replay/replay_recorder.dart';
import '../../screenshot/recorder.dart';
import '../../screenshot/recorder_config.dart';
import '../../screenshot/stabilizer.dart';
import '../native_memory.dart';

@internal
class CocoaReplayRecorder {
final SentryFlutterOptions _options;
final ScreenshotRecorder _recorder;
late final ScreenshotStabilizer<void> _stabilizer;
var _completer = Completer<Map<String, int>?>();

CocoaReplayRecorder(this._options)
: _recorder = ReplayScreenshotRecorder(
ScreenshotRecorderConfig(
pixelRatio:
_options.experimental.replay.quality.resolutionScalingFactor,
),
_options) {
_stabilizer = ScreenshotStabilizer(_recorder, _options, (screenshot) async {
ScreenshotRecorderConfig(
pixelRatio:
_options.experimental.replay.quality.resolutionScalingFactor,
),
_options,
);

Future<Map<String, int>?> captureScreenshot() async {
return _recorder.capture((screenshot) async {
final data = await screenshot.rawRgbaData;
_options.logger(
SentryLevel.debug,
Expand All @@ -35,15 +35,7 @@ class CocoaReplayRecorder {
final json = data.toNativeMemory().toJson();
json['width'] = screenshot.width;
json['height'] = screenshot.height;
_completer.complete(json);
});
}

Future<Map<String, int>?> captureScreenshot() async {
_completer = Completer();
_stabilizer.ensureFrameAndAddCallback((msSinceEpoch) {
_stabilizer.capture(msSinceEpoch).onError(_completer.completeError);
return json;
});
return _completer.future;
}
}
20 changes: 12 additions & 8 deletions flutter/lib/src/replay/scheduled_recorder.dart
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import 'dart:async';

import 'package:flutter/scheduler.dart';
import 'package:meta/meta.dart';

import '../../sentry_flutter.dart';
import '../screenshot/stabilizer.dart';
import '../screenshot/screenshot.dart';
import 'replay_recorder.dart';
import 'scheduled_recorder_config.dart';
Expand All @@ -19,7 +19,6 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder {
late final ScheduledScreenshotRecorderCallback _callback;
var _status = _Status.running;
late final Duration _frameDuration;
late final ScreenshotStabilizer<void> _stabilizer;
// late final _idleFrameFiller = _IdleFrameFiller(_frameDuration, _onScreenshot);

@override
Expand All @@ -35,15 +34,23 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder {
_frameDuration = Duration(milliseconds: 1000 ~/ config.frameRate);
assert(_frameDuration.inMicroseconds > 0);

_stabilizer = ScreenshotStabilizer(this, options, _onImageCaptured);
_scheduler = Scheduler(_frameDuration, _stabilizer.capture,
_stabilizer.ensureFrameAndAddCallback);
_scheduler = Scheduler(
_frameDuration,
(_) => capture(_onImageCaptured),
_addPostFrameCallback,
);

if (callback != null) {
_callback = callback;
}
}

void _addPostFrameCallback(FrameCallback callback) {
options.bindingUtils.instance!
..ensureVisualUpdate()
..addPostFrameCallback(callback);
}

set callback(ScheduledScreenshotRecorderCallback callback) {
_callback = callback;
}
Expand All @@ -63,12 +70,10 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder {
}

Future<void> _stopScheduler() {
_stabilizer.stopped = true;
return _scheduler.stop();
}

void _startScheduler() {
_stabilizer.stopped = false;
_scheduler.start();

// We need to schedule a frame because if this happens in-between user
Expand All @@ -82,7 +87,6 @@ class ScheduledScreenshotRecorder extends ReplayScreenshotRecorder {
options.logger(SentryLevel.debug, "$logName: stopping capture.");
_status = _Status.stopped;
await _stopScheduler();
_stabilizer.dispose();
// await Future.wait([_stopScheduler(), _idleFrameFiller.stop()]);
options.logger(SentryLevel.debug, "$logName: capture stopped.");
}
Expand Down
12 changes: 12 additions & 0 deletions flutter/lib/src/screenshot/recorder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,18 @@ class _Capture<R> {
final recorder = PictureRecorder();
final canvas = Canvas(recorder);
final image = await futureImage;

// Note: there's a weird bug when we write image to canvas directly.
// If the UI is updating quickly in some apps, the image could get
// out-of-sync with the UI and/or it can get completely mangled.
// This can be reproduced, for example, by switching between Spotube's
// Search vs Library (2nd and 3rd bottom bar buttons).
// Weirdly, dumping the image data seems to prevent this issue...
{
// we do so in a block so it can be GC'ed early.
final _ = await image.toByteData();
}

try {
canvas.drawImage(image, Offset.zero, Paint());
} finally {
Expand Down
147 changes: 0 additions & 147 deletions flutter/lib/src/screenshot/stabilizer.dart

This file was deleted.

21 changes: 0 additions & 21 deletions flutter/test/event_processor/screenshot_event_processor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -68,27 +68,6 @@ void main() {
await _addScreenshotAttachment(tester, null, added: true, isWeb: false);
});

testWidgets('does not block if the screenshot fails to stabilize',
(tester) async {
fixture.options.automatedTestMode = false;
fixture.options.experimental.privacy.maskAllText = true;
// Run with real async https://stackoverflow.com/a/54021863
await tester.runAsync(() async {
final sut = fixture.getSut(null, false);

await tester.pumpWidget(SentryScreenshotWidget(
child: Text('Catching Pokémon is a snap!',
textDirection: TextDirection.ltr)));

final throwable = Exception();
event = SentryEvent(throwable: throwable);
hint = Hint();
await sut.apply(event, hint);

expect(hint.screenshot, isNull);
});
});

testWidgets('adds screenshot attachment with canvasKit renderer',
(tester) async {
await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
Expand Down
Loading

0 comments on commit c6b7cb8

Please sign in to comment.