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

Missing (ig)UpdateHoveredWindowAndCaptureFlags in non-docking bindings #706

Open
MarijnS95 opened this issue Jan 18, 2023 · 5 comments
Open

Comments

@MarijnS95
Copy link
Contributor

The bindgen header for the "docking" feature has it, but the master branch does not.

I attempted manually rerunning cimgui, but besides getting an inordinate amount of API breakage (how old was cimgui when these were last generated?) the function never shows up in cimgui.cpp, definitions.json etc...

Besides, this function probably has to be mapped in the "safe" imgui crate, even though it is internal?

@dbr
Copy link
Contributor

dbr commented Jan 19, 2023

The bindgen header for the "docking" feature has it, but the master branch does not.

Hmm, that is expected currently,

  • UpdateHoveredWindowAndCaptureFlags is in imgui_internal.h
  • For the docking functions cimgui is run with luajit generator.lua gcc "internal" -DIMGUI_USE_WCHAR32 so that function would be exposed
  • So the features = ["docking"] would expose that function in imgui-sys, but not with default-features

I attempted manually rerunning cimgui, but besides getting an inordinate amount of API breakage (how old was cimgui when these were last generated?)

That is surprising - the version of cimgui should be up-to-date (using the cimgui tag for the relevant imgui release)

I wonder if this is confusion from a combination of the docking feature using internal, and your fork being behind the main repo? The changes in #707 changes should all (except for fixing my typos!) be present in current main branch - e.g https://github.com/MarijnS95/imgui-rs/tree/bindgen-0.63 "1 commit ahead, 87 commits behind imgui-rs:main" and main repo currently is all usign 0.63

/* automatically generated by rust-bindgen 0.63.0 */

That aside, exposing the internal functions would be good thing to do - however,

  • It would be nice if the internal functions were exposed differently, e.g imgui_sys::internal::* - and not sure if this is easily doable with cimgui
  • I'd like to investigate using dear bindings before spending much more time changing the current bindgen setup (as it's starting to get slightly convoluted) - although it currently doesn't support internal at all but is planned

@MarijnS95
Copy link
Contributor Author

  • For the docking functions cimgui is run with luajit generator.lua gcc "internal" -DIMGUI_USE_WCHAR32 so that function would be exposed

Thanks for pointing that out, it would have been trivial to see after diffing the cimgui shell scripts between both folders.

I wonder if this is confusion from a combination of the docking feature using internal, and your fork being behind the main repo? The changes in #707 changes should all (except for fixing my typos!) be present in current main branch - e.g https://github.com/MarijnS95/imgui-rs/tree/bindgen-0.63 "1 commit ahead, 87 commits behind imgui-rs:main" and main repo currently is all usign 0.63

Indeed, I messed that up badly! Inline blaming showed most changes from a few weeks ago, leaving me to assume everything was up-to-date given the pace here.

That aside, exposing the internal functions would be good thing to do

Ack, unfortunately when adding the "internal" feature to imgui-master a bunch of function signatures break by getting a _Float or _ID (based on types?) appended to the name...

however,

I need access to igUpdateHoveredWindowAndCaptureFlags() to "fix" touch input support, but am not comfortable enough to help/address those points...

before spending much more time changing the current bindgen setup (as it's starting to get slightly convoluted)

Agreed, it'd be much better if it's all tied to a single runnable command instead of spread across documentation and various shell scripts.

@dbr
Copy link
Contributor

dbr commented Jan 20, 2023

I need access to igUpdateHoveredWindowAndCaptureFlags() to "fix" touch input support, but am not comfortable enough to help/address those points...

Ack, unfortunately when adding the "internal" feature to imgui-master a bunch of function signatures break by getting a _Float or _ID (based on types?) appended to the name...

The methods with _Float appended etc are just a way to represent overloaded functions. E.g handling blah(float x) and blah(int x) by making two functions blah_Float and blah_Int. If there is no overload, the generator doesn't append the types. Enabling the internal functions sometimes changes functions like igSetScrollX because there is a public function with just a float, and there's an internal one which takes a ImGuiWindow*

So it should be perfectly safe just to use the "obvious" function with the closest name - e.g a call like igSetScrollX(1.2) you just change to igSetScrollX_Float(1.2) (and the type system will save you if you pick the wrong one like igSetScrollX_WindowPtr)

It's also quite possible #[cfg(feature = "docking")] parts of imgui/ already have the required changes (as some of those are cause by the internal functions being generated, but not all - some are actual code changes in the docking branch)

@MarijnS95
Copy link
Contributor Author

Ack, I searched for matching functions but did not immediately see those overloads, but they're definitely there. Indeed, all I had to do was unwrap the #[cfg(feature = "docking")] in a few places to get it to compile.


Does this all mean you're okay taking a contribution that switches imgui-master to the "internal" feature? And for igUpdateHoveredWindowAndCaptureFlags specifically, does it make sense to add this to impl Io (associated function or method, since it doesn't need self)?

@MarijnS95
Copy link
Contributor Author

Fwiw it might still be interesting to look into this, but igUpdateHoveredWindowAndCaptureFlags() isn't performing as advertised (setting the mouse position and left mouse button down over an IMGUI Window still makes want_capture_mouse return false...).

        WindowEvent::Touch(t) if t.id == 0 => {
            io.add_mouse_pos_event([
                t.location.x as f32,
                t.location.y as f32,
            ]);

            if t.phase == TouchPhase::Started {
                io.add_mouse_button_event(imgui::MouseButton::Left, true);
            } else if t.phase == TouchPhase::Ended {
                io.add_mouse_button_event(imgui::MouseButton::Left, false);
            }

            // https://github.com/imgui-rs/imgui-rs/issues/706
            // https://github.com/ocornut/imgui/blob/0359f6e94fb540501797de1f320082e4ad96ce9c/docs/FAQ.md?plain=1#L130
            unsafe { imgui::sys::igUpdateHoveredWindowAndCaptureFlags() };

            dbg!(io.want_capture_mouse)
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants