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.

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

[Question] NSMenuItem setTarget trouble #585

Closed
VisualEhrmanntraut opened this issue Feb 26, 2024 · 3 comments
Closed

[Question] NSMenuItem setTarget trouble #585

VisualEhrmanntraut opened this issue Feb 26, 2024 · 3 comments
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates question Further information is requested

Comments

@VisualEhrmanntraut
Copy link

Hello. I am migrating my project to use objc2 from objc, and I am encountering an issue. I am building a menu bar on top of an egui/winit app, and after switching to objc2, the menu bar options are greyed out. This suggests that the internal check for whether the action exists failed. I'm not sure if there's a problem with the crate or if it's my oversight (the documentation here is sparse, and my understanding of obj-c isn't that great).
Here's the relevant parts of the code

#[cfg(target_os = "macos")]
static OPENING_FILE: std::sync::Mutex<bool> = std::sync::Mutex::new(false);

#[cfg(target_os = "macos")]
static SAVING_FILE: std::sync::Mutex<bool> = std::sync::Mutex::new(false);

#[cfg(target_os = "macos")]
declare_class!(
    struct PlistOxideMenu;

    unsafe impl ClassType for PlistOxideMenu {
        type Super = NSObject;
        type Mutability = MainThreadOnly;
        const NAME: &'static str = "PlistOxideMenu";
    }

    impl DeclaredClass for PlistOxideMenu {}

    unsafe impl NSObjectProtocol for PlistOxideMenu {}

    unsafe impl PlistOxideMenu {
        #[method(openingFile)]
        unsafe fn opening_file(&self) {
            *OPENING_FILE.lock().unwrap() = true;
            (*EGUI_CTX.get()).assume_init_mut().request_repaint();
        }

        #[method(savingFile)]
        unsafe fn saving_file(&self) {
            *SAVING_FILE.lock().unwrap() = true;
            (*EGUI_CTX.get()).assume_init_mut().request_repaint();
        }
    }
);

// [...]

impl PlistOxide {
	// [...]

    #[cfg(target_os = "macos")]
    unsafe fn init_global_menu(cc: &eframe::CreationContext<'_>) {
        (*EGUI_CTX.get()).write(cc.egui_ctx.clone());
        let mtm = MainThreadMarker::new().unwrap();
        let file_menu = NSMenu::initWithTitle(mtm.alloc(), ns_string!("File"));

        let target: Id<PlistOxideMenu> = unsafe { msg_send_id![mtm.alloc(), init] };

        let file_open = NSMenuItem::initWithTitle_action_keyEquivalent(
            mtm.alloc(),
            ns_string!("Open..."),
            Some(sel!(openingFile)),
            ns_string!("o"),
        );
        file_open.setTarget(Some(&target));
        file_menu.addItem(&file_open);

        file_menu.addItem(&NSMenuItem::separatorItem(mtm));

        let file_save = NSMenuItem::initWithTitle_action_keyEquivalent(
            mtm.alloc(),
            ns_string!("Save..."),
            Some(sel!(savingFile)),
            ns_string!("s"),
        );
        file_save.setTarget(Some(&target));
        file_menu.addItem(&file_save);

        let file_item = NSMenuItem::new(mtm);
        file_item.setSubmenu(Some(&file_menu));
        NSApplication::sharedApplication(mtm)
            .mainMenu()
            .unwrap()
            .addItem(&file_item);
    }

    #[must_use]
    pub fn new(cc: &eframe::CreationContext<'_>, path: Option<PathBuf>) -> Self {
        #[cfg(target_os = "macos")]
        unsafe {
            Self::init_global_menu(cc);
        }
        // [...]
    }
    
    // [...]
}
@madsmtm
Copy link
Owner

madsmtm commented Feb 26, 2024

If you take a look at the -[NSMenuItem target] property, you will see that it says "weak". This means that when you call file_open.setTarget and file_save.setTarget, the menu items don't prevent the target value from being destroyed and deallocated when it goes out of scope.

The reason this worked in objc was likely due to a memory leak somewhere in your application, objc2 avoids those, but in turn you must handle stuff like this correctly.

If there is a natural place that you can store the target at the end of the scope, I'd recommend you do that, otherwise it's probably fine to leak it manually using std::mem::forget(target).

the documentation here is sparse

There's a bit more documentation on menus in here, but yeah, it definitely is limited.

@madsmtm madsmtm added question Further information is requested A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates labels Feb 26, 2024
@VisualEhrmanntraut VisualEhrmanntraut changed the title Problem with selectors in objc2 vs objc [Question] NSMenuItem setTarget trouble Feb 27, 2024
@VisualEhrmanntraut
Copy link
Author

Interesting how it can tell that it got deallocated instead of crashing. Calling forget on it seems to work, thanks.

@madsmtm
Copy link
Owner

madsmtm commented Feb 27, 2024

It's kinda like target is an std::sync::Arc<PlistOxideMenu>, and NSMenuItem stores a std::sync::Weak to that, though indeed this is something that we don't often use in the Rust world.

madsmtm added a commit that referenced this issue Sep 23, 2024
We also link to this documentation from the property setters themselves.

This is a common point of confusion, see:
- #585
- #606
- https://matrix.to/#/!SrJvHgAPHenBakQHSz:matrix.org/$OsgLE1-VtAZ5f383z_UoEpDyS7xDUk67xveA_zWvyZA?via=matrix.org
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants