Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow freeing NSView based on parent window closing notifications. #137

Closed
wants to merge 1 commit into from
Closed

Allow freeing NSView based on parent window closing notifications. #137

wants to merge 1 commit into from

Conversation

kunalarya
Copy link

In order to use third party crates that, under the hood, may increase the ref count of the given NSView, we add an additional "window closing" check by querying the given NSView for its NSWindow parent, and subscribing to the NSWindowWillCloseNotification message.

Note that in DAWs such as Reaper, we share a parent window with other plugins, so this approach is not reliable (since you can switch between different plugins but the window persists). Those cases may require another solution.

(Mostly) fixes #124

@glowcoil
Copy link
Member

glowcoil commented Jan 3, 2023 •
edited

I am not really a fan of heuristic solutions like this. As you say, in situations like in Reaper where the NSWindow outlives Baseview's NSView, this notification won't occur; and, in the reverse situation, where the host closes the containing NSWindow but retains a reference to Baseview's NSView and then reparents it to another window, this notification will result in premature destruction of the Baseview window.

There was a discussion on the Rust Audio Discord several months ago about changing Baseview's API contract so that windows may only be closed via an explicit call to WindowHandle::close() or Window::close(), which would resolve the refcount cycle issue in a more principled way. I'll write that up in more detail on #124.

@kunalarya kunalarya mentioned this pull request Jan 7, 2023
Open
@kunalarya kunalarya closed this Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

macos: Support user-created wgpu surfaces.
2 participants
@kunalarya @glowcoil