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

Can not initialize GLFW without joystick support #127

Closed
tux21b opened this issue Feb 23, 2015 · 27 comments
Closed

Can not initialize GLFW without joystick support #127

tux21b opened this issue Feb 23, 2015 · 27 comments
Assignees

Comments

@tux21b
Copy link

tux21b commented Feb 23, 2015

I'm not quite sure what's the problem with the joystick configuration of my system, but the Go GLFW package panics with:

panic: PlatformError: Linux: Failed to watch for joystick connections in /dev/input: No space left on device
goroutine 16 [running]:
runtime.panic(0x501f80, 0xc20803e020)
    /home/christoph/go/src/pkg/runtime/panic.c:279 +0xf5
github.com/go-gl/glfw/v3.1/glfw.acceptError(0x7f769b90ae98, 0x1, 0x1, 0x0, 0x0)
    /home/christoph/gopath/src/github.com/go-gl/glfw/v3.1/glfw/error.go:164 +0x13d
github.com/go-gl/glfw/v3.1/glfw.Init(0x0, 0x0)
    /home/christoph/gopath/src/github.com/go-gl/glfw/v3.1/glfw/glfw.go:33 +0x70

The error itself is triggered at glfw/src/linux_joystick.c:_glfwInitJoysticks():213 which contains a comment in the following line saying: // Continue without device connection notifications. Therefore, I expect everything (except discovering new joysticks) to work. Unfortunately, the GLFW Go binding is way more strict and throws a panic in this case.

@tapir
Copy link
Member

tapir commented Feb 24, 2015

Can you post some example code that triggers this?

Edit: We've made platform errors panic in purpose because they are supposed to be not recoverable. I will check.

@tux21b
Copy link
Author

tux21b commented Feb 24, 2015

Edit: We've made platform errors panic in purpose because they are supposed to be not recoverable. I will check.

I am not sure if that's the right thing, especially for those errors that are explicitly non-fatal according to the GLFW source. There are quite a lot of such ignore / resume without ... comments in the GLFW C source.

Here is a full example, but I doubt that it will be of much use. I'm still not sure why inotify isn't working for /dev/input on this system (there is plenty of available disk space of course):

package main

import (
    "github.com/go-gl/glfw/v3.1/glfw"
    "log"
    "runtime"
)

func init() {
    runtime.LockOSThread()
}

func main() {
    log.Println("GLFW version:", glfw.GetVersionString())

    err := glfw.Init()
    if err != nil {
        log.Fatal(err)
    }
    defer glfw.Terminate()
}

Output:

[christoph@tuxmobil ~]$ go run glfw_bug.go 
2015/02/24 10:08:03 GLFW version: 3.1.0 X11 GLX glXGetProcAddressARB clock_gettime /dev/js
panic: PlatformError: Linux: Failed to watch for joystick connections in /dev/input: No space left on device

goroutine 16 [running]:
runtime.panic(0x4e44c0, 0xc208046000)
  /home/christoph/go/src/pkg/runtime/panic.c:279 +0xf5
github.com/go-gl/glfw/v3.1/glfw.acceptError(0x7f449352ce80, 0x1, 0x1, 0x0, 0x0)
  /home/christoph/gopath/src/github.com/go-gl/glfw/v3.1/glfw/error.go:164 +0x13d
github.com/go-gl/glfw/v3.1/glfw.Init(0x0, 0x0)
  /home/christoph/gopath/src/github.com/go-gl/glfw/v3.1/glfw/glfw.go:33 +0x70
main.main()
  /home/christoph/glfw_bug.go:16 +0x103

goroutine 19 [finalizer wait]:
runtime.park(0x426e00, 0x7ae478, 0x7a3409)
  /home/christoph/go/src/pkg/runtime/proc.c:1369 +0x89
runtime.parkunlock(0x7ae478, 0x7a3409)
  /home/christoph/go/src/pkg/runtime/proc.c:1385 +0x3b
runfinq()
  /home/christoph/go/src/pkg/runtime/mgc0.c:2644 +0xcf
runtime.goexit()
  /home/christoph/go/src/pkg/runtime/proc.c:1445

goroutine 17 [syscall]:
runtime.goexit()
  /home/christoph/go/src/pkg/runtime/proc.c:1445
exit status 2

@tux21b
Copy link
Author

tux21b commented Feb 24, 2015

To be honest, I really dislike all those panics in the GLFW Go binding. In my opinion, a Go library should only panic when the API is used incorrectly (i.e. programming errors). Everything else should return an error. It's fine to use panics internally, but you should recover and return a error code in your public API.

@tapir
Copy link
Member

tapir commented Feb 24, 2015

All the recoverable errors are returned. We panic only when the error is not
recoverable. But it's possible we missed something.

The problem is, all GLFW methods are allowed to raise error so for an
idiomatic Go binding we would either have to return errors from all the
functions or panic when error is not recoverable and get rid off all the unnecessary
returns.

To be honest, I really dislike all those panics in the GLFW Go binding. In
my opinion, a Go library should only panic when the API is used incorrectly
(i.e. programming errors). Everything else should return an error. It's
fine to use panics internally, but you should recover and return a error
code in your public API.


Reply to this email directly or view it on GitHub
#127 (comment).

@dmitshur
Copy link
Member

In my opinion, a Go library should only panic when the API is used incorrectly (i.e. programming errors). Everything else should return an error.

This is our intention.

This looks like a bug that needs to be fixed on our end.

@slimsag
Copy link
Member

slimsag commented Feb 24, 2015

It seems to me like it is a documentation discrepancy. The GLFW Error docs clearly state that GLFW_PLATFORM_ERROR is:

A platform-specific error occurred that does not match any of the more specific categories.

Analysis
    A bug in GLFW or the underlying operating system. Report the bug to our issue tracker.

Either the documentation is incorrect -- or this is a bug with GLFW/OS.

@dmitshur
Copy link
Member

I've looked at glfwInit code in the C library, and I seen a few places where it emits GLFW_PLATFORM_ERROR while still continuing on, with comments like "Well, this isn't available, do something else."

It may be that the C library may return multiple errors per single func call... And not all errors are critical?

@tux21b
Copy link
Author

tux21b commented Feb 24, 2015

@shurcooL Yep, I think so. In my case, the inotify failure is definitely non-critical. Everything (including joystick input) is working fine. I think the error just tells me that I might have to restart the game when I plug in an additional controller. The C library also calls the error callback so that the GLFW user can log that error for example and then CONTINUES initializing everything else. The Go binding however is unusable after the panic and there is no way to bypass the (non-fatal) error.

@slimsag
Copy link
Member

slimsag commented Feb 24, 2015

My point is that either:

  1. GLFW shouldn't emit GLFW_PLATFORM_ERRORs when it can't initialize joystick support (this is a bug in GLFW) OR
  2. GLFW_PLATFORM_ERROR is not a GLFW/OS bug as the docs say, and they need updating.

Before we know the answer to that -- we can't correctly fix this issue. I'd be willing to bet that the docs need updating.

@slimsag
Copy link
Member

slimsag commented Feb 24, 2015

