-
Notifications
You must be signed in to change notification settings - Fork 2
Add InputManager with global keyboard event dispatching #210
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot See my reviews, and:
- Revert the changes on examples
- Revert all the JavaScript/TypeScript changes
src/runtime/embedder.cpp
Outdated
} | ||
|
||
// Initialize InputManager after constellation is created | ||
inputManager = make_shared<input_manager::TrInputManager>(constellation); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
input manager should be in constellation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved InputManager initialization to constellation.cpp instead of embedder.cpp. The InputManager is now created as part of TrConstellation and accessed via constellation->inputManager. Commit: 69ed0f1
* @param eventData The keyboard event data | ||
* @return true if the event was successfully injected, false otherwise | ||
*/ | ||
bool injectKeyboardEvent(uint32_t contentId, const TrKeyboardEventData &eventData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool injectKeyboardEvent(uint32_t contentId, const TrKeyboardEventData &eventData); | |
bool dispatchKeyboardEvent(const TrKeyboardEventData &eventData); |
Keyboard event is a global event, no need the content id to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed method name to dispatchKeyboardEvent
and removed the contentId parameter. Keyboard events are now globally dispatched to all active content runtimes. Commit: 69ed0f1
* @param eventData The keyboard event data | ||
* @return number of content runtimes that received the event | ||
*/ | ||
int broadcastKeyboardEvent(const TrKeyboardEventData &eventData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto, remove this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the broadcastKeyboardEvent
method as requested. Now there's only one dispatchKeyboardEvent
method that handles global dispatch. Commit: 69ed0f1
* This class provides an abstraction layer between platform-specific input | ||
* capture (Unity, Unreal, native, etc.) and JSAR Runtime's event system. | ||
*/ | ||
class TrInputManager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Please redesign this class as the input event is globally at runtime, we don't need to dispatch a keyboard event to a specific content, each input/keyboard event should be dispatched for both listened contents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redesigned the InputManager class to handle global input events. Removed content-specific methods and simplified the API to only support global dispatch via dispatchKeyboardEvent
. All keyboard events are now dispatched to all listening content runtimes automatically. Commit: 69ed0f1
KeyboardPress, | ||
MouseDown, | ||
MouseUp, | ||
MouseMove, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MouseMove can't be exist at this project, because we don't allow content receive the native window's mouse move event, as for MouseDown and MouseUp, it has been handled by WebXR input source such as primary actions, so you need to remove them here, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed MouseMove, MouseDown, and MouseUp from the TrInputEventType enum. The input event system now only supports keyboard events (KeyboardDown, KeyboardUp, KeyboardPress). Commit: 69ed0f1
…plify API, remove examples and TS changes Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
This PR implements a streamlined input event management system for JSAR Runtime, enabling host applications to dispatch keyboard events globally to all active content runtimes.
Architecture Overview
The implementation provides clean separation between event capture and event management:
Key Components
1. InputManager Module (
src/runtime/input-manager/
)2. Extended Event System
InputEvent
type added to native event system with keyboard support3. Simple Embedder API
Usage Example
Host Application (C++):
Web Application (JavaScript):
Design Benefits
Testing
This implementation provides a focused, global approach to keyboard input management suitable for spatial web environments where input events should be available to all active content.
Fixes #209.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.