godot
godot copied to clipboard
OpenXR: Add support for binding modifiers
Adds support for OpenXR binding modifiers to the action map.
This allows you to add modifiers to inputs such as applying thresholds or offsetting values, etc. Binding modifiers can add additional input paths or change the type of an input path.
Todos:
- [x] Create infrastructure for binding extension system to the action map
- [x] Make UI changes to the action map UI
- [x] Implement support for haptic triggers
- [x] Implement analog threshold extension (may move this into vendor plugin)
- [X] implement resource class
- [x] implement UI
- [x] Implement dpad binding extension
- [x] implement resource class
- [x] implement UI
Depends on:
- #97392
- #98163
Contributed by Khronos Group through the Godot Integration Project
Back working on this after dealing with a bout of the flu. Starting to get an interface together but need to change direction slightly.
While some of this is a bit of an unknown (but likely) future, I changed our action map structure slightly so we can record binding modifiers both on interaction profiles and on individual bindings.
Right now the dpad modifier is a bit of the odd duckling being the only modifier that is linked to an action set/source path combination and is thus recorded on the interaction profile level. The analog threshold modifier is linked to an action/source path combo which is recorded as a binding (hence the changes in #98163) and it is very likely that this is going to be true for future modifiers as well.
Right now they are all added to the next chain or XrInteractionProfileSuggestedBinding when calling xrSuggestInteractionProfileBindings however this may change/be enhanced in the future.
UI is finally coming together. Binding modifiers are accessible either on the interaction profile (1) or for individual bindings (2):
Either button opens up a popup that allows for creating applicable binding modifiers on that level:
I still need to change the dialog for selecting a new binding modifier as it doesn't filter by type, and we only have the two modifiers right now but that will change in the near future.
Some interesting feedback concerning the DPad modifier extension that we need to ensure gets documented well.
The extension actually does two things when enabled.
- It adds new binding paths you can use for each thumbstick and thumbpad input. These paths are immediately usable even if you do not add a dpad binding modifier
- And it adds the dpad binding modifier, which lets you customise the dpad behaviour
All the new paths have been added to the meta data database. In doing this I've cleaned up a lot of the code to make it easier to manage. Still planning on adding a bit of extra logic in the UI when editing the action map so we only show bindings that are available based on which extensions have been selected in project settings so we don't get a whole bunch the user isn't using.
Other then needing to do some more testing, this should now all work.
My only remaining issue is that I had to use the EditorInspector class to make the property editors work properly but these introduce the resource section into the UI and I haven't found a way to supress this:
These sections make no sense in this context so I hope there is a way to remove them.
One more extra thing, this PR also introduces the OpenXRHapticBase base class and OpenXRHapticVibration class, currently only used for the new features in the action map. There are a number of vendor specific implementations of the haptic feedback OpenXR struct that forms the base of this implementation that are also used for triggering haptic feedback pulses from code.
I'm thinking of replacing OpenXRAPI::trigger_haptic_pulse with a version that takes an OpenXRHapticBase object as a parameter and we'll be able to perform different haptic effects as they are added to the OpenXR spec generically.
This is something we'll do in a separate PR however.
Maybe you can figure out why the tests fail.
editor/editor_inspector.cpp:4098:57: runtime error: member access within null pointer of type 'struct EditorFeatureProfileManager'
editor/editor_inspector.cpp:4098:154: runtime error: member access within null pointer of type 'struct Node'
================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.4.dev.gh-97140 (b26897c0a0f76a2c8ee1f48419e975911b95d691)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] ./bin/godot.linuxbsd.editor.dev.double.x86_64.san(+0x380b26f7) [0x55bd781cc6f7] (??:?)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x43090) [0x7fc58e828090] (??:0)
[3] EditorInspector::_notification(int) (??:?)
[4] EditorInspector::_notificationv(int, bool) (??:?)
[5] Object::notification(int, bool) (??:?)
[6] Node::_propagate_ready() (??:?)
[7] Node::_propagate_ready() (??:?)
[8] Node::_propagate_ready() (??:?)
[9] Node::_set_tree(SceneTree*) (??:?)
[10] Node::_add_child_nocheck(Node*, StringName const&, Node::InternalMode) (??:?)
[11] Node::add_child(Node*, bool, Node::InternalMode) (??:?)
[12] void call_with_variant_args_helper<__UnexistingClass, Node*, bool, Node::InternalMode, 0ul, 1ul, 2ul>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool, Node::InternalMode), Variant const**, Callable::CallError&, IndexSequence<0ul, 1ul, 2ul>) (??:?)
[13] void call_with_variant_args_dv<__UnexistingClass, Node*, bool, Node::InternalMode>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool, Node::InternalMode), Variant const**, int, Callable::CallError&, Vector<Variant> const&) (??:?)
[14] MethodBindT<Node*, bool, Node::InternalMode>::call(Object*, Variant const**, int, Callable::CallError&) const (??:?)
[15] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (??:?)
[16] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (??:?)
[17] Object::callp(StringName const&, Variant const**, int, Callable::CallError&) (??:?)
[18] Variant::callp(StringName const&, Variant const**, int, Variant&, Callable::CallError&) (??:?)
[19] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (??:?)
[20] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (??:?)
[21] Node::_gdvirtual__ready_call() (??:?)
[22] Node::_notification(int) (??:?)
[23] Node::_notificationv(int, bool) (??:?)
[24] Object::notification(int, bool) (??:?)
[25] Node::_propagate_ready() (??:?)
[26] Node::_set_tree(SceneTree*) (??:?)
[27] Node::_add_child_nocheck(Node*, StringName const&, Node::InternalMode) (??:?)
[28] Node::add_child(Node*, bool, Node::InternalMode) (??:?)
[29] void call_with_variant_args_helper<__UnexistingClass, Node*, bool, Node::InternalMode, 0ul, 1ul, 2ul>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool, Node::InternalMode), Variant const**, Callable::CallError&, IndexSequence<0ul, 1ul, 2ul>) (??:?)
[30] void call_with_variant_args_dv<__UnexistingClass, Node*, bool, Node::InternalMode>(__UnexistingClass*, void (__UnexistingClass::*)(Node*, bool, Node::InternalMode), Variant const**, int, Callable::CallError&, Vector<Variant> const&) (??:?)
[31] MethodBindT<Node*, bool, Node::InternalMode>::call(Object*, Variant const**, int, Callable::CallError&) const (??:?)
[32] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (??:?)
[33] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (??:?)
[34] Node::_gdvirtual__process_call(double) (??:?)
[35] Node::_notification(int) (??:?)
[36] Node::_notificationv(int, bool) (??:?)
[37] CanvasItem::_notificationv(int, bool) (??:?)
[38] Control::_notificationv(int, bool) (??:?)
[39] Object::notification(int, bool) (??:?)
[40] SceneTree::_process_group(SceneTree::ProcessGroup*, bool) (??:?)
[41] SceneTree::_process(bool) (??:?)
[42] SceneTree::process(double) (??:?)
[43] Main::iteration() (??:?)
[44] OS_LinuxBSD::run() (??:?)
[45] ./bin/godot.linuxbsd.editor.dev.double.x86_64.san(main+0x4a3) [0x55bd781cbf9c] (??:?)
[46] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7fc58e809083] (??:0)
[47] ./bin/godot.linuxbsd.editor.dev.double.x86_64.san(_start+0x2e) [0x55bd781cba3e] (??:?)
-- END OF BACKTRACE --
Maybe you can figure out why the tests fail.
Looks like a failure to check for NULL parameters with standard unit tests. Sadly on a part of the API that is completely unrelated to this work, so I don't know why it's suddenly failing.
Hmm I wonder, the line of code that fails is EditorFeatureProfileManager::get_singleton()->connect("current_feature_profile_changed", callable_mp(this, &EditorInspector::_feature_profile_changed));
I'm guessing the testing doesn't setup the editor classes so possibly me using the EditorInspector in classes that are exposed as they are designed to be subclassed in the vendors plugin.
Did some more testing with this while addressing some of the feedback, found out that when both modifier extensions are enabled in the settings, one wouldn't work because both extensions require the base modifier extension. Fixed that up so Godot will react properly if multiple wrappers ask for the same extension to be enabled.
@BastiaanOlij Is there a test project that can be used to help validate those changes?
@m4gr3d, I added a demo project on godot-demo-projects, see link in OP.
Is that expected? If so, how should dpad modifiers be added?
DPad modifiers aren't added to individual bindings, they are added to the interaction profile with the binding button on the right. I need to figure out if I can add additional filters to the standard popup. I didn't want to create two separate base classes but maybe i need to go down that route.
@BastiaanOlij:
DPad modifiers aren't added to individual bindings, they are added to the interaction profile with the binding button on the right. I need to figure out if I can add additional filters to the standard popup. I didn't want to create two separate base classes but maybe i need to go down that route.
I think even just having a more user-friendly error message would be OK. Right now, I'm getting a very programmer-y error and the modifier just fails to be added.
Maybe we just need a popup with an explanation?
@BastiaanOlij:
DPad modifiers aren't added to individual bindings, they are added to the interaction profile with the binding button on the right. I need to figure out if I can add additional filters to the standard popup. I didn't want to create two separate base classes but maybe i need to go down that route.
I think even just having a more user-friendly error message would be OK. Right now, I'm getting a very programmer-y error and the modifier just fails to be added.
Maybe we just need a popup with an explanation?
I decided to go down the subclass path after all. So there is now an OpenXRIPBindingModifier class for binding modifiers that are recorded on interaction profile level, and a OpenXRActionBindingModifier class for binding modifiers that are recorded on individual actions/bindings.
Right now it seems a little overkill but once more binding modifiers become available it will start to make a whole lot more sense I recon.
@dsnopek Thanks for checking it again though, I do think it's time we merge this and any further things we find we can deal with later. It's a big PR but for people who don't use binding modifiers it's fairly low risk.
@AThousandShips dang :) That's a lot of cleanup of existing code. Worth doing I guess, I'll go through it later today :)
Limited it largely to new code 😅
Thanks!