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

Adding MacOS and IOS support.. #45

Closed
oscarbg opened this issue Mar 7, 2018 · 14 comments · Fixed by #97
Closed

Adding MacOS and IOS support.. #45

oscarbg opened this issue Mar 7, 2018 · 14 comments · Fixed by #97

Comments

@oscarbg
Copy link

oscarbg commented Mar 7, 2018

Hi..
I'm providing a diff file vs current master adding MacOS and IOS support..

maciosupport.diff.zip

Really adding Mac only support is very simple.. once adding "macx" platform pro custom libs and defines only a fix is needed if we don't want surface info:
it's a matter of adding and if(surface) on readSurfaceInfo as we haven't created one..

then for surface info we need the code in #ifdef GETSURFACEINFO (which also adds one mm file for creating a CAMetalLayer for view).. note this view has to be removed eventually for app being rendered correctly.. (this has been helpful : KhronosGroup/MoltenVK#78)
the code for that is different from MacOS or IOS so a BUILD_FOR_MAC define..
for MacOS works nice.. for IOS well the code is adapted from the link which has been recently shared how to use right on Qt+MoltenVK+IOS.. right now code now doesn't work (doesn't create surface correctly MoltenVK says error with the view passed), but I believe or it's already correct and MoltenVK need to be updated accordingly or some minor fix is still needed.. I'm asking on that thread so perhaps MoltenVK can gave a look and fix it..

I know there should be a builtin definition to difference platforms (IOS vs MacOS) but I use mine (BUILD_FOR_MAC)..
also in #idef BUILD_FOR_MAC diff you can find some copy&paste code from Internet to save json on folder with permissions on IOS, with that IOS program also is able to also send report to DB..

for .pro file I'm unable to use correctly platform settings although from quick read should be macx and ios so I keep commenting as needed although should be as clean as:

macx{
    #LIBS += /lib/libvulkan.1.dylib -framework Cocoa -framework QuartzCore
    LIBS += /Library/Frameworks/vulkan.framework/Versions/A/vulkan -framework Cocoa -framework QuartzCore
    DEFINES +=  VK_USE_PLATFORM_MACOS_MVK GETSURFACEINFO BUILD_FOR_MAC

}
ios
{
LIBS +=/Users/oscarbarenysgarcia/Downloads/vulkansdk-macos-1.0.69.0/MoltenVK/iOS/MoltenVK.framework/MoltenVK -framework UIKit -framework IOSurface -framework Metal
  DEFINES +=  VK_USE_PLATFORM_IOS_MVK GETSURFACEINFO
}

on MacOS I comment #LIBS += /lib/libvulkan.1.dylib although is the correct loader ICD so should be independent of newer versions of MoltenVK lib and also this loader should be updatable provided user updates file on this location..
there is a reason.. code compiles nice but for debugging from Qt Creator doesn't work due to @rpath issues..
of course for redistribution you should use that commented line..
for fixing the @rpath you should use the install_name_tool detailed below..

other problem is releasing as a redistributable executable for Macos for people without Qt SDK..
some quick notes.. for including needed libs in executable you should do something like:

$QTDIR/bin/macdeployqt /Users/oscarbarenysgarcia/build-vulkanCapsViewer-Desktop_Qt_5_10_1_clang_64bit-Release/Win32/Release/vulkanCapsViewer.app -executable=/Users/oscarbarenysgarcia/build-vulkanCapsViewer-Desktop_Qt_5_10_1_clang_64bit-Release/Win32/Release/vulkanCapsViewer.app/Contents/MacOS/vulkanCapsViewer

This copies but doesn’t fix references to libraries inside the app folder so I created a script and run:

./installfwkchange.sh QtCore
./installfwkchange.sh QtGui
./installfwkchange.sh QtWidgets
./installfwkchange.sh QtNetwork

Script is like:
install_name_tool -change @rpath/$1.framework/Versions/5/$1 @executable_path/../Frameworks/$1.framework/Versions/5/$1 /Users/oscarbarenysgarcia/build-vulkanCapsViewer-Desktop_Qt_5_10_1_clang_64bit-Release/Win32/Release/vulkanCapsViewer.app/Contents/MacOS/vulkanCapsViewer

I don't remember if libqcocoa if copied by macdeployqt if not have to locate plugins/platforms..

then you need to fix included libqcocoa.dylib in app frameworks folder:
You need to fix also libqcocoa.dylib:

install_name_tool -change @rpath/QtGui.framework/Versions/5/QtGui @executable_path/../Frameworks/QtGui.framework/Versions/5/QtGui /Users/oscarbarenysgarcia/build-vulkanCapsViewer-Desktop_Qt_5_10_1_clang_64bit-Release/Win32/Release/vulkanCapsViewer.app/Contents/MacOS/qt_plugins/platforms/libqcocoa.dylib

install_name_tool -change @rpath/QtCore.framework/Versions/5/QtCore @executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore /Users/oscarbarenysgarcia/build-vulkanCapsViewer-Desktop_Qt_5_10_1_clang_64bit-Release/Win32/Release/vulkanCapsViewer.app/Contents/MacOS/qt_plugins/platforms/libqcocoa.dylib