@tux21b do note that as a temporary workaround (if you need to develop while we're looking into a fix for this bug) you can recover() like so:

(redacted code -- see below)

@tux21b
Copy link
Author

tux21b commented Feb 24, 2015

@slimsag I think recovering here is useless, stack unwinding already happened because of the panic and glfw.Init might have been aborted prematurely. In fact, I do not think that it is possibly to work around this bug without modifying glfw/error.go.

@slimsag
Copy link
Member

slimsag commented Feb 24, 2015

@tux21b You're absolutely right -- my apologies for the incorrect workaround.

I've formed an issue asking for clarification on the docs -- we'll get to the bottom of this and get a fix pushed as soon as possible.

@tux21b
Copy link
Author

tux21b commented Feb 24, 2015

@slimsag I am neither an GLFW developer nor an experienced GLFW user, but my understanding from using the C library is that the error callback might be called for quite a lot of things, even when glfwInit succeeds. The intended use is probably to just log those. The errors reported by the error callback might or might not be the reason for failure.

The Go binding (in comparison to the C lib) seems to try to match calls of the error callback to the success / non-success status of some functions. But this seems to be flawed. glfwInit might call the error callback multiple times and still return success. In addition to that, the error callback might get called asynchronously and there might be no (or multiple) GLFW calls. So I think it would be better to stay with the GLFW C API. Therefore, glfwInit should just return true or false (or a general errors.New("GLFW: initialization failed")) and users should have the ability to register their own error callback.

@tapir
Copy link
Member

tapir commented Mar 6, 2015

So, It looks like PlatformError is recoverable after all (glfw/glfw#450)
It's a shame that the documents were at fault. So how many methods are affected by this @slimsag, can you work your magic to find out?

@slimsag
Copy link
Member

slimsag commented Mar 9, 2015

Sorry for the delay on this.

  • It's rather unfortunate that platform errors are indistinguishable. For example:
  • glfwInit fails if a user has broken joystick support (the case here), but the error can be ignored so long as the app doesn't use joysticks.
  • glfwMakeContextCurrent under arbitrary circumstances -- if it does and you proceed blindly, OpenGL will segfault (due to no active context).

I'm not yet convinced that we should return these as errors -- how on earth is the developer supposed to handle them properly aside from just printing them? (sometimes their fatal, other times not).

Leaning towards the idea that these are warnings not errors to be handled, and if that's the case we could just print them to stdout/stderr.

Thoughts?

These functions can generate GLFW_PLATFORM_ERROR:

glfwInit
glfwCreateWindow
glfwPollEvents
glfwWaitEvents
glfwSetWindowTitle
glfwCreateStandardCursor
glfwSetClipboardString
glfwGetClipboardString
glfwTerminate
glfwSetGammaRamp
glfwGetGammaRamp
glfwSetWindowPos
glfwGetWindowFrameSize
glfwGetWindowPos
glfwSetWindowSize
glfwHideWindow
glfwShowWindow
glfwGetWindowAttrib
glfwCreateCursor
glfwDestroyCursor
glfwGetCursorPos
glfwSetCursorPos
glfwSetCursorMode
glfwDestroyWindow
glfwMakeContextCurrent
glfwExtensionSupported

Here's a list of the error messages that are generated (note that some of them have dynamic %s strings, so comparison isn't possible either):

Cocoa: Failed to create application delegate
Cocoa: Failed to create window
Cocoa: Failed to create window delegate
Cocoa: Failed to retrieve object from pasteboard
Cocoa: Monitor mode list changed
EGL: Failed to bind OpenGL ES: %s
EGL: Failed to bind OpenGL: %s
EGL: Failed to create context: %s
EGL: Failed to create window surface: %s
EGL: Failed to find a suitable EGLConfig
EGL: Failed to retrieve visual for EGLConfig
Entry point retrieval is broken
Failed to convert string to UTF-8
Failed to load opengl32.dll
Failed to retrieve context version string
Failed to retrieve display name
Failed to retrieve extension string
Failed to retrieve extension string %i
GLX: Failed to create context
GLX: Failed to find a suitable GLXFBConfig
GLX: Failed to find libGL
GLX: Failed to retrieve visual for GLXFBConfig
Linux: Failed to compile regex
Linux: Failed to initialize inotify: %s
Linux: Failed to open joystick device directory %s: %s
Linux: Failed to watch for joystick connections in %s: %s
Mir: Failed to create event mutex: %s\n
Mir: Requested surface size is to large (%i %i)
Mir: Unable to connect to server: %s
Mir: Unable to create surface: %s
Mir: Unable to find a correct pixel format
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
Mir: Unsupported function %s!
No monitors found
No version found in context version string
NSGL: Failed to create OpenGL context
NSGL: Failed to create OpenGL pixel format
NSGL: Failed to locate OpenGL framework
POSIX: Failed to create context TLS
Wayland: Creating a buffer file for %d B failed: %m\n
Wayland: Cursor mmap failed: %m\n
Wayland: Cursor position setting not supported
Wayland: Failed to compile keymap
Wayland: Failed to connect to display
Wayland: Failed to create XKB state
Wayland: Failed to initialize xkb context
Wayland: GLFW_CURSOR_DISABLED not supported
Wayland: Unable to load default cursor theme\n
Wayland: Unable to load default left pointer\n
Wayland: Unsupported output interface version
Wayland: Window position retrieval not supported
Wayland: Window position setting not supported
WGL: Failed to create OpenGL context
WGL: Failed to enable sharing with specified "
WGL: Failed to find a suitable pixel format
Win32: Failed to allocate global handle for clipboard
Win32: Failed to allocate TLS index
Win32: Failed to convert clipboard string to
Win32: Failed to convert title to wide string
Win32: Failed to convert title to wide string
Win32: Failed to convert wide string to UTF-8
Win32: Failed to create window
Win32: Failed to load winmm.dll
Win32: Failed to load winmm functions
Win32: Failed to open clipboard
Win32: Failed to open clipboard
Win32: Failed to register window class
Win32: Failed to retrieve clipboard data
Win32: Failed to retrieve DC for window
Win32: Failed to retrieve PFD for selected pixel "
Win32: Failed to retrieve shared cursor
Win32: Failed to set selected pixel format
Win32: Failed to set video mode
Win32: Gamma ramp size must be 256
X11: Failed to become owner of the clipboard selection
X11: Failed to create standard cursor
X11: Failed to create window
X11: Failed to query RandR version
X11: Monitor mode list changed
X11: RandR gamma ramp support seems broken
X11: RandR monitor support seems broken

@dmitshur
Copy link
Member

Leaning towards the idea that these are warnings not errors to be handled, and if that's the case we could just print them to stdout/stderr.

Thoughts?

I think that's an improvement over current behavior.

I don't think it's ideal, but I can't propose something better at this time.

If I have better ideas later, I'll make another comment. But we don't need to wait for that, as any improvement is worth taking a step forward and we can always revisit the issue.

Additional Thoughts

Whether something is a warning or an error depends on many factors, I don't think it's a clear cut distinction that we the library makers can make for users. As an example, failed joystick init may be critical if your app uses joysticks or harmless if it doesn't.

I think returning errors that can be classified in some way (ala http://dave.cheney.net/2014/12/24/inspecting-errors) is okay. Errors are values, and while (error)(nil) is most common "it's okay, I can move on" value, there other non-nil errors that are non-fatal. Consider io.EOF error value. So I don't think we must always return nil error if there are some warnings, etc.

@slimsag
Copy link
Member

slimsag commented Mar 10, 2015

@shurcooL I largely agree with your additional thoughts. Problem is that these errors can't be uniquely identified through GLFW, as many of them contain arbitrary strings. They're all just thrown into a "platform error" group.

I'll send a PR for logging the errors next.

@dmitshur
Copy link
Member

Problem is that these errors can't be uniquely identified through GLFW, as many of them contain arbitrary strings. They're all just thrown into a "platform error" group.

Isn't @elmindreda working on improving that, according to glfw/glfw#450 (comment)? I might be reading that incorrectly though. But it seems to me that if it's possible to classify errors, it should be done upstream in GLFW C source, not in our Go wrapper.

@slimsag
Copy link
Member

slimsag commented Mar 10, 2015

I read that as improving the documentation for the GLFW_PLATFORM_ERROR type -- not splitting it up into several different error types that are distinguishable from one-another.

@slimsag
Copy link
Member

slimsag commented Mar 10, 2015

@tux21b We've merged a change that will solve the issue at hand. Please update go get -u github.com/go-gl/glfw and try again.

@elmindreda
Copy link

How about a new error token for these non-critical platform specific errors for 3.2?

@slimsag
Copy link
Member

slimsag commented Mar 10, 2015

@elmindreda having a new token like GLFW_PLATFORM_WARNING for non-critical platform-specific errors would be a big step forward.

That situation would be vastly better, but if we wanted to go the extra mile I would raise the question -- if glfwInit generated a GLFW_PLATFORM_WARNING due to failed joystick initialization: how would the app know joystick init failed as opposed to, say, some other GLFW feature?

@elmindreda
Copy link

@slimsag What would the application do differently if it knew?

@slimsag
Copy link
Member

slimsag commented Mar 10, 2015

@elmindreda if an application critically relies on being able to use the joystick (or another feature), for example, it could display a message to the user that the joystick is not available and the application cannot continue.

@tapir
Copy link
Member

tapir commented Apr 13, 2016

It's been a long time since this issue was reported and I don't remember all the details. GLFW 3.2 is around the corner. I've already started a devel branch. What shall we do about this? Can we now implement it the way we want? I've noticed that official GLFW docs now clearly state the errors that can be caused by the methods. Does it help?

@dmitshur
Copy link
Member

I'm also not sure of what the issue is exactly by now, because so much time has passed.

I think we should close and re-open when someone runs into this and can provide some details and updates. This issue doesn't seem very actionable right now.

I just wanted to point out that there's a seemingly related issue reported in GLFW (the C library) at glfw/glfw#833.

@tapir
Copy link
Member

tapir commented Oct 1, 2016

As suggested, If needed we can reopen it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants