feat(input): Implement SDL3 input and window management#2639
feat(input): Implement SDL3 input and window management#2639githubawn wants to merge 34 commits into
Conversation
a785545 to
7cc0861
Compare
|
| Filename | Overview |
|---|---|
| Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Input.cpp | Implements unified SDL3 input and gamepad synthesis, but coordinate scaling mixes SDL logical coordinates with pixel window dimensions. |
| Core/GameEngineDevice/Source/SDL3Device/Common/SDL3GameEngine.cpp | Integrates SDL event polling, text input, minimized-window handling, and unconditional base-engine initialization. |
| Core/Main/SDL3Main.cpp | Adds SDL3 startup, window creation, failure cleanup, and application bootstrap. |
| Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Cursor.cpp | Delegates animated cursor frame management to SDL3 and bounds cursor-direction resource lookups. |
| cmake/sdl3.cmake | Adds conditional SDL3 and SDL3-image dependency resolution. |
| vcpkg.json | Declares the SDL3 dependency versions used by vcpkg builds. |
Sequence Diagram
sequenceDiagram
participant SDL as SDL3
participant Manager as SDL3InputManager
participant Mouse as SDL3Mouse
participant Engine as GameEngine
SDL->>Manager: Poll keyboard, mouse, gamepad, and window events
Manager->>Mouse: Buffer mouse and synthetic gamepad events
Manager->>Engine: Forward keyboard, text, and quit state
Mouse->>Mouse: Translate and scale coordinates
Engine->>Engine: Process game input and update frame
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
Core/GameEngineDevice/Source/SDL3Device/GameClient/SDL3Input.cpp:355
**Logical coordinates use pixel scaling**
When SDL uses high-DPI or logical window scaling, mouse events and gamepad-generated events provide logical coordinates, but `scaleMouseCoordinates` scales them using pixel framebuffer dimensions. This coordinate-space mismatch maps cursor positions and clicks to the wrong in-game location.
Reviews (32): Last reviewed commit: "fix(sdl3): address PR review feedback on..." | Re-trigger Greptile
|
|
||
| namespace { | ||
|
|
||
| Bool DecodeNextUtf8Codepoint(const char* text, size_t length, size_t& offset, UnsignedInt& outCodepoint) |
There was a problem hiding this comment.
Do we need this, maybe better placed in UnicodeString? Or some other helper if it's generic stuff
There was a problem hiding this comment.
Good point.
I looked at #2045 and #2528 which are working on UTF-8 infrastructure.
If one of those lands first, forwardTextInputEvent could use the bulk Utf8_To_Utf16Le conversion and iterate the resulting wchar_t string directly, making this function unnecessary.
Happy to refactor once there's a clear winner between those two. For now it's self-contained here.
e365605 to
627ef23
Compare
eb9908a to
534e694
Compare
xezon
left a comment
There was a problem hiding this comment.
This needs polishing. Not yet reviewed extensively until the obvious style issues are fixed.
|
|
||
| namespace { | ||
|
|
||
| Bool DecodeNextUtf8Codepoint(const char* text, size_t length, size_t& offset, UnsignedInt& outCodepoint) |
There was a problem hiding this comment.
This function looks out of place. Bobtista was also working on Utf8 decoding in another change. It would be good to have this as a utility somewhere accessible engine wide, and not specific to this file.
|
Perhaps also check Fighter19's fork for SDL related implementations. As far as I am aware he has it all done. |
|
I have looked at all existing forks and attributed where possible. |
|
How about changing display-mode? eg getDisplayModeCount() / getDisplayModeDescription() using SDL_GetFullscreenDisplayModes + desktop mode + resolution filtering. EDIT: Also account for mouse position which can sometimes not match clicks to cursor position. |
|
Is it better to use SDL3InputManager (+ SDL3CursorManager) or to split out SDL3Keyboard + SDL3Mouse that subclass the engine's existing Keyboard/Mouse base classes? |
|
I've started cleaning up the style and structural issues in the latest commits. Regarding SDL3Main: The main benefit of the current approach is that we keep the game's native Win32 WinMain entry point and startup sequence intact. To get rid of the #ifdef blocks in WinMain while minimizing duplication, I can move the window initialization out into SDL3Window. Regarding splitting out subclasses: I don't think splitting the input handling into isolated SDL3Keyboard and SDL3Mouse subclasses is the right way. It runs contrary to how SDL3 tries to be a unified way; it relies on a single event pump where all keyboard, mouse, and gamepad signals flow together. |
But this is how you currently have set this up. What is the plan here? |
- Updated vcpkg.json overrides and regenerated vcpkg-lock.json (sdl3 to 3.4.10, sdl3-image to 3.4.4). - Normalized formatting on vcpkg.json using the official vcpkg format-manifest tool. - Changed translateScanCodeToKeyVal parameter type to SDL_Scancode to prevent high-range media keys from wrapping into valid letter keycodes. - Documented SDL3_image FetchContent Git pin reasoning in cmake/sdl3.cmake. Co-authored-by: fbraz3 <fbraz3@users.noreply.github.com> Co-authored-by: Fighter19 <Fighter19@users.noreply.github.com> Co-authored-by: feliwir <feliwir@users.noreply.github.com>
Replace legacy block comment banners with line comments. Group and sort header includes. Break long gamepad input lines in SDL3Input.cpp with newlines. Remove Sage mentions.
5da34d4 to
78f7e15
Compare
|
There's a lot of SDL stuff in my stack that you could use here, and I'd extract this into a separate SDL3Gamepad translator vs adopt its central input manager. See if Gippity can take a stab at taking the best of both |
- Fix symbol collisions for g_strFile, g_csfFile, gAppPrefix, and TheWin32Mouse in WinMain - Call AppMain::initBeforeWindow() in WinMain prior to accessing splash screen and TheGlobalData - Use timeGetTime() for key down timestamping in SDL3Input to match Keyboard::checkKeyRepeat() - Release active synthetic inputs and reset state on gamepad close in SDL3InputManager - Read live CapsLock toggle state from SDL via SDL_GetModState() in getCapsState() Co-authored-by: Bobby Battista <bobtista@gmail.com>
ca6aa95 to
38bc8f6
Compare
- Add radial deadzone removal, smoothstep response curve, and velocity acceleration for analog sticks - Retire precision mode in favor of continuous stick acceleration (LT is currently unmapped) - Add getControllerScrollScale() to scale camera panning when zoomed close to ground - Suppress mouse edge scrolling automatically while gamepad input is active - Expose getMinHeightAboveGround and getMaxHeightAboveGround on View Co-authored-by: stm <14291421+stephanmeesters@users.noreply.github.com>
- Replace SDL_GetWindowSizeInPixels with SDL_GetWindowSize in scaleMouseCoordinates - Fixes HiDPI/Retina coordinate space mismatch where mouse & gamepad motion events delivered in logical points were scaled by physical pixel dimensions Co-authored-by: Bobby Battista <bobtista@gmail.com>
- Fix camera panning and scroll state handling for gamepads - Fix double-click detection for gamepad button pulses to trigger Guard mode orders
| , m_Window(window) | ||
| , m_IsCaptured(false) | ||
| , m_IsVisible(true) | ||
| , m_LostFocus(false) |
There was a problem hiding this comment.
m_LostFocus is initialized false but nothing ever sets it
| if (m_state.stickUp) virtualPulseKey(SDL_SCANCODE_UP, false); | ||
| if (m_state.stickDown) virtualPulseKey(SDL_SCANCODE_DOWN, false); | ||
|
|
||
| if (m_state.buttonState[SDL_GAMEPAD_BUTTON_SOUTH]) virtualPulseKey(SDL_SCANCODE_RETURN, false); |
There was a problem hiding this comment.
These releases no longer match the mappings below. South/east synthesize mouse buttons, west maps to A, right shoulder maps to Shift, and the D-pad ordering differs. Start and Back are also left out. Disconnecting while held can therefore leave inputs latched and release unrelated keys.
Should press/release share the same mapping table, maybe in a separate gamepad translator?
| float resolutionScale = 1.0f; | ||
| int windowWidth = 0; | ||
| int windowHeight = 0; | ||
| if (m_window && SDL_GetWindowSizeInPixels(m_window, &windowWidth, &windowHeight) && windowHeight > 0) |
There was a problem hiding this comment.
Gamepad cursor deltas are applied to SDL logical-point coordinates, but this scale uses physical pixel height. On HiDPI displays that changes cursor speed by the backing-scale factor. Should this use SDL_GetWindowSize(), matching scaleMouseCoordinates()?
| { | ||
| switch (event.type) | ||
| { | ||
| case SDL_EVENT_QUIT: |
There was a problem hiding this comment.
This exits immediately, whereas the Win32 close path sends MSG_META_DEMO_INSTANT_QUIT so an in-game close opens the quit flow and allows a sequenced multiplayer disconnect. Should SDL use that same path, falling back to setQuitting() only before the message stream is ready?
| closeGamepad(); | ||
| break; | ||
|
|
||
| case SDL_EVENT_WINDOW_FOCUS_GAINED: |
There was a problem hiding this comment.
These events only update the mouse. The engine remains active, and keyboard state can stay held if key-up events are lost during Alt-Tab. Could focus handling update GameEngine::setIsActive() and reset keyboard state through an engine-level window-event handler
| case SDL_EVENT_KEY_DOWN: | ||
| case SDL_EVENT_KEY_UP: | ||
| if (!event.key.repeat) | ||
| addKeyboardSDLEvent(event); |
There was a problem hiding this comment.
SDL does not emit SDL_EVENT_TEXT_INPUT for Return. The text-entry gadgets commit on GWM_IME_CHAR(VK_RETURN), so chat and other entry fields may not submit. Could non-repeat Return/KP Enter key-down events send that message to the focused entry field?
| if (!m_gamepad) | ||
| return; | ||
|
|
||
| if (TheLookAtTranslator) |
There was a problem hiding this comment.
This marks controller input active whenever a gamepad is merely connected. canScrollAtScreenEdge() then disables physical-mouse edge scrolling for the entire session. Maybe track actual controller camera input or the last active input device instead?
|
|
||
| void SDL3Mouse::init() | ||
| { | ||
| Mouse::init(); |
There was a problem hiding this comment.
The common client initialization later forces the engine mouse position to (0,0), but SDL may emit no motion event when the cursor is stationary. That can immediately trigger top-left edge scrolling, which I've run into, though I don't remember if it was from this. Can the mouse interface provide syncPositionToSystemCursor(), with SDL reading the live OS cursor instead of forcing the origin?
| return exitcode; | ||
| } | ||
|
|
||
| Uint32 flags = SDL_WINDOW_HIDDEN | SDL_WINDOW_RESIZABLE; |
There was a problem hiding this comment.
The window is marked resizable, but the event pump does not handle SDL_EVENT_WINDOW_RESIZED, leaving the display resolution, UI layout, camera, and mouse bounds stale after a drag resize. I would remove SDL_WINDOW_RESIZABLE for this PR, or add deferred resize synchronization after the window size settles
|
|
||
| GameWindow* focusedWindow = TheWindowManager->winGetFocus(); | ||
| const Bool wantsTextInput = | ||
| focusedWindow != nullptr && BitIsSet(focusedWindow->winGetStyle(), GWS_ENTRY_FIELD); |
There was a problem hiding this comment.
Editable combo boxes also take text input, but this only starts text input for GWS_ENTRY_FIELD, so typing into a focused combo box does nothing. Could it check GWS_COMBO_BOX too?
|
It's improved a lot. I left more comments - also, there are thousands of lines changed in MainMenuUtils.cpp, CHATAPI.cpp, and both OptionsMenu.cpp that are just line ending stuff, could remove that and let a formatting pass deal with it later. My stack has working implementations of several of the inline items (close → MSG_META_DEMO_INSTANT_QUIT routing, focus → setIsActive + keyboard reset, Return → GWM_IME_CHAR commit, cursor position sync, deferred resize) if you want to lift from them. |
0acb86c to
3bd8573
Compare
SDL3 Input Backend
This PR implements an SDL3-based input and windowing backend as a modern alternative to DirectInput and Win32 window creation. By utilizing SDL3, we bypass DirectInput emulation layers on modern systems, providing a lower-latency pipeline for Windows 11 and Wine/Linux users.
Input handling
Unified event manager that centralizes keyboard, mouse, and gamepad events into a thread-safe buffer
Gamepad Support (v0.1)
This is an initial baseline implementation focused on providing functional out-of-the-box playability. Feedback is encouraged regarding the ergonomics and logic of these default mappings.
Scope Note: This implementation provides a hardcoded default layout to establish core functionality. Advanced features, such as input remapping, radial menus, adjustable deadzones, are currently out of scope for this PR and may be addressed in future iterations.
The foundation of this backend was built using the SDL3 input work from the generalsX fork by fbraz3.
todo:
properly attribute all forks that have inspired this.