v3.3/glfw: GetKeyName incorrectly panics on unknown key and scancode

[Jacalz]

We have a few issues open for cases where we are recovering from a panic in cases where a button that GLFW gets an invalid scancode because of keys that it doesn't recognize (https://github.com/search?q=repo%3Afyne-io%2Ffyne+poll+event+error++&type=issues&state=open). This is currently being handled by deferring a recover function:
https://github.com/fyne-io/fyne/blob/dc34dad76137def4466ab8eb4166e7ae741ef9ee/internal/driver/glfw/loop_desktop.go#L28-L34

It would be nice if we could teach acceptError() to avoid issuing a panic in this case.

[dmitshur]

Thanks for reporting.

From looking at fyne-io/fyne#1024, which is from 2020, I tried pressing the Fn key on macOS in an events test program (which uses the glfw API as is, without recovers), and it printed:

000001e4 to 1 at 3.736: Key 0x-001 Scancode 0x003f (UNKNOWN) (with no mods) was released
000001e5 to 1 at 4.209: Key 0x-001 Scancode 0x003f (UNKNOWN) (with no mods) was released

Notably, there wasn't a panic.

Are you able to reproduce this issue with the latest version of v3.3/glfw?

[Jacalz]

I don't have easy access to any macOS so can't confirm how it behaves. However, there are very recent comments on the issue that they can replicate the invalid scancode issues (with multiple keys): fyne-io/fyne#1024 (comment)

[dmitshur]

Thanks. I'm able to reproduce it now with the following program on macOS:

package main

import (
	"runtime"

	"github.com/go-gl/glfw/v3.3/glfw"
)

func init() { runtime.LockOSThread() }
func main() {
	if err := glfw.Init(); err != nil {
		panic(err)
	}
	defer glfw.Terminate()

	// The key callback handler reports the key glfw.KeyUnknown and scancode 0x003f
	// when the 'Fn' key is pressed on macOS.
	//
	// Upstream docs for glfwGetKeyName (https://www.glfw.org/docs/3.3/group__input.html#gaeaed62e69c3bd62b7ff8f7b19913ce4f)
	// say:
	//
	//	If the key is GLFW_KEY_UNKNOWN, the scancode is used to identify the key, otherwise the scancode is ignored. If you specify a non-printable key, or GLFW_KEY_UNKNOWN and a scancode that maps to a non-printable key, this function returns NULL but does not emit an error.
	//
	//	This behavior allows you to always pass in the arguments in the key callback without modification.
	//
	// So the next line should be okay.
	_ = glfw.GetKeyName(glfw.KeyUnknown, 0x003f)

	// Output:
	// panic: InvalidValue: Invalid scancode 63
	//
	// goroutine 1 [running, locked to thread]:
	// github.com/go-gl/glfw/v3.3/glfw.acceptError({0x0, 0x0, 0x1400004e738?})
	// 	$GOMODCACHE/github.com/go-gl/glfw/v3.3/glfw@v0.0.0-20231223183121-56fa3ac82ce7/error.go:173 +0x204
	// github.com/go-gl/glfw/v3.3/glfw.panicError(...)
	// 	$GOMODCACHE/github.com/go-gl/glfw/v3.3/glfw@v0.0.0-20231223183121-56fa3ac82ce7/error.go:184
	// github.com/go-gl/glfw/v3.3/glfw.GetKeyName(0x14000065f38?, 0x1005c43b0?)
	// 	$GOMODCACHE/github.com/go-gl/glfw/v3.3/glfw@v0.0.0-20231223183121-56fa3ac82ce7/input.go:445 +0x40
	// main.main()
	// 	/tmp/pErjaJJXDo/main.go:27 +0x48
	// exit status 2
}

My earlier attempt to reproduce this involved only the key callback, but it turns out this happens when GetKeyName is also involved.

Based on the upstream GLFW docs, it seems the problem is upstream and tracked in glfw/glfw#1785 and will be fixed in elmindreda/glfw@01631b9. (I tested it locally and it indeed causes GetKeyName to return the empty string instead of panicking.)

[Jacalz]

Thanks for working with upstream on this @dmitshur. This should be resolved now that we have v3.3.10 in the repos. Would you mind checking if you can reproduce with the latest code on master?

[dmitshur]

I can confirm the same program no longer panics now that GLFW 3.3.10 is pulled in. glfw.GetKeyName(glfw.KeyUnknown, 0x003f) returns an empty string. I think we can close this unless there's something more to do here.

[Jacalz]

Perfect. Having this fix in means that we can remove recover() from where we are running PollEvents() 60 times per second in the run loop of Fyne and as such improve performance.