fix: unfreeze the export paths, repair shader error logging, validate the UI - #5
Merged
Merged
Conversation
…rdown - The three export actions waited for a frame with an unbounded 'while (!depth)' spin. With the device stopped or absent - the menu items stay enabled then - _depthUpdate never turns YES, so this pinned a core on the main thread and froze the UI for good. Replaced by -waitForDepthDataUntil:, which gives up after two seconds and reports it in the status field. - GLProgram logged shader compile failures with NSLog(@"...%s", desc, code) where desc is an NSString*. %s read the object pointer as a C string: garbage or a crash on the one path meant to report shader errors. code was never printed. - -[GLView dealloc] released the display link without stopping it while its callback still draws into the view, then let closeScene issue glDelete* calls with no current context. - Mark _device and _halt volatile. Each is written on one thread and polled on the other with no barrier; the polls only work because they call opaque functions that force a reload. The race itself is unchanged. - Stream mesh indices through an element buffer object rather than a client-side array copied inline at draw time, unbinding before drawFrustrum, which still draws from a client array and would otherwise read its pointer as an offset. Verified with clang -fsyntax-only -Wall against the current macOS SDK: no errors, and GLProgram is now warning-clean. The remaining warnings are documented in the README as deliberately untouched. Still not built or run against Kinect hardware.
The export menu items were never validated, so they stayed clickable with no device running - the one state in which they had nothing to write and used to hang. Add -validateMenuItem: and -validateUserInterfaceItem: so they gray out. AppKit validates menu and toolbar items only; plain buttons in the nib still need their enabled state bound there. Replace AppKit calls that no longer exist or are deprecated: - runModalForDirectory:file: -> setDirectoryURL: + runModal - NSOKButton -> NSModalResponseOK - NSShiftKeyMask -> NSEventModifierFlagShift - NSString writeToFile:atomically: -> writeToFile:atomically:encoding:error: This raises the effective deployment target well above the 10.6 the project declares. It is a step toward building against a current SDK, not something Xcode 3 will still compile. setAllowedFileTypes: is left alone: allowedContentTypes needs the UniformTypeIdentifiers framework wired into the project file. clang -fsyntax-only -Wall against the current SDK: no errors, and no new warnings beyond the ones already documented as deliberately untouched.
Says what is true about the SDK rather than naming the IDE: the modernised AppKit calls will not compile against the 10.6 SDK the project declares, and the UniformTypeIdentifiers note refers to the project file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #2, which GitHub closed automatically when its base branch
perf/render-hot-pathswas deleted on merge of #1. Same commits, rebased ontomain.What this changes
Fixes a permanent UI freeze, a crash on the shader error path, unsound teardown, and the missing interface validation that made the freeze reachable.
Why
The freeze. All three export actions waited for a frame like this:
No exit. With the device stopped or absent,
_depthUpdatenever turnsYES, so this pinned a core on the main thread and froze the UI for good — force quit only. Nothing grayed the export menu items out, so they stayed clickable in exactly the state where they could not work. Now a bounded wait (-waitForDepthDataUntil:) that gives up after two seconds and says so in the status field, plus-validateMenuItem:/-validateUserInterfaceItem:so the actions gray out. AppKit validates menu and toolbar items only — plain buttons in the nib still need theirenabledstate bound there.Shader errors.
GLProgram.mlogged compile failures withNSLog(@"...%s", desc, code)wheredescis anNSString *.%sread the object pointer as a C string: garbage or a crash, on the single path whose job is to report shader errors.codewas passed and never printed. Surfaced by-Wall.Teardown.
-[GLView dealloc]released the display link without stopping it first, while its callback still draws into the view, then letcloseSceneissueglDelete*calls without making the view's context current.Threading.
_deviceand_haltare each written on one thread and polled on the other with no barrier. Both are nowvolatile. This does not fix the race — it makes explicit what the polls already rely on. A real fix wants a condition variable.Deprecated AppKit.
runModalForDirectory:file:→setDirectoryURL:+runModal,NSOKButton→NSModalResponseOK,NSShiftKeyMask→NSEventModifierFlagShift,writeToFile:atomically:→ theencoding:error:form. These will not compile against the 10.6 SDK the project declares.Also: mesh indices now stream through an element buffer object instead of a client-side array copied inline at draw time, with an unbind before
drawFrustrum, which still draws from a client array.How it was verified
sh scripts/syntax-check.shpasses;GLProgram.mis now warning-cleanNone of this is backed by observed behaviour — it is reasoning over the code plus what
-Wallreports. The freeze, the%scrash and the teardown ordering are each clear from the source, but the export paths and the mesh should be confirmed on hardware.Left alone deliberately
saveSTLBmixes&&and||without parentheses (-Wlogical-op-parentheses). The precedence applied may not be what the author meant; parenthesising it would freeze one reading.setAllowedFileTypes:needs UniformTypeIdentifiers wired into the project file.Provenance
CONTRIBorAUTHORSfile altered