GLFW 3.3 #256

pchampio · 2019-11-02T01:39:05Z

This PR adds support for the latest stable version of GLFW (3.3-stable from github).

I've addressed the issues of #219 :

idiomatic Go function (returns (*Thing, error))
Check for new methods implemented on GLFW side since 3.3 beta - don't merge yet #219 was made. (http://www.glfw.org/docs/3.3/news.html)
Check issue list for fixes postponed until 3.3 (except (*Window).SetIcon with a subimage might fail #235)
Re-base
Make sure vulkan's working. I've ommited Vulkan specific things but we've had reports that they are necessary. Last time I've checked vulkan-go's glfw fork was not so different.

(Not realy sure what to do here) Even if vulkan isn't very polished, important bug fixes, and other features are available.

I've renamed the v3.2 folder to v3.3 and then done the changes, this allows a 'cleaner' PR. I can also make this even cleaner with some filter (removes first commit)
I'll add the v3.2 folder back after everything is green on you side.

GLFW latest tag 3.3-stable glfw/glfw@e4e9581

* Glfw moved away from posix_tls.c to posix_thread.c (This to reflect that they now contain more than TLS.) * Glfw moved away from win32_tls.c to win32_thread.c (same as above) * Glfw removed wayland-.*-unstable-v1-client-protocol.c * Glfw added wl_platform.h * Glfw removed GLFW_USE_RETINA, _GLFW_USE_CHDIR and _GLFW_USE_MENUBAR compile-time macros * Glfw removed support for MIR More info -> https://www.glfw.org/docs/3.3/news.html#removals_33

* Glfw deprecate glfwSetCharModsCallback * Glfw deprecate the window parameter of glfwGetClipboardString and glfwSetClipboardString, NULL can be used as the window argument. * Add glfw.SetClipboardString(string) and glfw.GetClipboardString() function (no Window type required)

Use casting instead of callback in C helpers (same as #219)

COPY/PASTED from: #219 (comment)

#245 #255 #240 #230 #170

In order to compile using docker (cross-compiling linux to darwin). I had to remove some Vulkan flag

pchampio · 2019-11-02T03:01:29Z

I'm pretty sure vulkan isn't working on darwin.
We need to set some compile flag for this platform.
This can be done in another PR!!!

I am runing my tests using one of 'my' project (https://github.com/go-flutter-desktop/hover | https://github.com/go-flutter-desktop/go-flutter).
hover allows for cross compiling, and I can assert that each platform does compile properly.

Here are an example of hover created application using go-gl/glfw v3.3:
windows: windows.zip
linux: linux.zip
macos: darwin.zip (7 days only, wetransfer link)

pchampio · 2019-11-02T03:33:50Z

A tag named 'v3.3/glfw/v0.1.0' needs to be pushed (and updated) for go modules to work.

xlab · 2019-11-04T07:28:16Z

I'm pretty sure vulkan isn't working on darwin.
We need to set some compile flag for this platform.

Hey, any specific error report for that? I'm pretty sure that for darwin it requires just one more framework -framework Metal -framework MoltenVK
https://github.com/vulkan-go/glfw/blob/v3.3/v3.3/glfw/build.go#L19

Is there something beyond that?

xlab · 2019-11-04T07:30:27Z

Amazing work, I hope this will be merged soon. I especially like the concept of having Vulkan in another PR. Let's merge GLFW mainline, then we'll consider how to handle vulkan part in separate PR.

pchampio · 2019-11-04T08:45:44Z

Hey, any specific error report for that? I'm pretty sure that for darwin it requires just one more framework -framework Metal -framework MoltenVK
https://github.com/vulkan-go/glfw/blob/v3.3/v3.3/glfw/build.go#L19

Is there something beyond that?

If I put those flags, then cross-compiling using Docker does not work anymore.

Keep in mind that those LD-flag should only be used when the builg tag 'vulkan' is set (like the README says)

/*
#cgo darwin,vulkan CFLAGS: -D_GLFW_VULKAN_STATIC
#cgo darwin,vulkan LDFLAGS: -F/Library/Frameworks -framework Metal -framework MoltenVK -framework IOSurface -framework QuartzCore
*/
import "C"

TODO (In another PR) port back vulkan functions from vulkan-go@2a913f7

kivutar · 2019-11-11T05:36:14Z

I'm not sure about the #include "glfw/src/linux_joystick.c" in the freebsd build. It gives me errors. Is it something new?

pchampio · 2019-11-11T09:01:30Z

What's your error?
And no it's not something new, the linux_joystick import hasn't changed, it was already here in GLFW 3.2.

If you look at the GLFW imports here the only available files are {cocoa,linux,null,win32}_joystick.*
Freebsd should use the Linux import.

kivutar · 2019-11-12T06:57:36Z

My error is about some missing symbols because of a missing linux specific header. I can't copy the error messages because I'm using VirtualBox.

These lines also suggest that this file is Linux specific: https://github.com/go-gl/glfw/pull/256/files#diff-e4ef75560f509962482af9c7dc7397e5R66-R69

But you're right that this is nothing new, and we may just ignore it for now.

rcoreilly · 2019-11-17T22:00:29Z

What is the status of 3.3 here? I see this PR, and a 3.3-beta branch which just received a commit -- are there two separate 3.3 versions? One way or another, it would be great to get 3.3 finally available after a really long time since the upstream lib was released. Thanks!

pchampio · 2019-11-17T22:22:10Z

Your comment is misleading.
The 3.3-beta branch (#219) hasn't received a any commit since last year. https://github.com/go-gl/glfw/pull/219/commits

As discussed on #219 (comment) and stated in my PR description, I'm taken over #219.

I haven't received any updates from the maintainers of go-gl. I've invested not payed time into this PR (like everyone😑), and I also would like it get merged before upstream lib diverge too much.

rcoreilly · 2019-11-17T22:30:21Z

Sorry I was confused -- I received an email notification about #257 which looked new but apparently it was just closed! And I mistook the name of that for the 3.3-beta branch.. Anyway, I guess the only valid remaining point is: when will this PR be merged!?

pchampio · 2019-11-17T22:40:09Z

I can see why you got confused, I realllllly hope this PR gets merged before the end of the year.
If not, even though I don't like the idea, I will certainly start my own fork 😒.

(I don't want to throw the work I've done, and I don't want to stress out open source maintainers, as I know how hard it can be)

Speaking of which, if you want to add me as a contributor for go-gl/glfw, I will be honored!

kivutar · 2019-11-18T03:09:38Z

@pchampio you may want to take a look at these lines for OSX https://github.com/go-gl/glfw/pull/219/files#diff-c114ff13290eefc1d62b13112e94f1e5R13-R19

I don't know what -D_GLFW_USE_RETINA does exactly, but maybe it would help some people.

You can certainly add -framework Metal safely.

However, since not everybody has -framework MoltenVK, adding this framework would better be done with a build flag. If not it would break go get for users that don't have MoltenVK installed.

I can help you test this branch on OSX at least for OpenGL, as I'm already maintaining my own fork for 3.3. I would prefer to not use a fork.

hajimehoshi · 2019-11-18T13:03:47Z

However, since not everybody has -framework MoltenVK, adding this framework would better be done with a build flag. If not it would break go get for users that don't have MoltenVK installed.

Agreed. MoltenVK should be optional.

pchampio · 2019-11-18T23:45:12Z

@pchampio you may want to take a look at these lines for OSX https://github.com/go-gl/glfw/pull/219/files#diff-c114ff13290eefc1d62b13112e94f1e5R13-R19

I don't know what -D_GLFW_USE_RETINA does exactly, but maybe it would help some people.

I've intentionally removed those line (as described in a643862 commit 😃) because those compile-time macros have been removed in 3.3. https://www.glfw.org/docs/3.3/news.html#removals_33

You can certainly add -framework Metal safely.

It breaks compatibility with MacOSX10.10.sdk.
I'm basing this conclusion on https://github.com/docker/golang-cross.

https://s3.dockerproject.org/darwin/v2/MacOSX10.10.sdk.tar.xz Doesn't not contain Metal.framework
while https://raw.githubusercontent.com/ndeloof/golang-cross/113fix/MacOSX10.11.sdk.tar.xz does.

I don't think -framework Metal is necessary. https://github.com/glfw/glfw/blob/2bac7ee8da526257d808bd026b027246c98e4f2f/CMakeLists.txt#L277-L297
Am I missing something?

frou · 2019-11-19T08:21:18Z

All Macs that can run 10.10 can also run 10.11, so in theory no one is stuck on 10.10

pchampio · 2019-11-19T09:06:40Z

@frou yes, I get it. But is the flag necessary?
And since I am using 'docker/golang-cross', I'm stuck with 10.10.

I'm not against adding this flag, I just want to know why I should add it. As far as I understand, it's not needed.

xlab · 2019-11-19T09:42:42Z

Afaik Vulkan works through Metal, so afaik it's only needed for MoltenVK to work.

pchampio · 2019-11-20T21:15:35Z

Sooo.., Is the PR ready to be reviewed?
Friendly ping @tapir, @pwaller, @dmitshur, @slimsag.

pwaller

@pchampio thanks very much for this effort, and everyone else who chimed in.

I'm low on time, but I have done my best to move all the bits through my eyeballs. It looks good to me, and thanks for your efforts on making it easy to review. I only found a couple of trivial things to comment on.

Moving forward, my intent would be to merge this with the "squash" feature on github so that we don't get the removal/addition of v3.2 in the git history.

I will give a couple of days in case anyone else has any comments, but unless there is a show stopper, I don't see any reason not to merge. I agree with the rationale "Vulkan may not be working, but this is a step forward". Future PRs can fix other things.

If there is no further input on this thread by Saturday 23rd Nov, then I will merge. If I don't merge then, ping the thread and I'll do it by the end of the day on Sunday -- and if I don't respond then, I am inundated, so please feel free to ping the thread weekly or so.

v3.3/glfw/window.go

README.md

pwaller · 2019-11-20T22:14:28Z

v3.3/glfw/GLFW_C_REVISION.txt

@@ -0,0 +1 @@
+e4e9581557826bd522158ab9159859e20e10700e


I note that this choice of revision is after the 3.3 tag.

Is that really what we want? Or do we want to stick to "released" versions? I could be persuaded either way.

On the glfw.org/download webpage, it is stated:

the 3.3-stable branch only adds bug fixes for patch releases.

I'm guessing we want the latest patch available in go-gl/glfw

The previous `SetIcon` go doc was more accurate, reverse the change. Move Changelog bullets points description under the vXX specific changes

pwaller · 2019-11-24T19:51:30Z

Merged. Great work, thanks!

pchampio · 2019-11-25T22:08:25Z

Just for the sake of sharing numbers.
After a very fast investigation, here are some well stared project that rely on go-gl/glfw:

https://github.com/go-flutter-desktop/go-flutter 3.3k 👍
https://github.com/faiface/pixel 2.6k 👍
https://github.com/hajimehoshi/ebiten 2.2k 👍
https://github.com/goki/gi 570 👍
https://github.com/libretro/ludo 230 👍

Thanks to the go-gl team ❤️ !
Putting the 5-6h of work into this PR was well worth-it!

hajimehoshi · 2019-11-26T02:59:39Z

hajimehoshi/ebiten has already switched to 3.3. @pchampio, thank you for your awesome work!

dmitshur · 2019-11-26T03:34:24Z

Thank you @pchampio for authoring this PR, @pwaller for reviewing it, @tapir for creating the original PR #219, and everyone else who contributed to this change! 🎉 ❤️ 🚀

I've sent CL 208877 to update golang.org/x/exp/shiny/driver/mtldriver to use the new GLFW v3.3 library.

kivutar · 2019-11-26T04:25:13Z

libretro/ludo is also switching to 3.3. Thanks to all the people involved in this PR.

rcoreilly · 2019-11-26T04:48:21Z

Yep same with goki/gi and I can already say that it has made things more stable and functional! Thank you!

It has been released recently in go-gl/glfw#256. Start using it.

It has been released recently. Start using it. Updates go-gl/glfw#256

GLFW v3.2 was old and started generating some warnings on macOS 10.15 Catalina (and warnings get upgraded to errors on builders). Update to GLFW v3.3 that was recently released to resolve the problem. There is a small regression affecting the newer version of GLFW that is being tracked in issues glfw/glfw#1543 and go-gl/glfw#262. It causes the application to crash when glfwWaitEvents is called before the first window is opened: $ go run -tags='example metal' .../shiny/example/basic 2019-11-25 22:19:24.715 basic[9412:69556] *** Assertion failure in -[NSApplication run], /BuildRoot/Library/Caches/com.apple.xbs/Sources/AppKit/AppKit-1894.10.126/AppKit.subproj/NSApplication.m:3313 2019-11-25 22:19:24.715 basic[9412:69556] *** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'NSApp with wrong _running count' *** First throw call stack: ( 0 CoreFoundation 0x00007fff30503f53 __exceptionPreprocess + 250 1 libobjc.A.dylib 0x00007fff665c9835 objc_exception_throw + 48 2 CoreFoundation 0x00007fff3051f810 +[NSException raise:format:arguments:] + 88 3 Foundation 0x00007fff32bff5d1 -[NSAssertionHandler handleFailureInMethod:object:file:lineNumber:description:] + 191 4 AppKit 0x00007fff2d657ed3 -[NSApplication run] + 1007 5 basic 0x00000000040d473d _glfwPlatformCreateWindow + 77 6 basic 0x00000000040cd945 glfwCreateWindow + 485 7 basic 0x00000000040da71b _cgo_78603e0816ec_Cfunc_glfwCreateWindow + 43 8 basic 0x0000000004058430 runtime.asmcgocall + 112 ) libc++abi.dylib: terminating with uncaught exception of type NSException SIGABRT: abort PC=0x7fff67a7b49a m=0 sigcode=0 signal arrived during cgo execution [...] Work around it by waiting for the first window open request before entering the main loop. That way, glfwWaitEvents is not called before a window has been created, and the crash does not occur. This temporary workaround can be removed after the bug is fixed. Fixes golang/go#35766 Updates go-gl/glfw#256 Updates glfw/glfw#1543 Updates go-gl/glfw#262 Change-Id: Ie9b15ab6632af39871d94993a3e3097ea798a7e2 Reviewed-on: https://go-review.googlesource.com/c/exp/+/208877 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

fix regression from #256 "`GetClipboardString` No longer returns an e…

pchampio added 9 commits November 1, 2019 20:18

move to GLFW v3.3

4a65e39

GLFW latest tag 3.3-stable glfw/glfw@e4e9581

style: use casting and clang-format c code

b87a17d

Use casting instead of callback in C helpers (same as #219)

update travis

c2ce2ef

fix Darwin build tag

d5a3752

COPY/PASTED from: #219 (comment)

v3.3 implementations

fe48f72

fixes issues postponed until 3.3

a52001e

#245 #255 #240 #230 #170

remove vulkan from build

ecfa213

In order to compile using docker (cross-compiling linux to darwin). I had to remove some Vulkan flag

pchampio mentioned this pull request Nov 2, 2019

3.3 beta - don't merge yet #219

Closed

update README

0ce979d

pchampio mentioned this pull request Nov 3, 2019

go-gl/glfw v3.3 support go-flutter-desktop/go-flutter#298

Merged

chore: push back v3.2

c1dffc6

danfragoso mentioned this pull request Nov 6, 2019

Mustard - Fix input backspace removing two chars danfragoso/thdwb#5

Closed

hajimehoshi mentioned this pull request Nov 8, 2019

Add event API to allow low-level events (e.g. input) hajimehoshi/ebiten#926

Closed

pwaller approved these changes Nov 20, 2019

View reviewed changes

chore: adress PR comments

de1a4d6

The previous `SetIcon` go doc was more accurate, reverse the change. Move Changelog bullets points description under the vXX specific changes

pwaller merged commit e5ec435 into go-gl:master Nov 24, 2019

dmitshur added a commit to go-gl/example that referenced this pull request Nov 26, 2019

update to GLFW v3.3

6ff915f

It has been released recently in go-gl/glfw#256. Start using it.

dmitshur mentioned this pull request Nov 26, 2019

update to GLFW v3.3 go-gl/example#70

Merged

dmitshur added a commit to goxjs/glfw that referenced this pull request Nov 26, 2019

update to GLFW v3.3

d2efb5f

It has been released recently. Start using it. Updates go-gl/glfw#256

xlab mentioned this pull request Mar 10, 2020

Added missing functions for compatibility with Vulkan #273

Merged

pchampio mentioned this pull request Apr 17, 2020

fix regression from #256 "GetClipboardString No longer returns an e… #282

Merged

pwaller added a commit that referenced this pull request Apr 19, 2020

Merge pull request #282 from pchampio/master

9b86c98

fix regression from #256 "`GetClipboardString` No longer returns an e…

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GLFW 3.3 #256

GLFW 3.3 #256

pchampio commented Nov 2, 2019 •

edited

pchampio commented Nov 2, 2019 •

edited

pchampio commented Nov 2, 2019 •

edited

xlab commented Nov 4, 2019

xlab commented Nov 4, 2019

pchampio commented Nov 4, 2019 •

edited

kivutar commented Nov 11, 2019

pchampio commented Nov 11, 2019 •

edited

kivutar commented Nov 12, 2019

rcoreilly commented Nov 17, 2019

pchampio commented Nov 17, 2019 •

edited

rcoreilly commented Nov 17, 2019

pchampio commented Nov 17, 2019 •

edited

kivutar commented Nov 18, 2019

hajimehoshi commented Nov 18, 2019

pchampio commented Nov 18, 2019 •

edited

frou commented Nov 19, 2019

pchampio commented Nov 19, 2019 •

edited

xlab commented Nov 19, 2019

pchampio commented Nov 20, 2019

pwaller left a comment •

edited

pwaller Nov 20, 2019

pchampio Nov 22, 2019

pwaller commented Nov 24, 2019

pchampio commented Nov 25, 2019 •

edited

hajimehoshi commented Nov 26, 2019

dmitshur commented Nov 26, 2019

kivutar commented Nov 26, 2019

rcoreilly commented Nov 26, 2019

		@@ -0,0 +1 @@
		e4e9581557826bd522158ab9159859e20e10700e

GLFW 3.3 #256

GLFW 3.3 #256

Conversation

pchampio commented Nov 2, 2019 • edited

pchampio commented Nov 2, 2019 • edited

pchampio commented Nov 2, 2019 • edited

xlab commented Nov 4, 2019

xlab commented Nov 4, 2019

pchampio commented Nov 4, 2019 • edited

kivutar commented Nov 11, 2019

pchampio commented Nov 11, 2019 • edited

kivutar commented Nov 12, 2019

rcoreilly commented Nov 17, 2019

pchampio commented Nov 17, 2019 • edited

rcoreilly commented Nov 17, 2019

pchampio commented Nov 17, 2019 • edited

kivutar commented Nov 18, 2019

hajimehoshi commented Nov 18, 2019

pchampio commented Nov 18, 2019 • edited

frou commented Nov 19, 2019

pchampio commented Nov 19, 2019 • edited

xlab commented Nov 19, 2019

pchampio commented Nov 20, 2019

pwaller left a comment • edited

Choose a reason for hiding this comment

pwaller Nov 20, 2019

Choose a reason for hiding this comment

pchampio Nov 22, 2019

Choose a reason for hiding this comment

pwaller commented Nov 24, 2019

pchampio commented Nov 25, 2019 • edited

hajimehoshi commented Nov 26, 2019

dmitshur commented Nov 26, 2019

kivutar commented Nov 26, 2019

rcoreilly commented Nov 26, 2019

pchampio commented Nov 2, 2019 •

edited

pchampio commented Nov 2, 2019 •

edited

pchampio commented Nov 2, 2019 •

edited

pchampio commented Nov 4, 2019 •

edited

pchampio commented Nov 11, 2019 •

edited

pchampio commented Nov 17, 2019 •

edited

pchampio commented Nov 17, 2019 •

edited

pchampio commented Nov 18, 2019 •

edited

pchampio commented Nov 19, 2019 •

edited

pwaller left a comment •

edited

pchampio commented Nov 25, 2019 •

edited