From 17c3ba058049a582a4c38d3e01ad70d05e39fafe Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 19 Dec 2018 14:39:48 -0500 Subject: [PATCH] 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);