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

Switch to SDL2, drop winit ASAP. Point numbering broken, console broken on Linux due to winit kbd implementation being bad. #31

Closed
ctrlcctrlv opened this issue Sep 19, 2020 · 6 comments · Fixed by #79
Assignees
Labels
bug Something isn't working urgent Urgent bug causing significant breakage.
Milestone

Comments

@ctrlcctrlv
Copy link
Collaborator

ctrlcctrlv commented Sep 19, 2020

This says it all:

https://github.com/rust-windowing/winit/blob/386ead15a3e3fef5c0c02bd7491d3cc9a7a6e0aa/src/platform_impl/linux/x11/events.rs#L178

//ffi::XK_numbersign => VirtualKeyCode::Numbersign

🤦‍♂️🤦‍♂️🤦‍♂️ 😱❗❗❗

Issue discovered by @eliheuer in his Twitch stream. Confirmed and cause found just now by @iwsfutcmd.

I don't trust any software with code like this commented out period. They have no idea what they're doing in my humble opinion. The fact that Subtract and Minus were only recently unified is shocking as well.

So, this is an urgent issue. I had hoped not to need to rewrite main.rs again, but I don't mind given the severity of their keyboard handling.

SDL2 supports Unicode input, so console will still work without much worry about hooking OS APIs for IME's etc.

@ctrlcctrlv ctrlcctrlv added bug Something isn't working urgent Urgent bug causing significant breakage. labels Sep 19, 2020
@ctrlcctrlv
Copy link
Collaborator Author

Cf. rust-windowing/winit#1656, rust-windowing/winit#812, btw. These are years old issues, not new bugs.

@ctrlcctrlv ctrlcctrlv added this to the 1.0 milestone Sep 19, 2020
@ctrlcctrlv ctrlcctrlv self-assigned this Sep 19, 2020
@iwsfutcmd
Copy link

iwsfutcmd commented Sep 19, 2020

One suggestion is, when monitoring text input events, make sure to track SDL_TEXTINPUT rather than SDL_TEXTEDITING, especially for the console. Nothing's worse than a program ignoring my IME because it thinks it knows what to do with my keystrokes better than I do.

@ctrlcctrlv
Copy link
Collaborator Author

Yup, I made sure Japanese IME worked in current winit console and will do the same for SDL2. 😃

@ctrlcctrlv
Copy link
Collaborator Author

OK, I've sort of gotten it working. Sorry for the delay, was focused on Spiro, which is a 1.0 feature per #10. (Cf. fontforge/libspiro#27, fontforge/libspiro#28 (bottom))

[2020-09-21T11:07:57Z INFO  Qglif] KeyDown { timestamp: 16780, window_id: 1, keycode: Some(RShift), scancode: Some(RShift), keymod: RSHIFTMOD | NUMMOD, repeat: false }
[2020-09-21T11:07:57Z INFO  Qglif] KeyDown { timestamp: 16943, window_id: 1, keycode: Some(Num3), scancode: Some(Num3), keymod: RSHIFTMOD | NUMMOD, repeat: false }
[2020-09-21T11:07:57Z INFO  Qglif] TextInput { timestamp: 16943, window_id: 1, text: "#" }
[2020-09-21T11:07:57Z INFO  Qglif] KeyUp { timestamp: 17065, window_id: 1, keycode: Some(Num3), scancode: Some(Num3), keymod: RSHIFTMOD | NUMMOD, repeat: false }
[2020-09-21T11:07:57Z INFO  Qglif] KeyUp { timestamp: 17095, window_id: 1, keycode: Some(RShift), scancode: Some(RShift), keymod: NUMMOD, repeat: false }

Much better—seems consistent too.

image

I need to fix ImGui: the copied support.rs assumes winit. I also need to rewrite our event matching code, which shouldn't be too hard, and make en masse tweaks to the event handlers, which should be doable with a few vim commands.

I wonder if @aclysma would be interested in a support.rs for SDL2? Skulpin seems to be in maintenance mode so I'm not sure.

@ctrlcctrlv
Copy link
Collaborator Author

Oh, and this seems to be working too @iwsfutcmd :-)

[2020-09-21T11:19:49Z INFO  Qglif] TextInput { timestamp: 169569, window_id: 1, text: "アイ・エム・イー" }

@ctrlcctrlv
Copy link
Collaborator Author

The war is over! https://twitter.com/fr_brennan/status/1373849754715705344

@MatthewBlanchard figured out that we could solve all of our switchover problems by just drawing Imgui with Skia. He's writing a Skia-based renderer for Imgui, which we'll use. Then we can drop all of the hacks which allow us to draw @ocornut's stock Imgui Vulkan renderer on top of Skulpin (Skia+Vulkan).

This allows us to flip over to SDL extremely easily. And because SDL can be used from WebAssembly…that means that MFEKglif could, some day, be compiled for the web browser. Really! Amazing work.

ctrlcctrlv added a commit that referenced this issue Mar 28, 2021
Closes #31.

* First commit. Most of the way migrated to SDL2.

* More fixes. Needs review.

* Fix cargo.toml paths to point at remote repo.

* Moved imgui-skia-renderer to it's own repo and cleaned up main.

Fixed ctrl+Q.
Refactored main.
Moved imgui to it's own repo.

* Removed dead code.

* Guidelines and console fixed.

* Made console steal input. Removed main_winit.rs

* Fix Ctrl-Q

* rustfmt

Co-authored-by: Matthew Blanchard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working urgent Urgent bug causing significant breakage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants