From b78b3073ed9be04abce501d687ff740760c2e060 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Wed, 11 Sep 2024 08:42:01 +0200 Subject: [PATCH 1/3] fix: Dont redact clipped views --- .../iOS-Swift/Base.lproj/Main.storyboard | 25 +++++++- .../Swift/Tools/SentryViewPhotographer.swift | 25 +++++--- Sources/Swift/Tools/UIRedactBuilder.swift | 60 ++++++++++++++----- .../SentryViewPhotographerTests.swift | 36 +++++++++++ Tests/SentryTests/UIRedactBuilderTests.swift | 2 +- 5 files changed, 121 insertions(+), 27 deletions(-) diff --git a/Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard b/Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard index f163b2b6542..e22411a583e 100644 --- a/Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard +++ b/Samples/iOS-Swift/iOS-Swift/Base.lproj/Main.storyboard @@ -1,9 +1,9 @@ - + - + @@ -1139,6 +1139,27 @@ + + + + + + + + + diff --git a/Sources/Swift/Tools/SentryViewPhotographer.swift b/Sources/Swift/Tools/SentryViewPhotographer.swift index a0b539c79bb..d22708f618b 100644 --- a/Sources/Swift/Tools/SentryViewPhotographer.swift +++ b/Sources/Swift/Tools/SentryViewPhotographer.swift @@ -46,25 +46,32 @@ class SentryViewPhotographer: NSObject, SentryViewScreenshotProvider { context.cgContext.addRect(CGRect(origin: CGPoint.zero, size: imageSize)) context.cgContext.clip(using: .evenOdd) + UIColor.blue.setStroke() context.cgContext.interpolationQuality = .none image.draw(at: .zero) for region in redact { - context.cgContext.saveGState() - context.cgContext.concatenate(region.transform) - let rect = CGRect(origin: CGPoint.zero, size: region.size) + var transform = region.transform + let path = CGPath(rect: rect, transform: &transform) + switch region.type { case .redact: - (region.color ?? UIImageHelper.averageColor(of: context.currentImage, at: rect)).setFill() - context.fill(rect) - context.cgContext.restoreGState() - case .clip: + (region.color ?? UIImageHelper.averageColor(of: context.currentImage, at: rect.applying(region.transform))).setFill() + context.cgContext.addPath(path) + context.cgContext.fillPath() + case .clipOut: context.cgContext.addRect(context.cgContext.boundingBoxOfClipPath) - context.cgContext.addRect(rect) - context.cgContext.restoreGState() + context.cgContext.addPath(path) context.cgContext.clip(using: .evenOdd) + case .clipBegin: + context.cgContext.saveGState() + context.cgContext.resetClip() + context.cgContext.addPath(path) + context.cgContext.clip() + case .clipEnd: + context.cgContext.restoreGState() } } } diff --git a/Sources/Swift/Tools/UIRedactBuilder.swift b/Sources/Swift/Tools/UIRedactBuilder.swift index 8f9001dc61e..2acfd789f07 100644 --- a/Sources/Swift/Tools/UIRedactBuilder.swift +++ b/Sources/Swift/Tools/UIRedactBuilder.swift @@ -8,8 +8,20 @@ import WebKit #endif enum RedactRegionType { - case clip + /// Redacts the region. case redact + + /// Marks a region to not draw anything. + /// This is used for opaque views. + case clipOut + + /// Push a clip region to the drawing context. + /// This is used for views that clip to its bounds. + case clipBegin + + /// Pop the last Pushed region from the drawing context. + /// Used after prossing every child of a view that clip to its bounds. + case clipEnd } struct RedactRegion { @@ -137,40 +149,58 @@ class UIRedactBuilder { guard (redactOptions.redactAllImages || redactOptions.redactAllText) && !view.isHidden && view.alpha != 0 else { return } let layer = view.layer.presentation() ?? view.layer - let size = layer.bounds.size - let layerMiddle = CGPoint(x: size.width * layer.anchorPoint.x, y: size.height * layer.anchorPoint.y) - var newTransform = transform.translatedBy(x: layer.position.x, y: layer.position.y) - newTransform = view.transform.concatenating(newTransform) - newTransform = newTransform.translatedBy(x: -layerMiddle.x, y: -layerMiddle.y) + let newTransform = concatenateTranform(transform, with: layer) let ignore = shouldIgnore(view: view) let redact = shouldRedact(view: view, redactOptions: redactOptions) if !ignore && redact { - redacting.append(RedactRegion(size: size, transform: newTransform, type: .redact, color: self.color(for: view))) + redacting.append(RedactRegion(size: layer.bounds.size, transform: newTransform, type: .redact, color: self.color(for: view))) return - } else if isOpaque(view) { - let finalViewFrame = CGRect(origin: .zero, size: size).applying(newTransform) + } + + if isOpaque(view) { + let finalViewFrame = CGRect(origin: .zero, size: layer.bounds.size).applying(newTransform) if isAxisAligned(newTransform) && finalViewFrame == rootFrame { //Because the current view is covering everything we found so far we can clear `redacting` list redacting.removeAll() } else { - redacting.append(RedactRegion(size: size, transform: newTransform, type: .clip)) + redacting.append(RedactRegion(size: layer.bounds.size, transform: newTransform, type: .clipOut)) } } - if !ignore { - for subview in view.subviews { - mapRedactRegion(fromView: subview, redacting: &redacting, rootFrame: rootFrame, redactOptions: redactOptions, transform: newTransform) - } + guard !ignore else { return } + + if view.clipsToBounds { + /// Because the order in which we process the redacted regions is reversed, we add the end of the clip region first. + /// The beginning will be added after all the subviews have been mapped. + redacting.append(RedactRegion(size: layer.bounds.size, transform: newTransform, type: .clipEnd)) } + for subview in view.subviews { + mapRedactRegion(fromView: subview, redacting: &redacting, rootFrame: rootFrame, redactOptions: redactOptions, transform: newTransform) + } + if view.clipsToBounds { + redacting.append(RedactRegion(size: layer.bounds.size, transform: newTransform, type: .clipBegin)) + } + } + + /** + Apply the layer transformation and position to given transformation. + */ + private func concatenateTranform(_ transform: CGAffineTransform, with layer: CALayer) -> CGAffineTransform { + let size = layer.bounds.size + let layerMiddle = CGPoint(x: size.width * layer.anchorPoint.x, y: size.height * layer.anchorPoint.y) + + var newTransform = transform.translatedBy(x: layer.position.x, y: layer.position.y) + newTransform = CATransform3DGetAffineTransform(layer.transform).concatenating(newTransform) + return newTransform.translatedBy(x: -layerMiddle.x, y: -layerMiddle.y) } /** Whether the transform does not contains rotation or skew */ - func isAxisAligned(_ transform: CGAffineTransform) -> Bool { + private func isAxisAligned(_ transform: CGAffineTransform) -> Bool { // Rotation exists if b or c are not zero return transform.b == 0 && transform.c == 0 } diff --git a/Tests/SentryTests/SentryViewPhotographerTests.swift b/Tests/SentryTests/SentryViewPhotographerTests.swift index 041e588f357..df2ff39aa37 100644 --- a/Tests/SentryTests/SentryViewPhotographerTests.swift +++ b/Tests/SentryTests/SentryViewPhotographerTests.swift @@ -140,6 +140,42 @@ class SentryViewPhotographerTests: XCTestCase { assertColor(pixel2, .green) } + func testDontRedactClippedLabel() throws { + let label = UILabel(frame: CGRect(x: 0, y: 25, width: 50, height: 25)) + label.text = "Test" + + let labelParent = UIView(frame: CGRect(x: 0, y: 0, width: 50, height: 25)) + labelParent.backgroundColor = .green + labelParent.clipsToBounds = true + labelParent.addSubview(label) + + let image = try XCTUnwrap(prepare(views: [labelParent])) + let pixel1 = color(at: CGPoint(x: 10, y: 10), in: image) + + assertColor(pixel1, .green) + + let pixel2 = color(at: CGPoint(x: 10, y: 30), in: image) + assertColor(pixel2, .white) + } + + func testRedactLabelInsideClippedView() throws { + let label = UILabel(frame: CGRect(x: 0, y: 0, width: 50, height: 25)) + label.text = "Test" + + let labelParent = UIView(frame: CGRect(x: 0, y: 0, width: 50, height: 25)) + labelParent.backgroundColor = .green + labelParent.clipsToBounds = true + labelParent.addSubview(label) + + let image = try XCTUnwrap(prepare(views: [labelParent])) + let pixel1 = color(at: CGPoint(x: 10, y: 10), in: image) + + assertColor(pixel1, .black) + + let pixel2 = color(at: CGPoint(x: 10, y: 30), in: image) + assertColor(pixel2, .white) + } + private func assertColor(_ color1: UIColor, _ color2: UIColor) { let sRGBColor1 = color1.cgColor.converted(to: CGColorSpace(name: CGColorSpace.sRGB)!, intent: .defaultIntent, options: nil) let sRGBColor2 = color2.cgColor.converted(to: CGColorSpace(name: CGColorSpace.sRGB)!, intent: .defaultIntent, options: nil) diff --git a/Tests/SentryTests/UIRedactBuilderTests.swift b/Tests/SentryTests/UIRedactBuilderTests.swift index a4da3dece20..31e5827e1e2 100644 --- a/Tests/SentryTests/UIRedactBuilderTests.swift +++ b/Tests/SentryTests/UIRedactBuilderTests.swift @@ -134,7 +134,7 @@ class UIRedactBuilderTests: XCTestCase { let result = sut.redactRegionsFor(view: rootView, options: RedactOptions()) XCTAssertEqual(result.count, 1) - XCTAssertEqual(result.first?.type, .clip) + XCTAssertEqual(result.first?.type, .clipOut) XCTAssertEqual(result.first?.transform, CGAffineTransform(a: 1, b: 0, c: 0, d: 1, tx: 10, ty: 10)) } From fa352763fa710c52f22a9855c228d632a225a430 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Wed, 11 Sep 2024 08:43:38 +0200 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdaabbb3a43..f0c9ea697b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Resumes replay when the app becomes active (#4303) - Session replay redact view with transformation (#4308) +- Don't redact clipped views () ## 8.36.0 From 5046131198a9870032dce508fc834bdc9f3e5996 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Wed, 11 Sep 2024 09:04:39 +0200 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0c9ea697b6..1d73d400e06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ - Resumes replay when the app becomes active (#4303) - Session replay redact view with transformation (#4308) -- Don't redact clipped views () +- Don't redact clipped views (#4325) ## 8.36.0