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

x/mobile/app: call the Start callback in x11.go #9534

Closed
crawshaw opened this issue Jan 8, 2015 · 13 comments
Closed

x/mobile/app: call the Start callback in x11.go #9534

crawshaw opened this issue Jan 8, 2015 · 13 comments
Milestone

Comments

@crawshaw
Copy link
Contributor

@crawshaw crawshaw commented Jan 8, 2015

I had it in a client but just deleted it by accident. Easy to recreate if you have a working linux desktop.

@crawshaw crawshaw added the repo-mobile label Jan 8, 2015
@crawshaw crawshaw added this to the Go1.5 milestone Jan 8, 2015
@capnm
Copy link

@capnm capnm commented Jan 8, 2015

You should probably call the Stop too.

Maybe something along this (before closing the X11 window):
https://github.com/capnm/mobile/blob/patch-1/app/x11.c#L161
https://github.com/capnm/mobile/blob/patch-1/app/x11.c#L121

@crawshaw
Copy link
Contributor Author

@crawshaw crawshaw commented Jan 8, 2015

If you would like to mail a CL, please do. It will be a few days before I have a working linux machine to get to this.

@db47h
Copy link

@db47h db47h commented Feb 27, 2015

Not calling the start callback breaks the basic example on X11, which segfaults in eglDrawArrays().

@capnm: have you gotten around to submitting a CL? If not, I'll gladly do it.

@crawshaw
Copy link
Contributor Author

@crawshaw crawshaw commented Feb 27, 2015

Please mail a CL, I'll review it.

@db47h
Copy link

@db47h db47h commented Feb 27, 2015

Here you go: https://go-review.googlesource.com/6262 mobile/app: call the Start/Stop callbacks in x11.go

@capnm
Copy link

@capnm capnm commented Mar 1, 2015

@capnm: have you gotten around to submitting a CL? If not, I'll gladly do it.

@db47h Thanks for doing this. I missed the bug notification and deleted the cloned archive,
FWIW here is a copy: https://gist.github.com/capnm/9de3a3aab7bd1acdd13d

An another issue I had, was that the current event handling eats without a vsync 100% cpu.
I don't have currently the time to setup things and submit a CL myself, if you'd like, you can
take a look what I did and go ahead :-)
https://gist.github.com/capnm/9de3a3aab7bd1acdd13d#file-x11-c-L194

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Mar 1, 2015

Yeah, I think I saw vsync's ignored on intel but not nvidia, on linux. Or maybe vice versa, I forget. It could be a mesa bug but I don't know. I intend to investigate but it hasn't been high-priority for me.

@db47h
Copy link

@db47h db47h commented Mar 2, 2015

Well, it used to be a PITA to get VSync working at all on Linux. It does work at least with the proprietary nvidia drivers. Programmers cannot however expect that enabling VSync will have any effect. This applies to all OSes: if the user has globally disabled VSync in the driver settings, enabling it on the application side will do nothing. IMHO, the best approach to this is to try to enable it, but never expect it to be enabled, and in any case, this should not be dealt with in the mobile/app package since this is related to application timing (where requirements differ greatly from one app to another). #9413 seems to be a good place to debate about this.

@capnm I opened a new issue against the X11 event-loop in #10054 where your rewrite of the event loop looks to be a good fix. I will however leave the VSync related stuff and frame clamping out of it ;)

@tbruyelle
Copy link
Contributor

@tbruyelle tbruyelle commented Mar 2, 2015

Thanks for the fix, I can use the start callback now.

That said, I noticed geom.Width and geom.Height are not initialized when start is invoked. Is this fixable ?

@capnm
Copy link

@capnm capnm commented Mar 3, 2015

I propose to add an "onResize" callback. I did that for desktop, but could be probably used for setting glViewport (for landscape/portrait mode) on mobile too.

@db47h
Copy link

@db47h db47h commented Mar 3, 2015

We definitely need more event notifications or callbacks as well as a hook into the underlying windowing system, at least to set window properties (size, fullscreen, enable vsync based on user preferences, etc). If I can run a game in full HD on my Nexus, why not on my desktop? ;)

@rsc rsc changed the title mobile/app: call the Start callback in x11.go x/mobile/app: call the Start callback in x11.go Apr 14, 2015
@rsc rsc removed the repo-mobile label Apr 14, 2015
@rsc rsc modified the milestones: Unreleased, Go1.5 Apr 26, 2015
@crawshaw
Copy link
Contributor Author

@crawshaw crawshaw commented Jul 13, 2015

I believe this issue, as described, is now obsolete. There are however some other good points that have been mentioned. I believe the X11 port doesn't have all the lifecycle events plumbed through, and we have no notion of window size hinting or explicit resizing events. These are probably worth their own issues, if anyone feels like filing them.

@db47h
Copy link

@db47h db47h commented Jul 14, 2015

Yes, should have been closed a while back (my fault, I messed up the Fixes line in the CL). Regarding the missing X11 events, I'll give it another go after the 1.5 release.

@crawshaw crawshaw closed this Jul 14, 2015
@golang golang locked and limited conversation to collaborators Jul 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.