Skip to content
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

chore: screenshot masking #146

Merged
merged 8 commits into from
Jun 26, 2024
Merged

chore: screenshot masking #146

merged 8 commits into from
Jun 26, 2024

Conversation

marandaneto
Copy link
Member

@marandaneto marandaneto commented Jun 25, 2024

💡 Motivation and Context

💚 How did you test it?

Tests running with masking enabled and disabled, with SwiftUI and UIKit

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

Comment on lines +207 to +217
let redactedImage = UIGraphicsImageRenderer(size: image.size, format: .init(for: .init(displayScale: 1))).image { context in
context.cgContext.interpolationQuality = .none
image.draw(at: .zero)

for rect in maskableWidgets {
let path = UIBezierPath(roundedRect: rect, cornerRadius: 10)
UIColor.black.setFill()
path.fill()
}
}
wireframe.base64 = imageToBase64(redactedImage)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible optimization for the future, this can be done async but requires changing a few things and adjusting timestamps, if performance is an issue, this is a place to look at.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've started doing this optimization but led me to do other things that also required to be in the main thread so the changes didn't make much effect, most likely we'll need to pay the memory footprint price instead of processing the images right away, so we can batch the process in background threads but pay the price of holding a copy of the images in memory which can lead to OOM quite quickly as well.

Comment on lines 26 to 28
private let swiftUIImageTypes = ["_TtCOCV7SwiftUI11DisplayList11ViewUpdater8Platform13CGDrawingView",
"_TtC7SwiftUIP33_A34643117F00277B93DEBAB70EC0697122_UIShapeHitTestingView",
"SwiftUI._UIGraphicsView", "SwiftUI.ImageLayer"].compactMap { NSClassFromString($0) }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this changes for every new release or something, some of them are auto-generated but that's the best I found so far.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we won't know until we release but is there a chance that there are more auto-generated types in a customers app?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the types are auto-generated, we cannot just do view as? SwiftUI.Text similarly to UIKit.
So yeah I believe this is a moving thing that we'll discover more over time or new ones will come up within the next few SwiftUI versions, but those are the "root" auto-generated types that work well with the basic widgets at least.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of them are mapped to the UIKit though, for example:
TextField -> UITextField.
navigationTitle -> UILabel

@marandaneto marandaneto requested a review from a team June 25, 2024 14:27
@marandaneto marandaneto marked this pull request as ready for review June 25, 2024 14:27
Copy link

@daibhin daibhin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Could certainly be a blog post or X thread. It's a cool technical solution that's pretty well contained in a single PR and easy to follow with a relative understanding of Swift

Comment on lines 26 to 28
private let swiftUIImageTypes = ["_TtCOCV7SwiftUI11DisplayList11ViewUpdater8Platform13CGDrawingView",
"_TtC7SwiftUIP33_A34643117F00277B93DEBAB70EC0697122_UIShapeHitTestingView",
"SwiftUI._UIGraphicsView", "SwiftUI.ImageLayer"].compactMap { NSClassFromString($0) }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we won't know until we release but is there a chance that there are more auto-generated types in a customers app?

PostHog/Replay/UIView+Util.swift Show resolved Hide resolved
PostHogExample/AppDelegate.swift Show resolved Hide resolved
@marandaneto marandaneto merged commit a5b453c into main Jun 26, 2024
6 checks passed
@marandaneto marandaneto deleted the chore/screenshot-masking branch June 26, 2024 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants