Skip to content

Commit

Permalink
Fix deadlock
Browse files Browse the repository at this point in the history
Use Combine instead of notifications to transfer "needs update" information. This avoids a deadlock because notifications were delivered synchronously (which is not needed in this case).
  • Loading branch information
cochrane committed Mar 15, 2023
1 parent 74da60f commit 8be0215
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 16 deletions.
2 changes: 1 addition & 1 deletion GLLara/GLLItemDrawer.swift
Expand Up @@ -80,7 +80,7 @@ class GLLItemDrawer {
}

func propertiesChanged() {
sceneDrawer?.notifyRedraw()
sceneDrawer?.needsUpdate = true
}

private func permutationTableColumn(for assignment: GLLItemChannelAssignment) -> vector_float4 {
Expand Down
2 changes: 0 additions & 2 deletions GLLara/GLLNotifications.h
Expand Up @@ -8,8 +8,6 @@

#include <Foundation/Foundation.h>

extern NSString *GLLSceneDrawerNeedsUpdateNotification;

// Sent when bound textures or similar change outside of normal execution, to
// indicate that the draw state needs to be reset for the next frame.
extern NSString *GLLDrawStateChangedNotification;
3 changes: 0 additions & 3 deletions GLLara/GLLNotifications.m
Expand Up @@ -8,7 +8,4 @@

#import <Foundation/Foundation.h>


NSString *GLLSceneDrawerNeedsUpdateNotification = @"GLLSceneDrawerNeedsUpdateNotification";

NSString *GLLDrawStateChangedNotification = @"GLLDrawStateChangedNotification";
13 changes: 6 additions & 7 deletions GLLara/GLLSceneDrawer.swift
Expand Up @@ -8,8 +8,9 @@

import Cocoa
import CoreData
import Combine

@objc class GLLSceneDrawer: NSObject {
@objc class GLLSceneDrawer: NSObject, ObservableObject {

@objc init(document: GLLDocument) {
self.document = document
Expand Down Expand Up @@ -40,11 +41,11 @@ import CoreData
}
}
}
self.notifyRedraw()
self.needsUpdate = true
}

drawStateNotificationObserver = NotificationCenter.default.addObserver(forName: Notification.Name.GLLDrawStateChanged, object: nil, queue: OperationQueue.main) { [weak self] notification in
self?.notifyRedraw()
self?.needsUpdate = true
}

// Load existing items
Expand Down Expand Up @@ -78,7 +79,7 @@ import CoreData
@objc var selectedBones: [GLLItemBone] {
set {
skeletonDrawer.selectedBones = newValue
notifyRedraw()
needsUpdate = true
}
get {
return skeletonDrawer.selectedBones
Expand All @@ -95,9 +96,7 @@ import CoreData
skeletonDrawer.draw(into: commandEncoder)
}

func notifyRedraw() {
NotificationCenter.default.post(name: NSNotification.Name.GLLSceneDrawerNeedsUpdate, object: self)
}
@Published var needsUpdate = false

private var itemDrawers: [GLLItemDrawer] = []
private let skeletonDrawer: GLLSkeletonDrawer
Expand Down
11 changes: 8 additions & 3 deletions GLLara/GLLView.swift
Expand Up @@ -9,6 +9,7 @@
import Cocoa
import MetalKit
import GameController
import Combine

@objc class GLLView: MTKView {

Expand All @@ -27,9 +28,6 @@ import GameController
notificationObservers.append(NotificationCenter.default.addObserver(forName: UserDefaults.didChangeNotification, object: nil, queue: OperationQueue.main) { [weak self] notification in
self?.showSelection = UserDefaults.standard.bool(forKey: GLLPrefShowSkeleton)
})
notificationObservers.append(NotificationCenter.default.addObserver(forName: NSNotification.Name.GLLSceneDrawerNeedsUpdate, object: nil, queue: OperationQueue.main) { [weak self] notification in
self?.unpause()
})
notificationObservers.append(NotificationCenter.default.addObserver(forName: NSNotification.Name.GCControllerDidBecomeCurrent, object: nil, queue: OperationQueue.main) { [weak self] notification in
self?.unpause()
})
Expand Down Expand Up @@ -57,13 +55,20 @@ import GameController

@objc func set(camera: GLLCamera, sceneDrawer: GLLSceneDrawer) {
self.camera = camera
sceneDrawerUpdateRegistration?.cancel()
self.sceneDrawer = sceneDrawer
sceneDrawerUpdateRegistration = sceneDrawer.$needsUpdate.sink { [weak self] value in
if value {
self?.unpause()
}
}
self.viewDrawer = GLLViewDrawer(sceneDrawer: sceneDrawer, camera: camera, view: self)
}

var camera: GLLCamera? = nil
@objc weak var document: GLLDocument? = nil
var sceneDrawer: GLLSceneDrawer? = nil
var sceneDrawerUpdateRegistration: AnyCancellable? = nil
@objc var viewDrawer: GLLViewDrawer? = nil
var showSelection: Bool = false
var keysDown = CharacterSet()
Expand Down
2 changes: 2 additions & 0 deletions GLLara/GLLViewDrawer.swift
Expand Up @@ -290,6 +290,8 @@ import UniformTypeIdentifiers

private func draw(commandBuffer: MTLCommandBuffer, viewRenderPassDescriptor: MTLRenderPassDescriptor, surface: Surface, includeUI: Bool = true, screenScale: Double = 2.0) {

sceneDrawer.needsUpdate = false

var viewProjection = camera.viewProjectionMatrix(forAspectRatio: Float(surface.width) / Float(surface.height))

if needsUpdateLights {
Expand Down

0 comments on commit 8be0215

Please sign in to comment.