NOT FUNCTIONAL: `muda` context menu support #8803

npwoods · 2025-06-28T22:54:36Z

This represents the better part of a day of trying to implement native context menu support with muda. I've been able to get a context menu to appear, with the following caveats:

Only the Rust code generator is supported
Only Windows is supported
Menu activation is todo!()
Root menu items seem to display a subitem whether they have items or not

I've been finding it challenging navigating the Slint object model, particularly in the context of code generation, and I'm sure in some cases I'm overlooking simpler solutions. So I was hoping that I could get some feedback (perhaps what objects I need to "lock on to") about how to pull this over the finish line.

Thanks in advance, I'm really looking forward to having a native context menu in Slint!

This represents the better part of a day of trying to implement native context menu support with `muda`. I've been able to get a context menu to appear, with the following caveats: - Only the Rust code generator is supported - Only Windows is supported - Menu activation is `todo!()` - Root menu items seem to display a subitem whether they have items or not I've been finding it challenging navigating the Slint object model, particularly in the context of code generation, and I'm sure in some cases I'm overlooking simpler solutions. So I was hoping that I could get some feedback (perhaps what objects I need to "lock on to") about how to pull this over the finish line. Thanks in advance, I'm really looking forward to having a native context menu in Slint!

ogoffart

Thanks for the contribution!
Great work so far.

There are two kind of menus:
Simple menus that are entirely computed at compile time, or more complex menu (if let Expression::NumberLiteral(tree_index) = entries in the rust.rs code) which are a item tree.
Complex menu are used if if or for are being used within the menu.

To show a native menu, you will need, as you've already discovered , a VBox<MenuVTable> which is basically an equivalent of Box<dyn Menu>
This VBox/VTable dance is necessary because this might be generated by C++ code. and therefore everything need to be done with extern "C" and repr(C) type. Hence the use of the vtable trick.
I wisth at some point we could have "native" trait object in rust that can be shared with ffi.

To get the that VBox<MenuVTable> for complex menu, you can use MenuFromItemTree that there is already there:

slint/internal/compiler/generator/rust.rs

Line 2811 in ff0d450

    
           let context_menu_item_tree = sp::Rc::new(sp::MenuFromItemTree::new(sp::VRc::into_dyn(menu_item_tree_instance)));

but instead of putin it in a std::Rc, you can put it in a VBox. (in fact, to avoid alocations, we could consider VRc)
Currently this part is not in your patch.

The part you have in your patch (rust.rs) is for simple menu that have a generated activate and sub_menu. What you need to do is to have a wrapper like the one there:

slint/internal/compiler/generator/rust.rs

Line 3143 in ff0d450

    
           struct MenuBarWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>);

This should be a bit simpler than the ContextMenuFromItemTree you have currently

Only the Rust code generator is supported

I think it is fine to start small. Further generator can be handled later, as long as they don't break.

Only Windows is supported

Same here, mac can be done in a follow up. It just need to fallback correctly if not implemented.

I've been finding it challenging navigating the Slint object model, particularly in the context of code generation, and I'm sure in some cases I'm overlooking simpler solutions. So I was hoping that I could get some feedback (perhaps what objects I need to "lock on to") about how to pull this over the finish line.

I hope that the explanation above clarifies some of the things.
If not, please ask for questions for clarifications.

Thanks in advance, I'm really looking forward to having a native context menu in Slint!

And thanks for contributing

ogoffart · 2025-06-29T08:38:51Z

internal/core/window.rs

@@ -1150,6 +1157,17 @@ impl WindowInner {
        }
    }

+    /// Show a native context menu
+    pub fn show_context_menu(


there is already a stub function called show_native_popup_menu declared a bit down but that was never functional. I believe you could try to re-use that.

I think the function (also in the WindowAdapter) deserve to have native in the name to distinguish it from the function that show the slint based context menu.

ogoffart · 2025-06-29T08:45:17Z

internal/backends/winit/event_loop.rs

        }
    }
 }

