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

compare OpenGL contexts with == instead of isEqual #1100

Closed
wants to merge 1 commit into from
Closed

compare OpenGL contexts with == instead of isEqual #1100

wants to merge 1 commit into from

Conversation

glowcoil
Copy link

I don't entirely understand why, but starting with a recent update to macOS, is_current has started returning false even when the OpenGL context is in fact current, tripping this assert in Glium (see this issue; I experienced it in a project I was working on as well). However, this seemed to fix it.

@goddessfreya
Copy link
Contributor

Hey, does this solve the same problem as #1099? If so, which PR do you and @yvt think is better?

@yvt
Copy link

yvt commented Feb 17, 2019

Both PRs fix the issue in the same way except for the difference in the ways of checking equality — pointer comparison (this PR) or - isEqual: (#1099 and master).

I think they make no difference in 99.99% of use cases, but might break in the other rare cases. Pointer comparison is in general more fragile against changes in the internal implementation. For example, since NSOpenGLContext is a wrapper of a low-level object, it's possible that currentContext could return a different object for the same underlying object.

Imagine the case where you have a NSOpenGLContext and for some reason you activate it by directly calling CGLSetCurrentContext. Now, what would currentContext be supposed to do? (a) It could maintain a global hash table that stores mappings to the original NSOpenGLContext. But then it would require a process-global critical section. Furthermore, it would still have to return nil for contexts not created via NSOpenGLContext, thus deviating from the documented behavior. (b) Or, it could simply construct a new NSOpenGLContext pointing the same CGLContextObject, which behaves exactly the same as the original object, except for its pointer identity.

@glowcoil
Copy link
Author

glowcoil commented Feb 17, 2019

Yeah, I'll concur that @yvt's is the more correct of the two.

Unfortunate that the original mistake wasn't able to be caught by the type system.

@goddessfreya
Copy link
Contributor

#1099 has been merged into #1058 in favor of this PR, as you both seam to agree it's the more correct of the two. Thanks folks!

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

Successfully merging this pull request may close these issues.

3 participants