Proper handling of unknown SDL_Keymod bits in Event.from_ll

[kolen]

Currently, if SDL generates SDL_KEYDOWN keyboard event which have at least one bit set that we don't know, we treat it as "no key modifiers"

rust-sdl2/src/sdl2/event.rs

Line 1281 in aefd78d

keymod: Event::unwrap_keymod(keyboard::Mod::from_bits(event.keysym.mod_)),

rust-sdl2/src/sdl2/event.rs

Lines 1622 to 1627 in aefd78d

	pub fn unwrap_keymod(keymod_option: Option<keyboard::Mod>) -> keyboard::Mod {
	match keymod_option {
	None => keyboard::Mod::empty(),
	Some(x) => x,
	}
	}

Moreover, this behavior is only for SDL_KEYDOWN, for SDL_KEYUP behavior is to crash if any extra bit is set:

rust-sdl2/src/sdl2/event.rs

Line 1293 in aefd78d

keymod: keyboard::Mod::from_bits(event.keysym.mod_).unwrap(),

This was added in #780 after observed crashes because SDL sometimes set bits that we don't know (but only on certain systems).

Maybe we should ignore extra bits (which may be added in future versions of SDL) and look only at bits we know? I.e. by replacing from_bits with from_bits_truncate. I think SDL (from the viewpoint of caller) is free to set undocumented bits to any value (but shouldn't from the viewpoint of implementation). If I used SDL from C/C++ code, I wouldn't crash or default to "no modifiers" if I see extraneous bits set, I'd only check for known modifier bits like if (event->key.keysym.mod & (KMOD_LSHIFT | KMOD_RSHIFT)) ….

Also, current behavior (and also my proposed behavior too) can mask underlying problem with SDL itself (in which keyboard input is notorious to be broken often) or with C wrapping code. For example, in old sdl2 crate 0.22.0 I had mod_ having value of 32766 (15 bits is 1, least bit is 0) in all keyup events, causing unwrap panic. In current versions, I don't observe this behavior, despite keyup code is still from_bits(…).unwrap: probably C wrapper code was fixed (I built both versions of crate against the same underlying libsdl which reports correct key mod values according to this script).

[Cobrand]

Maybe we should ignore extra bits (which may be added in future versions of SDL) and look only at bits we know? I.e. by replacing from_bits with from_bits_truncate

I think that makes sense to use this option. If by any chance someone finds a new modifier that isn't handled with this yet, they can always use the sdl2-sys functions and retrieve the exact bit that they want. In either case, we never want a crash so we have to fix this.

[Lokathor]

#877 is the same general concept as this