+#[cfg(muda)]
+#[derive(Clone, Copy, Debug)]
+pub enum MudaType {


probably best to move that to muda.rs

ogoffart · 2025-06-29T08:53:39Z

internal/backends/winit/muda.rs

+        }
+        #[cfg(target_os = "macos")]
+        {
+            todo!(); // unsafe { menu.show_context_menu_for_nsview(nsview, position) };


if this function returned an option, you could return None there. And WindowAdatper::show_context_menu could return false to tell the runtime to fallback to the Slint based menu.

I suppose I was assuming (perhaps incorrectly) that the macOS version of the call was a relatively simple call, and that it in practice it might be easier to just identify the right call than it would be to build a bunch of scaffolding?

nvm - saw your next comment

ogoffart · 2025-06-29T08:59:13Z

internal/compiler/generator/rust.rs

+            let native_impl = if *no_native {
+                quote!()
+            } else {
+                quote!(if sp::WindowInner::from_pub(#window_adapter_tokens.window()).supports_native_menu_bar() {


re-using the supports_native_menu_bar for the context menu might not be needed. I can imagine platform having a native context menu but no native menubar or the opposite.
I'd prefer if the show_context_menu would return false for a fallback.

ogoffart · 2025-06-29T09:05:15Z

internal/compiler/passes/lower_menus.rs

@@ -303,6 +306,7 @@ fn process_context_menu(
                index: 0,
                ty: crate::typeregister::logical_point_type(),
            },
+            Expression::BoolLiteral(no_native_menu),


The no_native_menu was done for the live preview because we don't want to replace the live previews's menubar by the previewed menubar.
I think it's fine, and even desirable, if the live preview show the native context menu.

ogoffart · 2025-06-29T15:28:10Z

I added some comments there that I hope would help 73dc1c7

- Removed the `show_context_menu()` and started using the existing `show_native_popup_menu()` stub. Changed (probably erroneously) the type of the context menu parameter to what was on original change. - Created a `ContextMenuWrapper` for the case where have the generated `activate()` and `sub_menu()` calls (this is incomplete) - Removed the `no_native_menu` infrastructure that I cargo culted.

npwoods · 2025-06-29T21:11:03Z

Thanks for the pointers - I've made the following changes:

I got rid of my show_context_menu() and started using your show_native_popup_menu() stub. I did change the type of the context menu parameter to what I have. I strongly suspect that this is erroneous but I'm trying to be selective with the changes I make right now.
I created a ContextMenuWrapper for the case where have the generated activate() and sub_menu() calls. But as you can tell from my comments in the code, I didn't quite figure out how to plumb things through correctly.
I removed the no_native_menu infrastructure that I cargo culted.

I did not move MudaType just yet.

ogoffart · 2025-06-30T07:52:08Z

internal/compiler/generator/rust.rs

+                                    None => #entries,
+                                    Some(parent) =>  {
+                                        // PROBLEM - how do I wrie this up to the submenu?
+                                        // #access_sub_menu.call(&(parent.clone(),));


I think that line should work. Do you get a comilation error there? Same for activate.

When uncommenting that on sub_menu() I get:

error[E0308]: mismatched types | 3946 | ... ) . apply_pin (_self) . call (& (parent . clone () ,)) } | --------- ^^^^^ expected `Pin<&InnerPopupMenuImpl_root_26>`, found `Pin<&InnerMainWindow>` | | | arguments to this method are incorrect | = note: expected struct `Pin<&InnerPopupMenuImpl_root_26>` found struct `Pin<&InnerMainWindow>` note: method defined here --> C:\Users\Nate\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\field-offset-0.3.6\src\lib.rs:264:12 | 264 | pub fn apply_pin<'a>(self, x: Pin<&'a T>) -> Pin<&'a U> { | ^^^^^^^^^ For more information about this error, try `rustc --explain E0308`.

I get a virtually identical error when uncommenting that on activate():

error[E0308]: mismatched types | 3959 | ... ) . apply_pin (_self) . call (& (entry . clone () ,)) } | --------- ^^^^^ expected `Pin<&InnerPopupMenuImpl_root_26>`, found `Pin<&InnerMainWindow>` | | | arguments to this method are incorrect | = note: expected struct `Pin<&InnerPopupMenuImpl_root_26>` found struct `Pin<&InnerMainWindow>` note: method defined here --> C:\Users\Nate\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\field-offset-0.3.6\src\lib.rs:264:12 | 264 | pub fn apply_pin<'a>(self, x: Pin<&'a T>) -> Pin<&'a U> { | ^^^^^^^^^ For more information about this error, try `rustc --explain E0308`.

I assume there is a straight forward way to walk the object hierarchy to get what we want?

I see the problem, access_sub_menu / access_activated are in the context of the menu.
So the change i suggest might help.

ogoffart · 2025-06-30T12:32:56Z

internal/compiler/generator/rust.rs

                let fw_activated = forward_callback(access_activated, quote!(activated));
                quote! {
+                    let context_menu_item_tree = if sp::WindowInner::from_pub(#window_adapter_tokens.window()).supports_native_menu_bar() {
+                        // May seem overkill to have an instance of the struct for each call, but there should only be one call per component anyway
+                        struct ContextMenuWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>);


Suggested change

struct ContextMenuWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>);

struct ContextMenuWrapper(sp::VWeak<sp::ItemTreeVTable, #popup_id>);

ogoffart · 2025-06-30T12:33:58Z

internal/compiler/generator/rust.rs

+                                    None => #entries,
+                                    Some(parent) =>  {
+                                        // PROBLEM - how do I wrie this up to the submenu?
+                                        // #access_sub_menu.call(&(parent.clone(),));


I see the problem, access_sub_menu / access_activated are in the context of the menu.
So the change i suggest might help.

ogoffart · 2025-06-30T12:36:18Z

internal/compiler/generator/rust.rs

+                                todo!()
+                            }
+                        }
+                        Some(sp::VBox::new(ContextMenuWrapper(_self.self_weak.get().unwrap().clone())))


Suggested change

Some(sp::VBox::new(ContextMenuWrapper(_self.self_weak.get().unwrap().clone())))

Some(sp::VBox::new(ContextMenuWrapper(sp::VRc::downgrade(&popup_instance))))

I tried to do something like that; it wasn't clear what I need to change sp::ItemTreeVTable to in this context:

struct ContextMenuWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>);

- Root menu items seem to be empty submenus, and are this not usable - Root separators seem to be blank menu items - `ContextMenuWrapper` works by virtue of passing `Box<dyn ...>` callbacks to them. This is clearly a hack to get around the challenges of using the "quote macros" from within `impl ContextMenuWrapper` This is clearly not a complete and desirable solution (yet)

npwoods · 2025-07-02T09:31:28Z

Just pushed a few (marginal) improvements; details in the commit

npwoods · 2025-07-02T11:31:05Z

Screenshot of test program showing this, and accompanying Slint code

import { Button, HorizontalBox } from "std-widgets.slint";

export component MainWindow inherits Window {
    width: 300px;
    height: 200px;
    in-out property <string> my-text;
    Text {
        text: my-text;
    }

    ContextMenuArea {
        Menu {
            MenuItem {
                title: @tr("Cut");
                activated => {
                    my-text = "Cut";
                }
            }

            MenuItem {
                title: @tr("Copy");
                activated => {
                    my-text = "Copy";
                }
            }

            MenuItem {
                title: @tr("Paste");
                activated => {
                    my-text = "Paste";
                }
            }

            MenuSeparator { }

            Menu {
                title: @tr("Find");
                MenuItem {
                    title: @tr("Find Next");
                    activated => {
                        my-text = "Find Next";
                    }
                }

                MenuItem {
                    title: @tr("Find Previous");
                    activated => {
                        my-text = "Find Previous";
                    }
                }
            }
        }
    }
}

ogoffart · 2025-07-02T17:37:32Z

I think the reason why the root is always a sub-menu is because of the depth!=0 in this line:

slint/internal/backends/winit/muda.rs

Lines 90 to 91 in 0a25de3

    
           } else if !entry.has_sub_menu && depth != 0 { 
        
               // the top level always has a sub menu regardless of entry.has_sub_menu

I think this was needed for the menu bar in the past, but maybe now this comment is obsolete and the depth!=0 can be removed.

ogoffart · 2025-07-02T17:39:17Z

internal/compiler/generator/rust.rs

+                        // and access_activated from within ContextMenuWrapper
+                        type PopupAccessSubMenu = Box::<dyn Fn(&sp::MenuEntry) -> sp::ModelRc<sp::MenuEntry> + 'static>;
+                        type PopupActivate = Box::<dyn Fn(&sp::MenuEntry) + 'static>;
+                        struct ContextMenuWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>, PopupAccessSubMenu, PopupActivate);


did my suggestion to use the #popup_id instead of #inner_component_id not work?

- Removed awkward Box<dyn> usage - Using sp::VRc<sp::ItemTreeVTable> instead of recommended sp::VWeakMapped<sp::ItemTreeVTable> because the weak reference does not live enough to be activated Outstanding issues: - Root items are incorrectly all sub menus - No longer needed ContextMenuItemTree needs to be removed - MudaType needs to be moved

npwoods · 2025-07-03T00:21:03Z

