-
Notifications
You must be signed in to change notification settings - Fork 97
Minor tweaks for OSX build #90
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
Conversation
…r to osx build (NTSC filter works, however other filters seems to just pass through the original image regardless of scale).
|
Shouldn't snesfilter stay disabled if 7 out of 8 filters don't actually work? |
|
You're probably right. I'll edit the snesfilter Makefile to only build the filter that works on OSX. It's the neatest filter of the bunch so it's kind of a shame to omit it! |
|
Done. |
|
Cool, merged it. On a related note, I'm going to be doing a small bugfix release this week, and it might be kind of nice to have actual OS X build(s) to go along with it. I know @MatthewCallis had emailed me about possibly providing one after I released v3 but I don't think he has replied to my last email to him yet. |
|
I can make the builds, no problem. I even added a 'distribution' build target so I could archive builds for myself, which I believe was merged into mainline a good while back... I/we should find some users on older OSX versions to make sure the build works on at least a couple of versions back. Maybe we should also add the Qt libraries to the app bundle for release builds? Or maybe "lowering the barrier to entry" would just attract the wrong crowd. |
|
@devinacker I saw your email, sorry I got distracted by other things and haven't replied yet 😓 @Optiroc I can make/test builds on 10.9 but I think that is as far back as I have access to. |
|
@MatthewCallis no worries! I didn't want to rush you or anything, I just thought it might be good to get both OS X build conversations in one place. @Optiroc I think including the libraries is obviously the right choice (ideally just run Anyway, I'm going to copy some stuff I mentioned to Matthew previously just for continued reference:
|
|
I just submitted a new PR with a build script for distributable bundles. It builds separate (zipped) bundles for accuracy and compatibility. Here's the output from the current commit: @MatthewCallis Can you test these app bundles on a mac running something prior to 10.12? It should run on 10.10 and above. |
|
Also, I'll take a look at the bsxdat issue during the weekend. |
|
On second thought, it might be better to just put bsxdat alongside the app bundle(s) rather than inside them; that solves the issue of having to mess with the working directory at runtime, and keeps it simple for people to put more broadcast data in there later without having to navigate the bundle. |
|
Working directory is quite a hairy thing on OSX... When launching bsnes+ from Xcode it's set to the path of the bundle (which is what we'd want in this case), but when launching from Finder it seems to be set to "/". Not great! A solution could be to set the default bsxdat path to |
|
No, strike that one, nall::currentpath() just calls getcwd() which gives the same result. |
|
I made it work but had to resort to use CoreFoundation, which brought some name clashing trouble. Most severely, it typedefs "Style" so I had to rename the Style struct in ui-base – which is referenced all over the place. To get the path I added nall::string launchpath(), which could be put to good use for getting the initial file browsing path as well. On OSX you're currently thrown into "/". |
|
Regarding the snesfilter issues on macOS, I realized that the other filters didn't work because OpenMP isn't being linked. That was easy enough to remedy by compiling just snesreader with gcc. |
|
I had a feeling it was OpenMP-related (since that was the reason it was disabled in macOS builds in the first place). I'd still prefer to be able to link it when building with clang/xcode, but who knows when that'll finally become supported... Anyway, the 073+3a minor release is now up. I'm probably going to hold off on merging any more mac-related pull requests for right now, but if you (or anyone else) wants to handle an official macOS build then feel free to make any other changes on your end that you think are necessary. |
|
Yeah. I edited snesreader to compile with gcc with openmp on osx if available, and then include all filters. Otherwise it fall backs to only include the ntsc filter. Anyway, I'll let that stay in my fork until I've sorted out proper builds. I think the bigger problem is the Sierra patched Qt4 frameworks, that don't seem to work on older osx versions. Having different builds for <=10.11 and >=10.12 would be rather awful, so I'll do some poking around with solving that... I'll let you know if I'm able to get proper distribution builds working. Porting to Qt5 is not tremendously viable, I suppose? :) |
|
Knowing next to nothing about Qt I thought Qt5 was a radical change... and then I got bsnes+ running with two code changes (excluding includes and Makefiles). So technically it seems transitioning to Qt5 isn't an issue, but maybe there's other reasons for not moving to it? For macOS support it would be a big plus. |
|
Qt5 is definitely doable; I've just been sticking with Qt4 due to it being relatively lightweight compared to Qt5, but updating seems pretty inevitable. I've already long since started using Qt5 for all of my other projects anyway. The issue with Qt 4 on Sierra is pretty troubling, though. I agree that having separate builds isn't really an ideal solution. |
|
The only one of the filters that depends on OpenMP is hq2x. "The NTSC filter works but all the others pass through the image unmodified" is what's expected to happen on an accuracy build, because the accuracy PPU always renders at high resolution (512 pixel width) and the non-NTSC filters only work on low resolution (256x224) input. |
|
@awjackson Ah, thanks. I had indeed accuracy set as default target when testing the filters. Will disable only hq2x for openmp-less builds instead. For accuracy builds it's maybe best to omit filters all together. Or introduce another ifdef-path that compiles only the NTSC filter. In other news I have deployable Qt5 mac builds up and running now. A nice bonus in addition to it being supported for current osx versions is that the PPU debugger screens are rendered correctly. |
|
Do the official Qt 5.7 releases have any issues out of the box with 10.12? The "Supported Platforms" list only includes 10.8 through 10.11. If not, then it sounds like these builds would be good for an official release. |
|
Yup. I'll do some more testing on different setups first, but I think it's pretty much good to go! |
|
I've had some friends try this build out on a couple of different setups and I believe it's ready to go: bsnes_v073+3a_osx.zip (I haven't been able to find a machine with 10.9, but it's compiled with -mmacosx-version-min=10.9 without warnings so I reckon it should work) |
Simplified building by automatically adding version string to Info.plist while building.
I also re-enabled snesfilter on the OSX build. NTSC filter works (and it's the best one!), but the other filters seems to just pass through the original image regardless of scale.