Repository navigation
Modernize Window/CvTrackbar; delete DisposableObject (closes #1941) - #1972
Merged
Merged
Conversation
Final slice of #1941. Migrates the last two DisposableObject subclasses off it and removes the base class entirely. Centralized callback lifetime (also fixes a latent crash): - OpenCV highgui stores raw function pointers per window name and calls them later, so the managed delegates must outlive the call. Cv2.SetMouseCallback / Cv2.CreateTrackbar previously passed the delegate straight to native without rooting it, so the GC could collect a delegate OpenCV still invoked. Add a name-keyed registry in Cv2 that roots mouse/trackbar delegates and clears them on DestroyWindow / DestroyAllWindows. Window: - now `sealed class : IDisposable` (off DisposableObject), no finalizer. - delete the dead MouseCallback property + ScopedGCHandle machinery (its setter was never called) and the NotImplemented DisplayOverlay/DisplayStatusBar stubs. - Dispose routes through Cv2.DestroyWindow, which also forgets the callbacks. CvTrackbar: - now a plain `sealed class` (no IDisposable): it held two GCHandles purely to keep the callback delegates alive, which the Cv2 registry now does. It routes creation through Cv2.CreateTrackbar; no GCHandle, no GetFunctionPointerForDelegate. Cleanup: - delete DisposableObject (no remaining subclasses) and the now-unused ScopedGCHandle helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
@
Final slice of #1941 (part of #1938). Stacked on #1970 (review/merge order: #1968 → #1969 → #1970 → this). Migrates the last two
DisposableObjectsubclasses off it and removesDisposableObjectentirely — the goal of #1941.Decision (with @shimat): keep the OO
Window/CvTrackbarAPI (it has value as an object form of OpenCVs name-keyed highgui), but modernize it. OpenCV 5s highgui design is unchanged (name-basednamedWindow/imshow/setMouseCallback/createTrackbar, withMouseCallback/TrackbarCallbacktypedefs), so the wrappers premise is stable.Centralized callback lifetime (also fixes a latent crash)
OpenCV highgui stores raw function pointers per window name and invokes them later from its own UI loop, so the managed delegates must stay alive until the window is destroyed.
Cv2.SetMouseCallback/Cv2.CreateTrackbarpreviously passed the delegate straight to native without rooting it — the GC could collect a delegate OpenCV still calls → crash.Added a name-keyed registry in
Cv2that roots mouse/trackbar delegates and clears them onDestroyWindow/DestroyAllWindows. This mirrors OpenCVs own name-keyed, process-global model and is now the single place these delegates are rooted.Window
sealed class : IDisposable(offDisposableObject); no finalizer.MouseCallbackproperty +ScopedGCHandlemachinery (its setter was never called anywhere) and theNotImplementedExceptionDisplayOverlay/DisplayStatusBarstubs.Disposeroutes throughCv2.DestroyWindow, which also forgets the windows callbacks.CvTrackbar
sealed class(noIDisposable). It previously held twoGCHandles solely to keep the callback delegates alive — theCv2registry now does that. Creation routes throughCv2.CreateTrackbar; noGCHandle, noGetFunctionPointerForDelegate.Cleanup
DisposableObject(no remaining subclasses) and the now-unusedScopedGCHandlehelper.Notes
Window.GetWindowByNameand thestatic Dictionary<string, Window>registry are kept as-is (still public API). They remain a minor global-state smell but are out of scope here; can be revisited separately.@