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

Fix OpenGL VSync #533

Merged
merged 1 commit into from Jun 28, 2014
Merged

Fix OpenGL VSync #533

merged 1 commit into from Jun 28, 2014

Conversation

Anti-Ultimate
Copy link
Contributor

Hopefully this also works on Mac and Linux

The problem was that SwapInterval was always set to null and never changed. Because of that the error message popped up.

@delroth
Copy link
Member

delroth commented Jun 24, 2014

@Sonicadvance1 shouldn't GLExtensions be fetching the address of this function?

@dolphin-emu-bot rebuild

@Sonicadvance1
Copy link
Contributor

We grab this extension down in MakeCurrent.
Which this is how it is supposed to be done because Windows regenerates it's function tables each time MakeCurrent is called.
True that GLExtensions should really just be grabbing this function pointer, and when we switch to libepoxy this will be fixed fully there.

@JMC47
Copy link
Contributor

JMC47 commented Jun 26, 2014

Should this be merged in the meantime? Or should we just wait for the libepoxy?

@Sonicadvance1
Copy link
Contributor

We already grab the function pointer. I don't know why it is NULL on Windows still.
This is an incorrect fix.

@Anti-Ultimate
Copy link
Contributor Author

Should it be grabbed in ClearCurrent? It isn't grabbed in MakeCurrent as far as I can tell by the code.

@Sonicadvance1
Copy link
Contributor

Must have been an error that it ended up in ClearCurrent, it was supposed to be in MakeCurrent.
ClearCurrent is only called at the very end of OpenGL shutdown.

This is a proper fix
@dolphin-emu-bot rebuild

@BhaaLseN
Copy link
Member

Just rebase out those commits. git rebase -i master (replace master with whatever your upstream source is), then remove the two lines from the editor that opens up. The only thing that should remain is "pick a12c98d".
Alternately, create a new branch and git cherry-pick a12c98d1bc, then replace the old one by pushing to your remote branch with the same name as referenced in this PR.

@Anti-Ultimate
Copy link
Contributor Author

I'd like to do that, but the text file just says noop

@BhaaLseN
Copy link
Member

Oh, right, your branch is called master. Try something like origin/master (or whatever your remote is called). HEAD~3 should also work (as its 3 commits back you'd want to edit)

@Anti-Ultimate
Copy link
Contributor Author

There, done. Goddamnit Git is hard

@BhaaLseN
Copy link
Member

Its not (really) hard, you just have to get familiar with it. You (c|sh)ould also fix the commit message (git commit --amend) as it looks a bit...odd.

@Anti-Ultimate
Copy link
Contributor Author

hold on, that doesn't say anything either.

@Anti-Ultimate
Copy link
Contributor Author

Okay now

@JMC47
Copy link
Contributor

JMC47 commented Jun 27, 2014

Can someone check this? vsync hasn't worked in OGL in a good, well, 10 years (more like 1500 builds, but whatev) according to my exaggerated estimates.

@Anti-Ultimate
Copy link
Contributor Author

I don't want to put any more strain onto the buildbot so here you go:
https://www.dropbox.com/s/wtrybkbf653q9ud/oglvsyncfix.7z

@Sonicadvance1
Copy link
Contributor

@dolphin-emu-bot rebuild

Sonicadvance1 added a commit that referenced this pull request Jun 28, 2014
@Sonicadvance1 Sonicadvance1 merged commit aae1630 into dolphin-emu:master Jun 28, 2014
@Nucleoprotein
Copy link

I wrote about this issue in ClearCurrent/MakeCurrent a few months ago: https://code.google.com/p/dolphin-emu/issues/detail?id=7169#c10
Thanks for fixing this.

@CrossVR
Copy link
Contributor

CrossVR commented Jun 29, 2014

Hopefully this also works on Mac and Linux

@Anti-Ultimate As a side note: Since your PR only affected WGL, which is the Windows OpenGL interface, it will not affect any other OS.

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