-
Notifications
You must be signed in to change notification settings - Fork 6
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
Capture pixel screenshots implementation on iOS #99
Changes from 4 commits
29d6bcc
492dc9d
51cf3e2
80230c2
6627105
8bf82b2
0c8efa4
e43ff93
5dc579a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
|
||
@_implementationOnly import CapturePassable | ||
import Foundation | ||
import UIKit | ||
|
||
final class SessionReplayTarget { | ||
private let queue = DispatchQueue.serial(withLabelSuffix: "ReplayController", target: .default) | ||
|
@@ -41,14 +42,36 @@ extension SessionReplayTarget: CapturePassable.SessionReplayTarget { | |
} | ||
|
||
func captureScreenshot() { | ||
// TODO: Implement | ||
// DispatchQueue.main.async { [weak self] in | ||
// self?.queue.async { | ||
// self?.logger?.logSessionReplayScreenshot( | ||
// screen: SessionReplayCapture(data: Data()), | ||
// duration: 0 | ||
// ) | ||
// } | ||
// } | ||
DispatchQueue.main.async { | ||
guard let window = UIApplication.shared.sessionReplayWindows().first else { | ||
// According to the documentation for | ||
// `CapturePassable.SessionReplayTarget.captureScreenshot()`, this method must emit | ||
// a screenshot log. Without this, the Rust logger will ignore any further screenshot | ||
// capture actions. | ||
// To handle cases where no window is available, an assertion is included here. | ||
// TODO: Add a log to inform the Rust layer if the screenshot log emission fails, allowing it | ||
// to proceed to the next screenshot (if any). | ||
assertionFailure("no window to take screenshot of") | ||
return | ||
} | ||
|
||
let layer = window.layer | ||
let bounds = UIScreen.main.bounds.size | ||
|
||
self.queue.async { [weak self] in | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Reflejo for my android-side understanding: would these two operations happen on separate threads? are any of these new worker threads or do we re-use existing ones? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similarly, those these threads get spawned even when the runtime flag There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Queues do not map 1:1 to threads, we actually use a default queue for the target here meaning this won't create an additional thread. The lines that are under the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm more confused than before? then what's the diff between queue and thread? what does default mean? |
||
let start = Uptime() | ||
let format = UIGraphicsImageRendererFormat() | ||
format.scale = 1.0 | ||
|
||
let renderer = UIGraphicsImageRenderer(size: bounds, format: format) | ||
let jpeg = renderer.jpegData(withCompressionQuality: 0.1) { context in | ||
layer.render(in: context.cgContext) | ||
} | ||
self?.logger?.logSessionReplayScreenshot( | ||
screen: SessionReplayCapture(data: jpeg), | ||
duration: Uptime().timeIntervalSince(start) | ||
) | ||
} | ||
} | ||
} | ||
} |
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.
If that’s the case might be better to send an empty data object back for now, wdyt? @Augustyniak
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.
Sure, makes sense - so if we send a screenshot log without
screen
field set then you will handle it somehow on the frontend layer?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.
or do you want me to somehow annotated such log with an extra attribute?
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.
We'll handle in the fe
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.
cool, let me make a change in your PR in here