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

[OSX] wxGLCanvas resize fix on macOS 10.14.5 #1354

Closed
wants to merge 2 commits into from

Conversation

laptabrok
Copy link
Contributor

@laptabrok laptabrok commented Jun 13, 2019

Related ticket: https://trac.wxwidgets.org/ticket/18402

I honestly can't explain why this works, but it does. Maybe some bugs with multithreading in Apple macOS SDK .
Checked in macOS 10.14.5 and macOS 10.14.5 with Xcode 10.2.1.

@vadz
Copy link
Contributor

vadz commented Jun 13, 2019

Thanks for the fix but this is indeed pretty mysterious (I wonder how did you find it?).

Apple docs say that the default implementation calls NSOpenGLContext::update, which we already do ourselves in wxGLContext::SetCurrent() so it's really puzzling that doing this again breaks something. But if it fixes the problem, we probably need to apply it even if we don't understand how it works (and I hate this).

Could somebody please test this under earlier macOS versions? I.e. I'd like to be sure that this doesn't break things under < 10.14. TIA!

P.S. I guess we could also call the base class method inside a run-time version check, if we're not sure about the effects on the previous versions.

@laptabrok
Copy link
Contributor Author

"P.S. I guess we could also call the base class method inside a run-time version check, if we're not sure about the effects on the previous versions."
The fact is that call [super update] breaks resize again on 10.14.5 (did not try this call on < 10.14.5), so i left the method empty.

@vadz
Copy link
Contributor

vadz commented Jun 13, 2019

Just to be clear, I meant to call [super update] only under macOS < 10.14. AFAICS if we do this, we don't risk breaking anything.

@valid-ptr
Copy link
Contributor

PR + wxWidgets compiled with MacOSX 10.14 SDK (Xcode 10.1) on 10.13.6 (the latest available system for) Mac mini Server (Mid 2011) => works.

Just to be clear, I meant to call [super update] only under macOS < 10.14. AFAICS if we do this, we don't risk breaking anything.

Maybe MacOSX SDK should be checked also? ;)

@vadz
Copy link
Contributor

vadz commented Jun 28, 2019

Thanks again @laptabrok for the patch and @valid-ptr for testing, I'm finally going to apply this, sorry for the delay.

@vadz vadz closed this in ea68934 Jun 28, 2019
@vadz
Copy link
Contributor

vadz commented Aug 20, 2019

Please test #1492 which will replace this PR if no problems are found with it. TIA!

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

Successfully merging this pull request may close these issues.

3 participants