I missed that part (doh!).

I was able to get your suggestion to work, only with the catch that I couldn't use the weak reference because it didn't survive long enough for the context menu to be activated.

Outstanding issues:

Root items are incorrectly all sub menus (not sure what is happening here?)
No longer needed ContextMenuFromItemTree needs to be removed
MudaType needs to be moved

npwoods · 2025-07-03T00:42:37Z

I think the reason why the root is always a sub-menu is because of the depth!=0 in this line:

Seemed to do the trick!

ogoffart · 2025-07-03T06:09:51Z

internal/compiler/generator/rust.rs

+                    let parent_weak = _self.self_weak.get().unwrap().clone();
+                    #init_popup
+
+                    if !sp::WindowInner::from_pub(#window_adapter_tokens.window()).show_native_popup_menu(context_menu_item_tree.unwrap(), #position) {


the .unwrap() here seems bad, as it would panic when supports_native_menu_bar returns false.

ogoffart · 2025-07-03T06:12:57Z

internal/compiler/generator/rust.rs

                    {
                        let _self = popup_instance_vrc.as_pin_ref();
-                        #access_entries.set(entries);
+                        #access_entries.set(entries.clone());


why do we need to clone here?

ogoffart · 2025-07-03T06:14:51Z

internal/compiler/generator/rust.rs

+                    else {
+                        None
+                    };
+
                    {


I think we only need this part when not using the native context menu (fallback)

ogoffart · 2025-07-03T06:20:19Z

internal/compiler/generator/rust.rs

+                        #context_menu.popup_id.set(Some(id));
+                        #popup_id::user_init(popup_instance_vrc);
+                    }
+                }
            })
        }


note that you only take care of the else branch of the if that ends here. But the other branch is for when the menu contains if _: MenuItem{} or for _ in _ : MenuItem{}

There, the context_menu_item_tree is in a Rc instead of a VBox. It might be worth putting them in a VRc in both branches.

- Removed `ContextMenuFromItemTree` - Moved `MudaType` to `internal/backends/winit/muda.rs` - Stopped unnecessary use of `Option` when creating `context_menu_item_tree`

npwoods · 2025-07-03T23:29:21Z

Committed a few more changes!

Regarding this feedback:

There, the context_menu_item_tree is in a Rc instead of a VBox. It might be worth putting them in a VRc in both branches.

context_menu_item_tree is now an Rc on the first path and a VBox on the second path. Changing the latter to VRc::new() causes a compilation error about VTableMetaDropInPlace to appear; not sure what to make of this. That said, in this generated code it is really a problem if they are different types if the code is compatible?

Separately I also tried to investigate why the separator appears as a blank entry. Despite the Slint entry being a MenuSeparator, MenuEntry::is_separator appears to be false.

ogoffart · 2025-07-04T07:01:26Z

it is really a problem if they are different types if the code is compatible?

If the code is compatible then it's not a problem. But since show_native_popup_menu takes a VBox, it might not be compatible.

error about VTableMetaDropInPlace

Right, VRc would need extra function in the MenuVTable.

…VBox<MenuVTable> -- Problem: is using `vtable::VRc::borrow(&menubar)` the correct approach? -- Problem: `VTableMetaDropInPlace` is not properly implemented for `MenuVTable` -- Problem: Codegen for menus has not been updated yet - Updated Context Menu code to use `VRc` so both sides are supported

npwoods · 2025-07-04T11:35:19Z

Pushed more changes; got both sides supported by using VRc<ItemVTable> as per your suggestions. This is still not complete because it wasn't clear to me if the surgery required to make everything in internal/backends/winit/muda.rs is correct.

Right, VRc would need extra function in the MenuVTable

I didn't figure out how to do this, so I created a bogus implementation of VTableMetaDropInPlace

ogoffart · 2025-07-04T11:59:42Z

I didn't figure out how to do this, so I created a bogus implementation of VTableMetaDropInPlace

You need to add a dealloc and drop_in_place function in the MenuVTable struct.
See https://docs.rs/vtable/latest/vtable/attr.vtable.html#:~:text=drop_in_place
(Yes, that documentation is a bit weak)

npwoods · 2025-07-04T12:30:58Z

I fixed the MenuVTable struct and changed the menubar codegen to use VRc.

To my knowledge there are two outstanding issues:

C++ support (I have no idea how any of this works)
macOS support; I suppose that we could just return false, but presumably this is easy?

npwoods · 2025-07-10T11:14:15Z

I'm curious what the next steps are?

ogoffart · 2025-07-11T07:27:41Z

The Next step is to get the CI to pass so we can merge this.

The current error is (while compiling the slint-interpreter crate)

error[E0308]: mismatched types
    --> internal/interpreter/eval.rs:705:79
     |
705  |             if component.access_window(|window| window.show_native_popup_menu(&item_rc, position)) {
     |                                                        ---------------------- ^^^^^^^^ expected `VRc<MenuVTable>`, found `&ItemRc`
     |                                                        |
     |                                                        arguments to this method are incorrect
     |
     = note: expected struct `VRc<MenuVTable>`
             found reference `&ItemRc`

That was just a stub in eval.rs and that code can be removed (then it still won't use the native context menu in the live preview), or adapted to your change.
There might be a bit more adjustment to do in the C++ codegen so everything can pass.

If that's too much to ask, I can also take this over this change, but it'll take some time as I'll be busy/vacations the next weeks.

Feel free to ask any questions otherwise and thanks a lot for your contributions.

npwoods · 2025-07-12T15:17:29Z

I fixed the compilation problems on the internal/interpreter crate.

The internal/core crate has a compilation error when the ffi feature is active; it isn't clear to me how to fix this code to use VRc:

    /// Setup the native menu bar
    #[unsafe(no_mangle)]
    pub unsafe extern "C" fn slint_windowrc_setup_native_menu_bar(
        handle: *const WindowAdapterRcOpaque,
        vtable: NonNull<MenuVTable>,
        menu_instance: NonNull<c_void>,
    ) {
        let window_adapter = &*(handle as *const Rc<dyn WindowAdapter>);
        window_adapter
            .internal(crate::InternalToken)
            .map(|x| x.setup_menubar(vtable::VBox::from_raw(vtable, menu_instance.cast())));
    }

I'm very inclined to leave the C++ codegen to the pros... enjoy your vacation!

ogoffart reviewed Jun 29, 2025

View reviewed changes

ogoffart reviewed Jun 30, 2025

View reviewed changes

ogoffart reviewed Jul 2, 2025

View reviewed changes

Fixing issue that caused root items to be submenus

7a9507f

ogoffart reviewed Jul 3, 2025

View reviewed changes

Various improvements:

7a6a588

- Removed `ContextMenuFromItemTree` - Moved `MudaType` to `internal/backends/winit/muda.rs` - Stopped unnecessary use of `Option` when creating `context_menu_item_tree`

[autofix.ci] apply automated fixes

41f9bf6

npwoods added 2 commits July 4, 2025 08:19

Proper implementation of VTableMetaDropInPlace for MenuVTable

12cf3d0

Fixing menu bar code

11300e3

The internal/interpreter crate now compiles

086c867

	struct ContextMenuWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>);
	struct ContextMenuWrapper(sp::VWeak<sp::ItemTreeVTable, #popup_id>);

	Some(sp::VBox::new(ContextMenuWrapper(_self.self_weak.get().unwrap().clone())))
	Some(sp::VBox::new(ContextMenuWrapper(sp::VRc::downgrade(&popup_instance))))

+                                  else {
+                                      None
+                                  };
                                   {

NOT FUNCTIONAL: muda context menu support #8803

Are you sure you want to change the base?

NOT FUNCTIONAL: muda context menu support #8803

Uh oh!

Conversation

npwoods commented Jun 28, 2025

Uh oh!

ogoffart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogoffart commented Jun 29, 2025

Uh oh!

npwoods commented Jun 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

npwoods commented Jul 2, 2025

Uh oh!

npwoods commented Jul 2, 2025

Uh oh!

ogoffart commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

npwoods commented Jul 3, 2025

Uh oh!

npwoods commented Jul 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

npwoods commented Jul 3, 2025

Uh oh!

ogoffart commented Jul 4, 2025

Uh oh!

npwoods commented Jul 4, 2025

Uh oh!

ogoffart commented Jul 4, 2025

Uh oh!

npwoods commented Jul 4, 2025

Uh oh!

npwoods commented Jul 10, 2025

Uh oh!

ogoffart commented Jul 11, 2025

Uh oh!

npwoods commented Jul 12, 2025

Uh oh!

Uh oh!

NOT FUNCTIONAL: `muda` context menu support #8803

NOT FUNCTIONAL: `muda` context menu support #8803

ogoffart commented Jul 2, 2025 •

edited

Loading