install_name_tool -change @rpath/QtWidgets.framework/Versions/5/QtWidgets @executable_path/../Frameworks/QtWidgets.framework/Versions/5/QtWidgets /Users/oscarbarenysgarcia/build-vulkanCapsViewer-Desktop_Qt_5_10_1_clang_64bit-Release/Win32/Release/vulkanCapsViewer.app/Contents/MacOS/qt_plugins/platforms/libqcocoa.dylib

install_name_tool -change @rpath/QtPrintSupport.framework/Versions/5/QtPrintSupport @executable_path/../Frameworks/QtPrintSupport.framework/Versions/5/QtPrintSupport /Users/oscarbarenysgarcia/build-vulkanCapsViewer-Desktop_Qt_5_10_1_clang_64bit-Release/Win32/Release/vulkanCapsViewer.app/Contents/MacOS/qt_plugins/platforms/libqcocoa.dylib

To have icon file your ico file is converted to icns
https://findicons.com/convert ico to icns online
Generates 3 files I use the largest 256
Copy to resources folder..

in your info.plist add
CFBundleIconFile
iconfile
with icon file iconfile.icns in your Resources directory

phew..
feel free to ask questions you need..

@SaschaWillems
Copy link
Owner

Thx for all your effort. Will take a look at this once I get a bit more time and see how I can incorporate this into the current build.

@oscarbg
Copy link
Author

oscarbg commented Mar 10, 2018

no pressure.. moltenvk also doesn't evolve so fast..

@ikryukov
Copy link

Thanks @oscarbg! I tried MoltenVK with Qt and found another solution for rpath resolving:
put lib to app bundle and use QMAKE_RPATHDIR
here details: https://github.com/ikryukov/QtVulkan/blob/master/QtVulkan.pro

@SaschaWillems
Copy link
Owner

SaschaWillems commented Oct 3, 2020

I'm currently testing Mac OSX CI builds, but I don't own a Mac OSX device myself. Can some one with such a test the following version: https://vulkan.gpuinfo.org/downloads/Vulkan_Caps_Viewer-osx-x86_64.dmg

And see if this actually works? It was build using Travis CI, and if this works fine there'll be automated updated builds for the Vulkan hardware capability viewer for Mac OSX too :)

@oscarbg
Copy link
Author

oscarbg commented Oct 3, 2020

Hi,
yes it works!
Save report works and even I uploaded a report correctly:
http://vulkan.gpuinfo.org/displayreport.php?id=9626

waiting to test on new MoltenVK release next week with Vulkan 1.1 support..
EDIT: also waiting to see reports of Mac ARMs (A14X GPU) + MoltenVK ..

only problems (minor issues) I see:

  1. Icon not present..

  2. Surface tab: Could not get a valid surface, no surface information available!

  3. I get a crash just after sending report but report is sent correctly..
    will share crash info once I upload a new report for new MoltenVK..

good work.. thanks!
Captura de pantalla 2020-10-03 a las 18 26 16

@SaschaWillems
Copy link
Owner

Thank you very much for the quick response. So the Mac OSX BI build is actually the way to go :)

The bugs and errors are to be expected, as this was just a quick test to see if this is actually feasible. I expect that there are missing code paths for Mac OSX, e.g. for the surface info. Will try to fix that.

@oscarbg
Copy link
Author

oscarbg commented Oct 3, 2020

Hi,
just updated first Vulkan 1.1 MacOS report using MoltenVK from CrossOver 20.0 beta 3 which seems to use modified one from Vulkan SDK 1.2.154:
http://vulkan.gpuinfo.org/displayreport.php?id=9630

@SaschaWillems
Copy link
Owner

Awesome. That report also contains properties and features for the new portability extension. 👍

I also just added code to retrieve surface information on Mac OSX. Can you re-download (same link as above), and check if surface information is present?

@didito
Copy link

didito commented Oct 4, 2020

Hi,

I've uploaded reports for both of my MBP (Mid 2012) GPUs, running macOS Mojave 10.14.6.
Still no app icon, and surface tab says "Could not get a valid, no surface information available!" but does not crash after uploading reports.

https://vulkan.gpuinfo.org/displayreport.php?id=9637
https://vulkan.gpuinfo.org/displayreport.php?id=9638

@SaschaWillems
Copy link
Owner

Thank you very much for the uploads and reporting back on the surface and info. So it looks that I can't just pass the winid for surface creation as e.g. on Linux. @oscarbg did some more complex stuff to make the view Metal-compatible in his patch, so I'll try to incorporate that.

@SaschaWillems
Copy link
Owner

I have tried to incorporate the surface related changes for Mac OSX from the diff provided @oscarbg, but can't test myself. If one of you could download the recent build (same link as above), and see if surface information is now properly read, that would be great.

@oscarbg
Copy link
Author

oscarbg commented Oct 6, 2020

yep.. latest version now reports correctly surface information!

Captura de pantalla 2020-10-06 a las 7 16 59

with that, guess you add the icon, and you can already publish it!

@didito
Copy link

didito commented Oct 6, 2020

Yes, works for me too.

@SaschaWillems
Copy link
Owner

Great to hear that it's pretty much feature complete in OSX now 👍

I have also added an OSX icon set, so the latest build should also contain a proper icon.

Will merge the changes, so that from now on we also always have up-to-date Mac OSX builds :)

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 a pull request may close this issue.

4 participants