Skip to content

Commit

Permalink
Fix CircularProgress crash (#142)
Browse files Browse the repository at this point in the history
  • Loading branch information
sindresorhus committed Oct 8, 2019
1 parent 91dd426 commit 20bc098
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 26 deletions.
2 changes: 0 additions & 2 deletions Gifski.xcodeproj/xcshareddata/xcschemes/Gifski.xcscheme
Expand Up @@ -59,8 +59,6 @@
useCustomWorkingDirectory = "NO"
ignoresPersistentStateOnLaunch = "NO"
debugDocumentVersioning = "YES"
stopOnEveryThreadSanitizerIssue = "YES"
stopOnEveryMainThreadCheckerIssue = "YES"
migratedStopOnEveryIssue = "YES"
debugServiceExtension = "internal"
allowLocationSimulation = "YES">
Expand Down
9 changes: 9 additions & 0 deletions Gifski/Vendor/CircularProgress+Util.swift
Expand Up @@ -225,3 +225,12 @@ extension NSWindow {
attachedSheet != nil
}
}


func assertMainThread(
function: StaticString = #function,
file: String = #file,
line: UInt = #line
) {
assert(Thread.isMainThread, "\(function) in \(file.nsString.lastPathComponent):\(line) must run on the main thread!")
}
75 changes: 51 additions & 24 deletions Gifski/Vendor/CircularProgress.swift
Expand Up @@ -61,8 +61,12 @@ public final class CircularProgress: NSView {
Defaults to the user's accent color. For High Sierra and below it uses a fallback color.
*/
@IBInspectable public var color: NSColor {
get { _color }
get {
assertMainThread()
return _color
}
set {
assertMainThread()
_color = newValue
originalColor = newValue
needsDisplay = true
Expand All @@ -78,21 +82,23 @@ public final class CircularProgress: NSView {
The progress value in the range `0...1`.
- Note: The value will be clamped to `0...1`.
- Note: Can be set from a background thread.
*/
@IBInspectable public var progress: Double {
get { _progress }
get {
assertMainThread()
return _progress
}
set {
assertMainThread()

_progress = newValue.clamped(to: 0...1)

// swiftlint:disable:next trailing_closure
CALayer.animate(duration: 0.5, timingFunction: .easeOut, animations: {
self.progressCircle.progress = self._progress
})

DispatchQueue.main.async {
self.progressLabel.isHidden = self.progress == 0 && self.isIndeterminate ? self.cancelButton.isHidden : !self.cancelButton.isHidden
}
progressLabel.isHidden = progress == 0 && isIndeterminate ? cancelButton.isHidden : !cancelButton.isHidden

if !progressLabel.isHidden {
progressLabel.string = "\(Int(_progress * 100))%"
Expand All @@ -102,9 +108,6 @@ public final class CircularProgress: NSView {
if _progress == 1 {
isFinished = true
}

// TODO: Figure out why I need to flush here to get the label to update in `Gifski.app`.
CATransaction.flush()
}
}

Expand All @@ -114,13 +117,17 @@ public final class CircularProgress: NSView {
*/
@IBInspectable public private(set) var isFinished: Bool {
get {
assertMainThread()

if let progressInstance = progressInstance {
return progressInstance.isFinished
}

return _isFinished
}
set {
assertMainThread()

_isFinished = newValue

if _isFinished {
Expand All @@ -129,10 +136,7 @@ public final class CircularProgress: NSView {
if !isCancelled, showCheckmarkAtHundredPercent {
progressLabel.string = ""
cancelButton.isHidden = true

DispatchQueue.main.async {
self.successView.isHidden = false
}
successView.isHidden = false
}
}
}
Expand All @@ -143,33 +147,42 @@ public final class CircularProgress: NSView {
*/
public var progressInstance: Progress? {
didSet {
assertMainThread()

if let progressInstance = progressInstance {
progressObserver = progressInstance.observe(\.fractionCompleted) { sender, _ in
guard !self.isCancelled && !sender.isFinished else {
return
}
DispatchQueue.main.async {
guard !self.isCancelled && !sender.isFinished else {
return
}

self.progress = sender.fractionCompleted
self.progress = sender.fractionCompleted
}
}

finishedObserver = progressInstance.observe(\.isFinished) { sender, _ in
guard !self.isCancelled && sender.isFinished else {
return
}
DispatchQueue.main.async {
guard !self.isCancelled && sender.isFinished else {
return
}

self.progress = 1
self.progress = 1
}
}

cancelledObserver = progressInstance.observe(\.isCancelled) { sender, _ in
self.isCancelled = sender.isCancelled
DispatchQueue.main.async {
self.isCancelled = sender.isCancelled
}
}

indeterminateObserver = progressInstance.observe(\.isIndeterminate) { sender, _ in
self.isIndeterminate = sender.isIndeterminate
DispatchQueue.main.async {
self.isIndeterminate = sender.isIndeterminate
}
}

isCancellable = progressInstance.isCancellable

isIndeterminate = progressInstance.isIndeterminate
}
}
Expand Down Expand Up @@ -240,6 +253,8 @@ public final class CircularProgress: NSView {
Reset the progress back to zero without animating.
*/
public func resetProgress() {
assertMainThread()

alphaValue = 1

_color = originalColor
Expand All @@ -261,6 +276,8 @@ public final class CircularProgress: NSView {
Cancels `Progress` if it's set and prevents further updates.
*/
public func cancelProgress() {
assertMainThread()

guard isCancellable else {
return
}
Expand All @@ -284,13 +301,17 @@ public final class CircularProgress: NSView {
*/
@IBInspectable public var isCancellable: Bool {
get {
assertMainThread()

if let progressInstance = progressInstance {
return progressInstance.isCancellable
}

return _isCancellable
}
set {
assertMainThread()

_isCancellable = newValue
updateTrackingAreas()
}
Expand All @@ -302,13 +323,17 @@ public final class CircularProgress: NSView {
*/
@IBInspectable public private(set) var isCancelled: Bool {
get {
assertMainThread()

if let progressInstance = progressInstance {
return progressInstance.isCancelled
}

return _isCancelled
}
set {
assertMainThread()

_isCancelled = newValue

if newValue {
Expand Down Expand Up @@ -407,6 +432,8 @@ public final class CircularProgress: NSView {
return _isIndeterminate
}
set {
assertMainThread()

willChangeValue(for: \.isIndeterminate)
_isIndeterminate = newValue
didChangeValue(for: \.isIndeterminate)
Expand Down

0 comments on commit 20bc098

Please sign in to comment.