From 65ead451c083fc98d53f0377d41d6240a71e682a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 28 Feb 2019 18:02:57 -0500 Subject: [PATCH 1/5] Don't enable undo in stroke view for items created before stroke view. --- .../ImageEditor/ImageEditorBrushViewController.swift | 10 +++++++++- .../Views/ImageEditor/ImageEditorModel.swift | 11 +++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift index 4f9e5ccfd..9e677b2e7 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift @@ -23,6 +23,11 @@ public class ImageEditorBrushViewController: OWSViewController { private var brushGestureRecognizer: ImageEditorPanGestureRecognizer? + // We only want to let users undo changes made in this view. + // So we snapshot any older "operation id" and prevent + // users from undoing it. + private let firstUndoOperationId: String? + init(delegate: ImageEditorBrushViewControllerDelegate, model: ImageEditorModel, currentColor: ImageEditorColor) { @@ -30,6 +35,7 @@ public class ImageEditorBrushViewController: OWSViewController { self.model = model self.canvasView = ImageEditorCanvasView(model: model) self.paletteView = ImageEditorPaletteView(currentColor: currentColor) + self.firstUndoOperationId = model.currentUndoOperationId() super.init(nibName: nil, bundle: nil) @@ -86,8 +92,10 @@ public class ImageEditorBrushViewController: OWSViewController { let doneButton = navigationBarButton(imageName: "image_editor_checkmark_full", selector: #selector(didTapDone(sender:))) + // Prevent users from undo any changes made before entering the view. + let canUndo = model.canUndo() && firstUndoOperationId != model.currentUndoOperationId() var navigationBarItems = [UIView]() - if model.canUndo() { + if canUndo { navigationBarItems = [undoButton, doneButton] } else { navigationBarItems = [doneButton] diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift b/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift index ab6daed7a..cd7272096 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorModel.swift @@ -242,9 +242,12 @@ public class ImageEditorTransform: NSObject { // (multiple times) to preserve/restore editor state. private class ImageEditorOperation: NSObject { + let operationId: String + let contents: ImageEditorContents required init(contents: ImageEditorContents) { + self.operationId = UUID().uuidString self.contents = contents } } @@ -361,6 +364,14 @@ public class ImageEditorModel: NSObject { return !redoStack.isEmpty } + @objc + public func currentUndoOperationId() -> String? { + guard let operation = undoStack.last else { + return nil + } + return operation.operationId + } + // MARK: - Observers private var observers = [Weak]() From 93cb0e3a102e84f43d290391d3837658a09aab18 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 1 Mar 2019 09:53:56 -0500 Subject: [PATCH 2/5] Fix bar button layout on iOS 9. --- .../OWSViewController+ImageEditor.swift | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/SignalMessaging/Views/ImageEditor/OWSViewController+ImageEditor.swift b/SignalMessaging/Views/ImageEditor/OWSViewController+ImageEditor.swift index 7c2e46b11..a7f50cf3e 100644 --- a/SignalMessaging/Views/ImageEditor/OWSViewController+ImageEditor.swift +++ b/SignalMessaging/Views/ImageEditor/OWSViewController+ImageEditor.swift @@ -29,11 +29,24 @@ public extension UIViewController { return } + let spacing: CGFloat = 8 let stackView = UIStackView(arrangedSubviews: navigationBarItems) stackView.axis = .horizontal - stackView.spacing = 8 + stackView.spacing = spacing stackView.alignment = .center + // Ensure layout works on older versions of iOS. + var stackSize = CGSize.zero + for item in navigationBarItems { + let itemSize = item.sizeThatFits(.zero) + stackSize.width += itemSize.width + spacing + stackSize.height = max(stackSize.height, itemSize.height) + } + if navigationBarItems.count > 0 { + stackSize.width -= spacing + } + stackView.frame = CGRect(origin: .zero, size: stackSize) + self.navigationItem.rightBarButtonItem = UIBarButtonItem(customView: stackView) } } From 3d96cd488e0d6208e38dc1008d7bc1224b83a96e Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 1 Mar 2019 10:37:22 -0500 Subject: [PATCH 3/5] Improve color continuity in the image editor. --- .../ImageEditor/ImageEditorBrushViewController.swift | 4 ++-- .../Views/ImageEditor/ImageEditorView.swift | 11 ++++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift index 9e677b2e7..0ba64c3c2 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift @@ -6,7 +6,7 @@ import UIKit @objc public protocol ImageEditorBrushViewControllerDelegate: class { - func brushDidComplete() + func brushDidComplete(currentColor: ImageEditorColor) } // MARK: - @@ -121,7 +121,7 @@ public class ImageEditorBrushViewController: OWSViewController { } private func completeAndDismiss() { - self.delegate?.brushDidComplete() + self.delegate?.brushDidComplete(currentColor: paletteView.selectedValue) self.dismiss(animated: false) { // Do nothing. diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift index 146c0a2de..ba38915eb 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift @@ -24,7 +24,8 @@ public class ImageEditorView: UIView { private let canvasView: ImageEditorCanvasView - // TODO: We could hang this on the model or make this static. + // TODO: We could hang this on the model or make this static + // if we wanted more color continuity. private var currentColor = ImageEditorColor.defaultColor() @objc @@ -444,7 +445,8 @@ public class ImageEditorView: UIView { let cropTool = ImageEditorCropViewController(delegate: self, model: model, srcImage: srcImage, previewImage: previewImage) self.delegate?.imageEditor(presentFullScreenView: cropTool, isTransparent: false) - }} + } +} // MARK: - @@ -498,6 +500,8 @@ extension ImageEditorView: ImageEditorTextViewControllerDelegate { } else { model.append(item: newItem) } + + self.currentColor = color } public func textEditDidCancel() { @@ -520,6 +524,7 @@ extension ImageEditorView: ImageEditorCropViewControllerDelegate { // MARK: - extension ImageEditorView: ImageEditorBrushViewControllerDelegate { - public func brushDidComplete() { + public func brushDidComplete(currentColor: ImageEditorColor) { + self.currentColor = currentColor } } From 1a159d4d70916c668600b051e8e187214918d84d Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 1 Mar 2019 10:37:36 -0500 Subject: [PATCH 4/5] Clean up brush stroke gesture usage. --- .../ImageEditorBrushViewController.swift | 16 ++++++-- .../ImageEditorCropViewController.swift | 6 +-- .../ImageEditorPanGestureRecognizer.swift | 40 ++++++++++++++----- .../Views/ImageEditor/ImageEditorView.swift | 4 +- 4 files changed, 46 insertions(+), 20 deletions(-) diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift index 0ba64c3c2..afa220829 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift @@ -21,8 +21,6 @@ public class ImageEditorBrushViewController: OWSViewController { private let paletteView: ImageEditorPaletteView - private var brushGestureRecognizer: ImageEditorPanGestureRecognizer? - // We only want to let users undo changes made in this view. // So we snapshot any older "operation id" and prevent // users from undoing it. @@ -68,8 +66,8 @@ public class ImageEditorBrushViewController: OWSViewController { let brushGestureRecognizer = ImageEditorPanGestureRecognizer(target: self, action: #selector(handleBrushGesture(_:))) brushGestureRecognizer.maximumNumberOfTouches = 1 brushGestureRecognizer.referenceView = canvasView.gestureReferenceView + brushGestureRecognizer.delegate = self self.view.addGestureRecognizer(brushGestureRecognizer) - self.brushGestureRecognizer = brushGestureRecognizer updateNavigationBar() } @@ -171,7 +169,7 @@ public class ImageEditorBrushViewController: OWSViewController { // Apply the location history of the gesture so that the stroke reflects // the touch's movement before the gesture recognized. - for location in gestureRecognizer.locations { + for location in gestureRecognizer.locationHistory { tryToAppendStrokeSample(location) } @@ -230,3 +228,13 @@ extension ImageEditorBrushViewController: ImageEditorPaletteViewDelegate { // TODO: } } + +// MARK: - + +extension ImageEditorBrushViewController: UIGestureRecognizerDelegate { + @objc public func gestureRecognizer(_ gestureRecognizer: UIGestureRecognizer, shouldReceive touch: UITouch) -> Bool { + // Ignore touches that begin inside the palette. + let location = touch.location(in: paletteView) + return !paletteView.bounds.contains(location) + } +} diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorCropViewController.swift b/SignalMessaging/Views/ImageEditor/ImageEditorCropViewController.swift index 814a403ac..e3e2b0b0d 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorCropViewController.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorCropViewController.swift @@ -501,7 +501,7 @@ class ImageEditorCropViewController: OWSViewController { Logger.verbose("") - guard let locationStart = gestureRecognizer.locationStart else { + guard let locationStart = gestureRecognizer.locationFirst else { owsFailDebug("Missing locationStart.") return } @@ -666,7 +666,7 @@ class ImageEditorCropViewController: OWSViewController { owsFailDebug("Missing pinchTransform.") return } - guard let oldLocationView = gestureRecognizer.locationStart else { + guard let oldLocationView = gestureRecognizer.locationFirst else { owsFailDebug("Missing locationStart.") return } @@ -685,7 +685,7 @@ class ImageEditorCropViewController: OWSViewController { } private func cropRegion(forGestureRecognizer gestureRecognizer: ImageEditorPanGestureRecognizer) -> CropRegion? { - guard let location = gestureRecognizer.locationStart else { + guard let location = gestureRecognizer.locationFirst else { owsFailDebug("Missing locationStart.") return nil } diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorPanGestureRecognizer.swift b/SignalMessaging/Views/ImageEditor/ImageEditorPanGestureRecognizer.swift index 835df3b93..699a2c832 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorPanGestureRecognizer.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorPanGestureRecognizer.swift @@ -14,39 +14,57 @@ public class ImageEditorPanGestureRecognizer: UIPanGestureRecognizer { public weak var referenceView: UIView? // Capture the location history of this gesture. - public var locations = [CGPoint]() + public var locationHistory = [CGPoint]() - public var locationStart: CGPoint? { - return locations.first + public var locationFirst: CGPoint? { + return locationHistory.first } // MARK: - Touch Handling @objc public override func touchesBegan(_ touches: Set, with event: UIEvent) { - super.touchesBegan(touches, with: event) + updateLocationHistory(event: event) - guard let referenceView = referenceView else { - owsFailDebug("Missing view") - return - } - locations.append(location(in: referenceView)) + super.touchesBegan(touches, with: event) } @objc public override func touchesMoved(_ touches: Set, with event: UIEvent) { + updateLocationHistory(event: event) + super.touchesMoved(touches, with: event) + } + + @objc + public override func touchesEnded(_ touches: Set, with event: UIEvent) { + updateLocationHistory(event: event) + super.touchesEnded(touches, with: event) + } + + private func updateLocationHistory(event: UIEvent) { + guard let touches = event.allTouches, + touches.count > 0 else { + owsFailDebug("no touches.") + return + } guard let referenceView = referenceView else { owsFailDebug("Missing view") return } - locations.append(location(in: referenceView)) + // Find the centroid. + var location = CGPoint.zero + for touch in touches { + location = location.plus(touch.location(in: referenceView)) + } + location = location.times(CGFloat(1) / CGFloat(touches.count)) + locationHistory.append(location) } public override func reset() { super.reset() - locations.removeAll() + locationHistory.removeAll() } } diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift index ba38915eb..8c31e1907 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorView.swift @@ -274,7 +274,7 @@ public class ImageEditorView: UIView { switch gestureRecognizer.state { case .began: - guard let locationStart = gestureRecognizer.locationStart else { + guard let locationStart = gestureRecognizer.locationFirst else { owsFailDebug("Missing locationStart.") return } @@ -294,7 +294,7 @@ public class ImageEditorView: UIView { guard let textItem = movingTextItem else { return } - guard let locationStart = gestureRecognizer.locationStart else { + guard let locationStart = gestureRecognizer.locationFirst else { owsFailDebug("Missing locationStart.") return } From 871dceac3a6b84c39999bfcc7aac5675e30f656a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 1 Mar 2019 11:21:19 -0500 Subject: [PATCH 5/5] Improve palette interactions. --- .../ImageEditorBrushViewController.swift | 2 +- .../ImageEditor/ImageEditorPaletteView.swift | 98 ++++++++----------- .../ImageEditorTextViewController.swift | 4 +- 3 files changed, 44 insertions(+), 60 deletions(-) diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift index afa220829..63a7590a7 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorBrushViewController.swift @@ -59,7 +59,7 @@ public class ImageEditorBrushViewController: OWSViewController { paletteView.delegate = self self.view.addSubview(paletteView) paletteView.autoVCenterInSuperview() - paletteView.autoPinEdge(toSuperviewEdge: .trailing, withInset: 20) + paletteView.autoPinEdge(toSuperviewEdge: .trailing, withInset: 0) self.view.isUserInteractionEnabled = true diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorPaletteView.swift b/SignalMessaging/Views/ImageEditor/ImageEditorPaletteView.swift index a4040e774..2e7689604 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorPaletteView.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorPaletteView.swift @@ -97,9 +97,8 @@ public class ImageEditorPaletteView: UIView { owsFailDebug("Missing image.") } addSubview(imageView) - // We use an invisible margin to expand the hot area of - // this control. - let margin: CGFloat = 8 + // We use an invisible margin to expand the hot area of this control. + let margin: CGFloat = 20 imageView.autoPinEdgesToSuperviewEdges(with: UIEdgeInsets(top: margin, left: margin, bottom: margin, right: margin)) selectionWrapper.layoutCallback = { [weak self] (view) in @@ -144,14 +143,49 @@ public class ImageEditorPaletteView: UIView { } private func value(for palettePhase: CGFloat) -> ImageEditorColor { - guard let image = imageView.image else { - owsFailDebug("Missing image.") + // We find the color in the palette's gradient that corresponds + // to the "phase". + // + // 0 = top of gradient, first color. + // 1 = bottom of gradient, last color. + struct GradientSegment { + let color0: UIColor + let color1: UIColor + let palettePhase0: CGFloat + let palettePhase1: CGFloat + } + var segments = [GradientSegment]() + let segmentCount = ImageEditorColor.gradientUIColors.count - 1 + var prevColor: UIColor? + for color in ImageEditorColor.gradientUIColors { + if let color0 = prevColor { + let index = CGFloat(segments.count) + let color1 = color + let palettePhase0: CGFloat = index / CGFloat(segmentCount) + let palettePhase1: CGFloat = (index + 1) / CGFloat(segmentCount) + segments.append(GradientSegment(color0: color0, color1: color1, palettePhase0: palettePhase0, palettePhase1: palettePhase1)) + } + prevColor = color + } + var bestSegment = segments.first + for segment in segments { + if palettePhase >= segment.palettePhase0 { + bestSegment = segment + } + } + guard let segment = bestSegment else { + owsFailDebug("Couldn't find matching segment.") return ImageEditorColor.defaultColor() } - guard let color = image.color(atLocation: CGPoint(x: CGFloat(image.size.width) * 0.5, y: CGFloat(image.size.height) * palettePhase)) else { - owsFailDebug("Missing color.") + guard palettePhase >= segment.palettePhase0, + palettePhase <= segment.palettePhase1 else { + owsFailDebug("Invalid segment.") return ImageEditorColor.defaultColor() } + let segmentPhase = palettePhase.inverseLerp(segment.palettePhase0, segment.palettePhase1).clamp01() + // If CAGradientLayer doesn't do naive RGB color interpolation, + // this won't be WYSIWYG. + let color = segment.color0.blend(with: segment.color1, alpha: segmentPhase) return ImageEditorColor(color: color, palettePhase: palettePhase) } @@ -170,7 +204,6 @@ public class ImageEditorPaletteView: UIView { @objc func didTouch(gesture: UIGestureRecognizer) { - Logger.verbose("gesture: \(NSStringForUIGestureRecognizerState(gesture.state))") switch gesture.state { case .began, .changed, .ended: break @@ -201,55 +234,6 @@ public class ImageEditorPaletteView: UIView { // MARK: - -extension UIImage { - func color(atLocation locationPoints: CGPoint) -> UIColor? { - guard let cgImage = cgImage else { - owsFailDebug("Missing cgImage.") - return nil - } - guard let dataProvider = cgImage.dataProvider else { - owsFailDebug("Could not create dataProvider.") - return nil - } - guard let pixelData = dataProvider.data else { - owsFailDebug("dataProvider has no data.") - return nil - } - let bytesPerPixel: Int = cgImage.bitsPerPixel / 8 - guard bytesPerPixel == 4 else { - owsFailDebug("Invalid bytesPerPixel: \(bytesPerPixel).") - return nil - } - let imageWidth: Int = cgImage.width - let imageHeight: Int = cgImage.height - guard imageWidth > 0, - imageHeight > 0 else { - owsFailDebug("Invalid image size.") - return nil - } - - // Convert the location from points to pixels and clamp to the image bounds. - let xPixels: Int = Int(round(locationPoints.x * self.scale)).clamp(0, imageWidth - 1) - let yPixels: Int = Int(round(locationPoints.y * self.scale)).clamp(0, imageHeight - 1) - let dataLength = (pixelData as Data).count - let data: UnsafePointer = CFDataGetBytePtr(pixelData) - let index: Int = (imageWidth * yPixels + xPixels) * bytesPerPixel - guard index >= 0, index < dataLength else { - owsFailDebug("Invalid index.") - return nil - } - - let red = CGFloat(data[index]) / CGFloat(255.0) - let green = CGFloat(data[index+1]) / CGFloat(255.0) - let blue = CGFloat(data[index+2]) / CGFloat(255.0) - let alpha = CGFloat(data[index+3]) / CGFloat(255.0) - - return UIColor(red: red, green: green, blue: blue, alpha: alpha) - } -} - -// MARK: - - // The most permissive GR possible. Accepts any number of touches in any locations. private class PaletteGestureRecognizer: UIGestureRecognizer { diff --git a/SignalMessaging/Views/ImageEditor/ImageEditorTextViewController.swift b/SignalMessaging/Views/ImageEditor/ImageEditorTextViewController.swift index a865aa5e3..f243a6f90 100644 --- a/SignalMessaging/Views/ImageEditor/ImageEditorTextViewController.swift +++ b/SignalMessaging/Views/ImageEditor/ImageEditorTextViewController.swift @@ -187,9 +187,9 @@ public class ImageEditorTextViewController: OWSViewController, VAlignTextViewDel paletteView.delegate = self self.view.addSubview(paletteView) paletteView.autoAlignAxis(.horizontal, toSameAxisOf: textView) - paletteView.autoPinEdge(toSuperviewEdge: .trailing, withInset: 20) + paletteView.autoPinEdge(toSuperviewEdge: .trailing, withInset: 0) // This will determine the text view's size. - paletteView.autoPinEdge(.leading, to: .trailing, of: textView, withOffset: 8) + paletteView.autoPinEdge(.leading, to: .trailing, of: textView, withOffset: 0) updateNavigationBar() }