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

Remove QTKit support from wxMediaCtrl for OS X #337

Merged
merged 1 commit into from Oct 19, 2016

Conversation

TcT2k
Copy link
Contributor

@TcT2k TcT2k commented Oct 18, 2016

QTKit has been removed from OS X 10.12 SDK.
QTKit has been superseded by AVFoundation since OS X 10.7.
Since wxWidgets >= 3.1 requires 10.7 anyway there is no reason to support the old API.

Additionally the AVFoundation implementation may use AVKit (available since 10.9). It will be used when available and the deployment target is met.

See #336

QTKit has been removed from OS X 10.12 SDK.
QTKit has been superseded by AVFoundation since OS X 10.7.
Since wxWidgets >= 3.1 requires 10.7 anyway there is no reason to support the old API.

Additionally the AVFoundation implementation may use AVKit (available since 10.9). It will be used when available and the deployment target is met.
@vadz vadz merged commit 065135a into wxWidgets:master Oct 19, 2016
@vadz
Copy link
Contributor

vadz commented Oct 19, 2016

Thanks!

@TcT2k TcT2k deleted the osx_remove_qtkit branch October 19, 2016 12:15
@mojca
Copy link
Contributor

mojca commented Oct 20, 2016

Does this mean that trunk now only supports OS X 10.9 and newer and that support for < 10.9 will be/has been dropped?

@TcT2k
Copy link
Contributor Author

TcT2k commented Oct 20, 2016

No 10.7 is still the supported. Only if you build with a deployment target >= 10.9 AVKit will additionally be used.

@mojca
Copy link
Contributor

mojca commented Oct 20, 2016

OK, but I'm confused about one thing then. In b98cbab hundreds of lines were removed that were there to support QTKit. Does AVFOUNDATION without AVKit provide all the functionality that was there in QTKit then?

@mojca
Copy link
Contributor

mojca commented Oct 20, 2016

I would like to backport some of your patches to version 3.0 (which is supposed to work on 10.5+). I guess I can cherry-pick some of your commits and apply them on top of the 3.0 branch. But given that you've been working on this code for the past few days, perhaps you would be willing to look into backporting your patches to the 3.0 branch (without removal of QTKit)? I need to provide a working version of wxWidgets 3.0.x to MacPorts users and wouldn't mind "outsourcing" some effort :)

@TcT2k
Copy link
Contributor Author

TcT2k commented Oct 20, 2016

I'll try to look into the backport in the next few days (but no promises).
The base for the backport should be 2816101 as it still contains the QTKit support, but is buildable on the 10.12 SDK.

@TcT2k
Copy link
Contributor Author

TcT2k commented Oct 21, 2016

@mojca see #341

@mojca
Copy link
Contributor

mojca commented Dec 12, 2016

I don't want to highjack pull requests, I can open a new ticket, but just to mention here that the build of the latest master (c70abf2) on 10.7 fails for me with: ld: framework not found AVKit.

@TcT2k
Copy link
Contributor Author

TcT2k commented Dec 12, 2016

That should have been fixed by 7684f92
I tested with 10.12 SDK and 10.7 deployment target. Maybe you could post the configure.log somewhere.

@mojca
Copy link
Contributor

mojca commented Dec 12, 2016

The relevant line in output of configure is

checking if AVKit is availble... checking CXXWARNINGS for gcc -Woverloaded-virtual... -Woverloaded-virtual

and config.log says

configure:35239: checking if AVKit is availble
configure:35258: /usr/bin/clang -c -pipe -Os -arch x86_64 -x objective-c++ -I/opt/local/include -I/opt/local/include conftest.c >&5
configure:35258: $? = 0
configure:35447: checking CXXWARNINGS for gcc -Woverloaded-virtual
configure:35474: /usr/bin/clang++ -c -pipe -Os -stdlib=libstdc++ -arch x86_64 -pedantic -Werror  -Woverloaded-virtual -I/opt/local/include -I/opt/local/include conftest.cpp >&5
configure:35474: $? = 0
configure:35488: result: -Woverloaded-virtual
configure:35500: : CXXWARNINGS="$CXXWARNINGS $ac_cv_cxxflags_gcc_option__Woverloaded_virtual"
configure:35503: $? = 0

Maybe there's a minor bug in configure.in and the grouping is not right?

The problem is that MAC_OS_X_VERSION_10_9 is not declared when compiling on 10.7. And for some reason

#if MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_9

resolves to true. This could potentially work better:

#if defined(MAC_OS_X_VERSION_10_9) && MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_9

Also, shouldn't the test in configure[.in] also check whether the linker flag -framework AVKit actually works and try to add #import <AVKit/AVKit.h> to includes? And the AC_TRY_COMPILE function should also print something to the screen. It looks to me as if the complete wxWidgets is almost built already, it's just that a "broken" flag is added in one of the latest steps.

I can try to find a better solution and open a PR, or open ticket on Trac in case I run out of ideas.

@TcT2k
Copy link
Contributor Author

TcT2k commented Dec 12, 2016

I think I figured out the issue, please see #370 for a fix.
Please test this with 10.7 SDK, but I'm pretty certain that the issue should be fixed with the additional check.

vadz pushed a commit that referenced this pull request Dec 12, 2016
This fixes the check for older SDKs and outputs the check result.

This is a follow up to #337 and the
improvement of #342.

Closes #370
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.

None yet

3 participants