From 5ac6d97bce91a7b611963be26dcd9087b5a1baab Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 17 Dec 2018 15:19:33 -0500 Subject: [PATCH 1/4] Revert "Revert "Debug scaffolding."" This reverts commit a5e71e6eaa71b4ccc7f45bc574b7a1e1c3ebb772. --- .../ConversationView/ConversationViewController.m | 13 +++++++++++++ .../ViewControllers/HomeView/HomeViewController.m | 6 ++++++ 2 files changed, 19 insertions(+) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index f4f9a7a12..27b75feaf 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -1190,6 +1190,19 @@ typedef enum : NSUInteger { // Clear the "on open" state after the view has been presented. self.actionOnOpen = ConversationViewActionNone; + + dispatch_async(dispatch_get_main_queue(), ^{ + NSURL *_Nullable url = [[NSBundle mainBundle] URLForResource:@"qr@2x" withExtension:@"png"]; + OWSAssertDebug(url); + + DataSource *_Nullable dataSource = [DataSourcePath dataSourceWithURL:url shouldDeleteOnDeallocation:NO]; + OWSAssertDebug(dataSource); + SignalAttachment *attachment = [SignalAttachment attachmentWithDataSource:dataSource + dataUTI:(NSString *)kUTTypePNG + imageQuality:TSImageQualityOriginal]; + + [self showApprovalDialogForAttachments:@[ attachment ]]; + }); } // `viewWillDisappear` is called whenever the view *starts* to disappear, diff --git a/Signal/src/ViewControllers/HomeView/HomeViewController.m b/Signal/src/ViewControllers/HomeView/HomeViewController.m index 586ef500e..8438abf18 100644 --- a/Signal/src/ViewControllers/HomeView/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeView/HomeViewController.m @@ -481,6 +481,12 @@ NSString *const kArchivedConversationsReuseIdentifier = @"kArchivedConversations [self.searchResultsController viewDidAppear:animated]; self.hasEverAppeared = YES; + + dispatch_async(dispatch_get_main_queue(), ^{ + NSIndexPath *indexPath = [NSIndexPath indexPathForRow:0 inSection:HomeViewControllerSectionConversations]; + TSThread *thread = [self threadForIndexPath:indexPath]; + [self presentThread:thread action:ConversationViewActionNone animated:YES]; + }); } - (void)viewDidDisappear:(BOOL)animated From a6bc3287797ed4ba097b067db957cca937870aac Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 19 Dec 2018 11:26:25 -0500 Subject: [PATCH 2/4] Revert "Revert "Revert "Debug scaffolding.""" This reverts commit 075ea35bbee4815fb77ecf3024ef29a933cc0ec0. --- .../ConversationView/ConversationViewController.m | 13 ------------- .../ViewControllers/HomeView/HomeViewController.m | 6 ------ 2 files changed, 19 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 27b75feaf..f4f9a7a12 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -1190,19 +1190,6 @@ typedef enum : NSUInteger { // Clear the "on open" state after the view has been presented. self.actionOnOpen = ConversationViewActionNone; - - dispatch_async(dispatch_get_main_queue(), ^{ - NSURL *_Nullable url = [[NSBundle mainBundle] URLForResource:@"qr@2x" withExtension:@"png"]; - OWSAssertDebug(url); - - DataSource *_Nullable dataSource = [DataSourcePath dataSourceWithURL:url shouldDeleteOnDeallocation:NO]; - OWSAssertDebug(dataSource); - SignalAttachment *attachment = [SignalAttachment attachmentWithDataSource:dataSource - dataUTI:(NSString *)kUTTypePNG - imageQuality:TSImageQualityOriginal]; - - [self showApprovalDialogForAttachments:@[ attachment ]]; - }); } // `viewWillDisappear` is called whenever the view *starts* to disappear, diff --git a/Signal/src/ViewControllers/HomeView/HomeViewController.m b/Signal/src/ViewControllers/HomeView/HomeViewController.m index 8438abf18..586ef500e 100644 --- a/Signal/src/ViewControllers/HomeView/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeView/HomeViewController.m @@ -481,12 +481,6 @@ NSString *const kArchivedConversationsReuseIdentifier = @"kArchivedConversations [self.searchResultsController viewDidAppear:animated]; self.hasEverAppeared = YES; - - dispatch_async(dispatch_get_main_queue(), ^{ - NSIndexPath *indexPath = [NSIndexPath indexPathForRow:0 inSection:HomeViewControllerSectionConversations]; - TSThread *thread = [self threadForIndexPath:indexPath]; - [self presentThread:thread action:ConversationViewActionNone animated:YES]; - }); } - (void)viewDidDisappear:(BOOL)animated From 17c3ba058049a582a4c38d3e01ad70d05e39fafe Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 19 Dec 2018 14:39:48 -0500 Subject: [PATCH 3/4] Image editor fixes. --- .../AttachmentApprovalViewController.swift | 2 +- .../ImageEditorGestureRecognizer.swift | 30 ++++++-- .../Views/ImageEditor/ImageEditorModel.swift | 14 ++-- .../Views/ImageEditor/ImageEditorView.swift | 71 ++++++++++++++----- SignalServiceKit/src/Util/NSData+Image.m | 7 +- 5 files changed, 92 insertions(+), 32 deletions(-) diff --git a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift index baeb1f6cb..e84076b8b 100644 --- a/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift +++ b/SignalMessaging/ViewControllers/AttachmentApprovalViewController.swift @@ -950,7 +950,7 @@ public class AttachmentPrepViewController: OWSViewController, PlayerProgressBarD let imageMediaView = mediaMessageView.contentView { let imageEditorView = ImageEditorView(model: imageEditorModel) - if imageEditorView.createImageView() { + if imageEditorView.configureSubviews() { mediaMessageView.isHidden = true imageMediaView.isUserInteractionEnabled = true diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorGestureRecognizer.swift b/SignalMessaging/Views/ImageEditor/ImageEditorGestureRecognizer.swift index 3040fc804..afe779a2c 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorGestureRecognizer.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorGestureRecognizer.swift @@ -6,6 +6,12 @@ import UIKit class ImageEditorGestureRecognizer: UIGestureRecognizer { + @objc + public var shouldAllowOutsideView = true + + @objc + public weak var referenceView: UIView? + @objc override func canPrevent(_ preventedGestureRecognizer: UIGestureRecognizer) -> Bool { return false @@ -95,8 +101,12 @@ class ImageEditorGestureRecognizer: UIGestureRecognizer { } private func touchType(for touches: Set, with event: UIEvent) -> TouchType { - guard let view = self.view else { - owsFailDebug("Missing view") + guard let gestureView = self.view else { + owsFailDebug("Missing gestureView") + return .invalid + } + guard let referenceView = referenceView else { + owsFailDebug("Missing referenceView") return .invalid } guard let allTouches = event.allTouches else { @@ -112,25 +122,31 @@ class ImageEditorGestureRecognizer: UIGestureRecognizer { guard let firstTouch: UITouch = touches.first else { return .invalid } - let location = firstTouch.location(in: view) let isNewTouch = firstTouch.phase == .began if isNewTouch { // Reject new touches that are inside a control subview. - if subviewControl(ofView: view, contains: firstTouch) { + if subviewControl(ofView: gestureView, contains: firstTouch) { return .invalid } } // Reject new touches outside this GR's view's bounds. - guard view.bounds.contains(location) else { - return isNewTouch ? .invalid : .outside + let location = firstTouch.location(in: referenceView) + if !referenceView.bounds.contains(location) { + if shouldAllowOutsideView { + // Do nothing + } else if isNewTouch { + return .invalid + } else { + return .outside + } } if isNewTouch { // Ignore touches that start near the top or bottom edge of the screen; // they may be a system edge swipe gesture. - let rootView = self.rootView(of: view) + let rootView = self.rootView(of: gestureView) let rootLocation = firstTouch.location(in: rootView) let distanceToTopEdge = max(0, rootLocation.y) let distanceToBottomEdge = max(0, rootView.bounds.size.height - rootLocation.y) diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift b/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift index d77707d26..898509f05 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift @@ -500,7 +500,7 @@ public class ImageEditorModel: NSObject { public func crop(unitCropRect: CGRect) { guard let croppedImage = ImageEditorModel.crop(imagePath: contents.imagePath, unitCropRect: unitCropRect) else { - owsFailDebug("Could not crop image.") + Logger.warn("Could not crop image.") return } // Use PNG for temp files; PNG is lossless. @@ -584,10 +584,10 @@ public class ImageEditorModel: NSObject { } let srcImageSize = srcImage.size // Convert from unit coordinates to src image coordinates. - let cropRect = CGRect(x: unitCropRect.origin.x * srcImageSize.width, - y: unitCropRect.origin.y * srcImageSize.height, - width: unitCropRect.size.width * srcImageSize.width, - height: unitCropRect.size.height * srcImageSize.height) + let cropRect = CGRect(x: round(unitCropRect.origin.x * srcImageSize.width), + y: round(unitCropRect.origin.y * srcImageSize.height), + width: round(unitCropRect.size.width * srcImageSize.width), + height: round(unitCropRect.size.height * srcImageSize.height)) guard cropRect.origin.x >= 0, cropRect.origin.y >= 0, @@ -598,7 +598,9 @@ public class ImageEditorModel: NSObject { } guard cropRect.size.width > 0, cropRect.size.height > 0 else { - owsFailDebug("Empty crop rectangle.") + // Not an error; indicates that the user tapped rather + // than dragged. + Logger.warn("Empty crop rectangle.") return nil } diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift index bc702edab..4d3baea43 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift @@ -15,7 +15,20 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { case crop } - private var editorMode = EditorMode.brush + private var editorMode = EditorMode.brush { + didSet { + AssertIsOnMainThread() + + switch editorMode { + case .brush: + // Brush strokes can start and end (and return from) outside the view. + editorGestureRecognizer?.shouldAllowOutsideView = true + case .crop: + // Crop gestures can start and end (and return from) outside the view. + editorGestureRecognizer?.shouldAllowOutsideView = true + } + } + } @objc public required init(model: ImageEditorModel) { @@ -36,9 +49,10 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { private let imageView = UIImageView() private var imageViewConstraints = [NSLayoutConstraint]() private let layersView = OWSLayerView() + private var editorGestureRecognizer: ImageEditorGestureRecognizer? @objc - public func createImageView() -> Bool { + public func configureSubviews() -> Bool { self.addSubview(imageView) guard updateImageView() else { @@ -54,8 +68,10 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { self.isUserInteractionEnabled = true layersView.isUserInteractionEnabled = true - let anyTouchGesture = ImageEditorGestureRecognizer(target: self, action: #selector(handleTouchGesture(_:))) - layersView.addGestureRecognizer(anyTouchGesture) + let editorGestureRecognizer = ImageEditorGestureRecognizer(target: self, action: #selector(handleTouchGesture(_:))) + editorGestureRecognizer.referenceView = layersView + self.addGestureRecognizer(editorGestureRecognizer) + self.editorGestureRecognizer = editorGestureRecognizer return true } @@ -217,6 +233,15 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { self.currentStroke = nil self.currentStrokeSamples.removeAll() } + let tryToAppendStrokeSample = { + let newSample = self.unitSampleForGestureLocation(gestureRecognizer, shouldClamp: false) + if let prevSample = self.currentStrokeSamples.last, + prevSample == newSample { + // Ignore duplicate samples. + return + } + self.currentStrokeSamples.append(newSample) + } // TODO: Color picker. let strokeColor = UIColor.blue @@ -227,14 +252,14 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { case .began: removeCurrentStroke() - currentStrokeSamples.append(unitSampleForGestureLocation(gestureRecognizer)) + tryToAppendStrokeSample() let stroke = ImageEditorStrokeItem(color: strokeColor, unitSamples: currentStrokeSamples, unitStrokeWidth: unitStrokeWidth) model.append(item: stroke) currentStroke = stroke case .changed, .ended: - currentStrokeSamples.append(unitSampleForGestureLocation(gestureRecognizer)) + tryToAppendStrokeSample() guard let lastStroke = self.currentStroke else { owsFailDebug("Missing last stroke.") @@ -258,12 +283,17 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { } } - private func unitSampleForGestureLocation(_ gestureRecognizer: UIGestureRecognizer) -> CGPoint { + private func unitSampleForGestureLocation(_ gestureRecognizer: UIGestureRecognizer, + shouldClamp: Bool) -> CGPoint { let referenceView = layersView // TODO: Smooth touch samples before converting into stroke samples. let location = gestureRecognizer.location(in: referenceView) - let x = CGFloatClamp01(CGFloatInverseLerp(location.x, 0, referenceView.bounds.width)) - let y = CGFloatClamp01(CGFloatInverseLerp(location.y, 0, referenceView.bounds.height)) + var x = CGFloatInverseLerp(location.x, 0, referenceView.bounds.width) + var y = CGFloatInverseLerp(location.y, 0, referenceView.bounds.height) + if shouldClamp { + x = CGFloatClamp01(x) + y = CGFloatClamp01(y) + } return CGPoint(x: x, y: y) } @@ -350,18 +380,22 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { self.model.crop(unitCropRect: unitCropRect) } + let currentUnitSample = { + self.unitSampleForGestureLocation(gestureRecognizer, shouldClamp: true) + } + switch gestureRecognizer.state { case .began: - let unitSample = unitSampleForGestureLocation(gestureRecognizer) + let unitSample = currentUnitSample() cropStartUnit = unitSample cropEndUnit = unitSample startCrop() case .changed: - cropEndUnit = unitSampleForGestureLocation(gestureRecognizer) + cropEndUnit = currentUnitSample() updateCrop() case .ended: - cropEndUnit = unitSampleForGestureLocation(gestureRecognizer) + cropEndUnit = currentUnitSample() endCrop() default: cancelCrop() @@ -501,12 +535,10 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { viewSize: CGSize) -> CALayer? { AssertIsOnMainThread() - Logger.verbose("\(item.itemId), viewSize: \(viewSize)") - let strokeWidth = ImageEditorStrokeItem.strokeWidth(forUnitStrokeWidth: item.unitStrokeWidth, dstSize: viewSize) let unitSamples = item.unitSamples - guard unitSamples.count > 1 else { + guard unitSamples.count > 0 else { // Not an error; the stroke doesn't have enough samples to render yet. return nil } @@ -532,7 +564,10 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { let point = points[index] let forwardVector: CGPoint - if index == 0 { + if points.count <= 1 { + // Skip forward vectors. + forwardVector = .zero + } else if index == 0 { // First sample. let nextPoint = points[index + 1] forwardVector = CGPointSubtract(nextPoint, point) @@ -552,6 +587,10 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { if index == 0 { // First sample. bezierPath.move(to: point) + + if points.count == 1 { + bezierPath.addLine(to: point) + } } else { let previousPoint = points[index - 1] // We apply more than one kind of smoothing. diff --git a/SignalServiceKit/src/Util/NSData+Image.m b/SignalServiceKit/src/Util/NSData+Image.m index b87fb3d90..4ba9b9673 100644 --- a/SignalServiceKit/src/Util/NSData+Image.m +++ b/SignalServiceKit/src/Util/NSData+Image.m @@ -388,11 +388,14 @@ typedef NS_ENUM(NSInteger, ImageFormat) { = (__bridge_transfer NSDictionary *)CGImageSourceCopyPropertiesAtIndex(source, 0, (CFDictionaryRef)options); BOOL result = NO; if (properties) { - NSNumber *hasAlpha = properties[(NSString *)kCGImagePropertyHasAlpha]; + NSNumber *_Nullable hasAlpha = properties[(NSString *)kCGImagePropertyHasAlpha]; if (hasAlpha) { result = hasAlpha.boolValue; } else { - OWSFailDebug(@"Could not determine transparency of image: %@", url); + // This is not an error; kCGImagePropertyHasAlpha is an optional + // property. + OWSLogWarn(@"Could not determine transparency of image: %@", url); + result = NO; } } CFRelease(source); From 5dcde44486c5319a1e8b2dbb346bc3af10c03444 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 19 Dec 2018 14:41:26 -0500 Subject: [PATCH 4/4] Image editor fixes. --- .../ImageEditor/ImageEditorGestureRecognizer.swift | 10 +++++----- .../Views/ImageEditor/ImageEditorModel.swift | 2 ++ .../Views/ImageEditor/ImageEditorView.swift | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorGestureRecognizer.swift b/SignalMessaging/Views/ImageEditor/ImageEditorGestureRecognizer.swift index afe779a2c..2fab76936 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorGestureRecognizer.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorGestureRecognizer.swift @@ -10,7 +10,7 @@ class ImageEditorGestureRecognizer: UIGestureRecognizer { public var shouldAllowOutsideView = true @objc - public weak var referenceView: UIView? + public weak var canvasView: UIView? @objc override func canPrevent(_ preventedGestureRecognizer: UIGestureRecognizer) -> Bool { @@ -105,8 +105,8 @@ class ImageEditorGestureRecognizer: UIGestureRecognizer { owsFailDebug("Missing gestureView") return .invalid } - guard let referenceView = referenceView else { - owsFailDebug("Missing referenceView") + guard let canvasView = canvasView else { + owsFailDebug("Missing canvasView") return .invalid } guard let allTouches = event.allTouches else { @@ -132,8 +132,8 @@ class ImageEditorGestureRecognizer: UIGestureRecognizer { } // Reject new touches outside this GR's view's bounds. - let location = firstTouch.location(in: referenceView) - if !referenceView.bounds.contains(location) { + let location = firstTouch.location(in: canvasView) + if !canvasView.bounds.contains(location) { if shouldAllowOutsideView { // Do nothing } else if isNewTouch { diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift b/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift index 898509f05..5d1b73d2f 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift @@ -500,6 +500,8 @@ public class ImageEditorModel: NSObject { public func crop(unitCropRect: CGRect) { guard let croppedImage = ImageEditorModel.crop(imagePath: contents.imagePath, unitCropRect: unitCropRect) else { + // Not an error; user might have tapped or + // otherwise drawn an invalid crop region. Logger.warn("Could not crop image.") return } diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift index 4d3baea43..2dfcd4268 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift @@ -69,7 +69,7 @@ public class ImageEditorView: UIView, ImageEditorModelDelegate { self.isUserInteractionEnabled = true layersView.isUserInteractionEnabled = true let editorGestureRecognizer = ImageEditorGestureRecognizer(target: self, action: #selector(handleTouchGesture(_:))) - editorGestureRecognizer.referenceView = layersView + editorGestureRecognizer.canvasView = layersView self.addGestureRecognizer(editorGestureRecognizer) self.editorGestureRecognizer = editorGestureRecognizer