ClassiCube icon indicating copy to clipboard operation
ClassiCube copied to clipboard

[WIP] macOS: Kill AGL and Carbon, fix FreeType, use SDL2 for 64-bit bliss

Open easyaspi314 opened this issue 5 years ago • 24 comments

To Apple, 32-bit is dead. All macOS versions since Lion require a 64-bit processor, and Catalina won't even execute a 32-bit binary.

With the death of 32-bit also comes Carbon. Carbon was never released for 64-bit, and was only included for backwards compatibility.

Not only that, but the Carbon backend was completely glitchy.

Let's get rid of them and use the functional SDL2 backend on macOS. We can also remove AGL because SDL2 handles OpenGL.

I also fixed FreeType. It now works on unmodified FreeType 2.10 from Homebrew, and if macOS is the only reason to include the entire source tree, kill it too, now there is MUCH less code to maintain.

make SYSTEM_FREETYPE=1

should compile against system FreeType. I haven't tested other platforms, but I can compile a seemingly functional build with that for macOS.

I also fixed a few Makefile issues, by the way.

The build bot needs to be updated, and there are still some deprecated APIs I don't like using.

easyaspi314 avatar Aug 08 '19 20:08 easyaspi314

I also removed the GetCurrentProcess hack, removing a dependency on a deprecated ApplicationServices API. Everything seems to work, but I don't exactly understand the problem it is trying to solve.

easyaspi314 avatar Aug 08 '19 20:08 easyaspi314

As for a modernization checklist, this is what I am planning:

  • [x] Remove Carbon
  • [x] 64-bit builds
  • [x] Remove ApplicationServices
  • [x] Remove FreeType hack
  • [ ] Remove ucontext.h stuff
  • [ ] Standalone .dpkg + .app packaging instead of an unconventional raw binary. We can then package the libraries there.
  • [ ] Move config stuff into ~/Library/Application Support/ClassiCube/. We should do similar for the other platforms (%AppData%\ClassiCube\, ~/.config/ClassiCube/). The path of the program might not be writable, and .app programs probably shouldn't write to themselves.
  • [ ] Remove provided FreeType in favor of vanilla upstream if we have no specific reason to use it
  • [ ] No deprecated API usage except for OpenGL.
  • [ ] Clean with zero warnings

easyaspi314 avatar Aug 08 '19 21:08 easyaspi314

GetCurrentProcess/TransformProcessType are needed so dialog boxes work. From memory an app bundle is already foreground though.

FreeType is included so windows/osx exes don't have require any dlls to work

Not sure why you'd want to remove ucontext.h. needed for dumping registers in crashes

The game writes to its own folder deliberately so it's portable app

How is the carbon backend completely glitchy?

SDL seems to be limited to osx10.5+ intel only which isn't really great, since I was intending to add support for tiger. And it would mean 10.5 powerpc support would get broken too On Aug 9, 2019 7:57 AM, "easyaspi314 (Devin)" [email protected] wrote:

As for a modernization checklist, this is what I am planning:

  • Remove Carbon
  • 64-bit builds
  • Remove ApplicationServices
  • Remove FreeType hack
  • Remove ucontext.h stuff
  • Standalone .dpkg + .app packaging instead of an unconventional raw binary. We can then package the libraries there.
  • Move config stuff into ~/Library/Application Support/ClassiCube/. We should do similar for the other platforms (%AppData%\ClassiCube, ~/.config/ClassiCube/). The path of the program might not be writable, and .app programs probably shouldn't write to themselves.
  • Remove provided FreeType in favor of vanilla upstream if we have no specific reason to use it
  • No deprecated API usage except for OpenGL.
  • Clean with zero warnings

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/UnknownShadow200/ClassiCube/pull/599?email_source=notifications&email_token=ABRVGJFJAFCNGUGKUIS5EXTQDSJDDA5CNFSM4IKOKZHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD35AOVA#issuecomment-519702356, or mute the thread https://github.com/notifications/unsubscribe-auth/ABRVGJGF2U77RYW4C6KOLQDQDSJDDANCNFSM4IKOKZHA .

UnknownShadow200 avatar Aug 08 '19 23:08 UnknownShadow200

GetCurrentProcess/TransformProcessType are needed so dialog boxes work. From memory an app bundle is already foreground though.

hmm. I guess I still don't understand. I launch the program from the Terminal and it focuses properly. :thinking:

FreeType is included so windows/osx exes don't have require any dlls to work

I guess you haven't heard of static libraries. All the libraries required for this are either system-provided or able to be statically linked.

Homebrew has provided all the static libraries for making this app: ClassiCube.zip

(I managed to get the path working, it will now write into Application Support)

On macOS, applications are folders that are just treated like files in Finder. That is why they are always distributed as .dpkg, .pkg, or .zip. The only time you see a standalone executable is when it is a command line tool.

ClassiCube.app/
 -> Contents/
 ---> Info.plist
 ---> PkgInfo
 ---> Frameworks/
 -----> libSDL2.dylib
 -----> libpng.dylib
 -----> libfreetype.dylib
 ---> MacOS/
 -----> ClassiCube
 ---> Resources/
 -----> CCicon.icns

Not sure why you'd want to remove ucontext.h. needed for dumping registers in crashes

Ok, fair point. It is a deprecated API, but I guess that is a useful feature

The game writes to its own folder deliberately so it's portable app

Well that is not like any macOS applications I have seen. All of them are bundled into a .app folder and usually write into ~/Library/Application Support.

How is the carbon backend completely glitchy?

You keep moving to the side.

SDL seems to be limited to osx10.5+ intel only which isn't really great, since I was intending to add support for tiger. And it would mean 10.5 powerpc support would get broken too

Are you nuts?

Unlike Windows or Linux, Mac/iOS doesn't have a huge fragmentation problem.

https://gs.statcounter.com/macos-version-market-share/desktop/worldwide

< 2% of all Macs online are running either Catalina or versions less than 10.9. And I presume that 99% of those are Catalina testers.

Unless your hardware is truly incompatible (and dosdude1 hasn't made a patch), if you are on 10.6 or later, you can upgrade to 10.8+ for free, and you will generally find most macOS applications only supporting 10.9 or 10.10.

My changes would aim to support 10.7+, which was released in 2011.

It is the trolley problem. Would you kill 1 old Mac to save 50 million and make the code more maintainable?

easyaspi314 avatar Aug 09 '19 02:08 easyaspi314

I support using an .app package if possible, since it's a lot easier for users to deal with. Made this a while ago but it doesn't work because it's trying to write to the current directory. https://cdn.discordapp.com/attachments/549750423314235444/575375474407112704/ClassiCube.zip

buildist avatar Aug 09 '19 03:08 buildist

The app focuses fine, it's dialogs with Window_ShowDialog that have the problems.

I have heard of static libraries - the version of freetype included in the source code has been stripped of functionality not used to reduce the final exe size to below 1 mb.

Hmmm. Not sure why it would get stuck moving to the side, haven't seen that issue occur myself or had reports from others.

My thinking is from you should be able to run the game from a usb without it leaving files behind on the system.

One of my primary goals is to support as many OS versions as possible (heck the windows build even works on 98 with KernelEX). Increasing minimum OS version is not really acceptable to me.

How I planned to handle this was to add a cocoa backend that is used for a 64 bit osx build once apple finalised a release date. No need to drop any compatibility nor would catalina users miss out. On Aug 9, 2019 12:59 PM, "easyaspi314 (Devin)" [email protected] wrote:

GetCurrentProcess/TransformProcessType are needed so dialog boxes work. From memory an app bundle is already foreground though.

hmm. I guess I still don't understand. I launch the program from the Terminal and it focuses properly. 🤔

FreeType is included so windows/osx exes don't have require any dlls to work

I guess you haven't heard of static libraries https://en.wikipedia.org/wiki/Static_library. All the libraries required for this are either system-provided or able to be statically linked.

Homebrew has provided all the static libraries for making this app: ClassiCube.zip https://github.com/UnknownShadow200/ClassiCube/files/3484487/ClassiCube.zip

(I managed to get the path working, it will now write into Application Support)

On macOS, applications are folders that are just treated like files in Finder. That is why they are always distributed as .dpkg, .pkg, or .zip. The only time you see a standalone executable is when it is a command line tool.

ClassiCube.app/

-> Contents/

---> Info.plist

---> PkgInfo

---> Frameworks/

-----> libSDL2.dylib

-----> libpng.dylib

-----> libfreetype.dylib

---> MacOS/

-----> ClassiCube

---> Resources/

-----> CCicon.icns

Not sure why you'd want to remove ucontext.h. needed for dumping registers in crashes

Ok, fair point. It is a deprecated API, but I guess that is a useful feature

The game writes to its own folder deliberately so it's portable app

Well that is not like any macOS applications I have seen. All of them are bundled into a .app folder and usually write into ~/Library/Application Support.

How is the carbon backend completely glitchy?

You keep moving to the side.

SDL seems to be limited to osx10.5+ intel only which isn't really great, since I was intending to add support for tiger. And it would mean 10.5 powerpc support would get broken too

Are you nuts?

Unlike Windows or Linux, Mac/iOS doesn't have a huge fragmentation problem.

https://gs.statcounter.com/macos-version-market-share/desktop/worldwide

< 2% of all Macs online are running either Catalina or versions less than 10.9. And I presume that 99% of those are Catalina testers.

Unless your hardware is truly incompatible (and dosdude1 hasn't made a patch), if you are on 10.6 or later, you can upgrade to 10.8+ for free, and you will generally find most macOS applications only supporting 10.9 or 10.10.

My changes would aim to support 10.7+, which was released in 2011.

It is the trolley problem https://en.wikipedia.org/wiki/Trolley_problem. Would you kill 1 old Mac to save 50 million and make the code more maintainable?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/UnknownShadow200/ClassiCube/pull/599?email_source=notifications&email_token=ABRVGJBLFFKHNI47J2UM7JTQDTMRPA5CNFSM4IKOKZHKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD35OIVY#issuecomment-519758935, or mute the thread https://github.com/notifications/unsubscribe-auth/ABRVGJGYAH32NDKFN2JMRW3QDTMRPANCNFSM4IKOKZHA .

UnknownShadow200 avatar Aug 09 '19 04:08 UnknownShadow200

@buildist did you try my zip? That has the Application Support path and uses SDL.

easyaspi314 avatar Aug 09 '19 13:08 easyaspi314

Well, I found the movement issue in the Carbon backend. It was because I had my MacBook in the Romaji layout (I have hiragana mode mapped to Caps lock).

Apparently, when you have your keyboard set to Romaji mode (I presume others as well), there are two kEventRawKeyDown events fired instead of one. The first has the key code and the second has a zero. This maps to 'A', which is the 'move left' command by default.

The way we detect these is by checking kEventParamKeyboardType. On these "fake" key events, when we try to get this parameter, we get an error, so we can ignore them.

--- a/src/Window.c
+++ b/src/Window.c
@@ -1559,10 +1559,18 @@ static OSStatus Window_ProcessKeyboardEvent(EventRef inEvent) {
                case kEventRawKeyDown:
                case kEventRawKeyRepeat:
                case kEventRawKeyUp:
+                       res = GetEventParameter(inEvent, kEventParamKeyboardType, typeUInt32,
+                                                                       NULL, sizeof(UInt32), NULL, &code);
+                       if (res) {
+                               /* In some languages, kEventRawKeyDown fires twice with a 'fake' 0 code.
+                                * This causes a bug when a Mac is set to the Romaji layout, as it will
+                                * cause 'a' to be held down.
+                                * The way we detect this is if there is an error with KeyboardType. */
+                                 return 0;
+                       }
                        res = GetEventParameter(inEvent, kEventParamKeyCode, typeUInt32,
                                                                        NULL, sizeof(UInt32), NULL, &code);
                        if (res) Logger_Abort2(res, "Getting key button");

                        key = Window_MapKey(code);
                        if (!key) { Platform_Log1("Ignoring unmapped key %i", &code); return 0; }

easyaspi314 avatar Aug 10 '19 00:08 easyaspi314

Can confirm I see the same issue when I use japanese keyboard layout with Hiragana mode, and that your changes fix the problem. Would appreciate a separate pull request with that fix.

UnknownShadow200 avatar Aug 10 '19 11:08 UnknownShadow200

Screen Shot 2019-08-10 at 10 07 17 PM

Mac on Mac action.

Host: MacBook Pro (15-inch, early 2011) / MacBookPro8,2 macOS Mojave 10.14.2 2.0GHz Intel Core i7-2635QM 8 GB RAM Intel "HD" 3000 Integrated Graphics (borked dGPU :frowning:) qemu-system-ppc 4.1.0-rc4

Guest: Mac OS X 10.4.11 Tiger (PowerPC32) Simulated Power G4 7400 AGP graphics 1 GB RAM

SDK Xcode 2.2.1, powerpc-apple-darwin8-gcc-4.0.1, against 10.4u SDK

I have to disable getcontext (it was added in 10.5) and use -DCC_BUILD_GL11. I get a few messages in stderr:

MenuBar won't work!
Failed to create full-screen pixel format
Trying to create a non-fullscreen pixel format.

I haven't tried networking because libcurl is incredibly old and I don't trust it.

Can't judge performance because it is qemu without acceleration on integrated graphics.

I'll make a PR that adds the previous thing and the patch to fix tiger. Then we need to move to either cocoa (not ez) or sdl2 (ez) for a proper 64-bit version.

easyaspi314 avatar Aug 11 '19 02:08 easyaspi314

Hmm, I was using 10.5 sdk with -mmacosx-version-min=10.4, and there was a getcontext import in the output binary.. no idea why that worked.

I'll just kill off getcontext entirely. Register dump doesn't really matter for Logger_Abort anyways.

UnknownShadow200 avatar Aug 11 '19 03:08 UnknownShadow200

Yeah, that is what GDB is for.

easyaspi314 avatar Aug 11 '19 04:08 easyaspi314

You’d be killing PowerPC (32 bit) support by getting rid of Carbon. For those who aren’t aware, I consider ClassiCube the Crysis for our PowerPC Macs. I get it that Apple no longer supports 32 bit, but think for a second that ClassiCube is meant to run on the most systems possible.

RetroSoftwareRepository avatar Feb 09 '21 23:02 RetroSoftwareRepository

Are you nuts?

If they are nuts, then I am completed and certifiably insane.

I was just about to add ClassiCube to leopard.sh.

cellularmitosis avatar Aug 29 '23 23:08 cellularmitosis

I would actually prefer to get rid of the Carbon backend now that there is a dedicated Cocoa backend (although if I recall, I still need to fixup a few things so the Cocoa backend works on really old macOS versions)

UnknownShadow200 avatar Aug 30 '23 13:08 UnknownShadow200

I'll give it a try!

Thanks so much for keeping the OpenGL 1.1 implementation alive!

https://www.youtube.com/watch?v=qDaVipOLr6E

cellularmitosis avatar Aug 30 '23 14:08 cellularmitosis

Just got Leopard to build sans Carbon using the following patch:

macuser@pmacg5(leopard)$ diff -urN classicube-1.3.6.orig classicube-1.3.6
diff -urN classicube-1.3.6.orig/Makefile classicube-1.3.6/Makefile
--- classicube-1.3.6.orig/Makefile	2023-08-28 17:52:14.000000000 -0500
+++ classicube-1.3.6/Makefile	2023-08-30 21:31:34.000000000 -0500
@@ -16,6 +16,8 @@
 	ifeq ($(PLAT),darwin)
 		ifeq ($(shell uname -m), x86_64)
 			PLAT=mac_x64
+		else ifeq ($(shell uname -m), Power Macintosh)
+			PLAT=mac_ppc
 		else
 			PLAT=mac_x32
 		endif
@@ -59,6 +61,13 @@
 LDFLAGS=-rdynamic -framework Cocoa -framework OpenGL -framework IOKit -lobjc
 endif
 
+ifeq ($(PLAT),mac_ppc)
+OBJECTS+=src/interop_cocoa.o
+CFLAGS=-std=c99 -g -pipe -fno-math-errno
+LIBS=
+LDFLAGS=-framework Cocoa -framework OpenGL -framework IOKit -framework OpenAL -lobjc
+endif
+
 ifeq ($(PLAT),freebsd)
 CFLAGS=-g -pipe -I /usr/local/include -fno-math-errno
 LDFLAGS=-L /usr/local/lib -rdynamic
@@ -124,6 +133,8 @@
 	$(MAKE) $(ENAME) PLAT=mac_x32
 mac_x64:
 	$(MAKE) $(ENAME) PLAT=mac_x64
+mac_ppc:
+	$(MAKE) $(ENAME) PLAT=mac_ppc
 freebsd:
 	$(MAKE) $(ENAME) PLAT=freebsd
 openbsd:
diff -urN classicube-1.3.6.orig/src/Core.h classicube-1.3.6/src/Core.h
--- classicube-1.3.6.orig/src/Core.h	2023-08-28 17:52:14.000000000 -0500
+++ classicube-1.3.6/src/Core.h	2023-08-30 21:30:29.000000000 -0500
@@ -176,11 +176,12 @@
 	#define CC_BUILD_IOS
 	#define CC_BUILD_TOUCH
 	#define CC_BUILD_CFNETWORK
-	#elif defined __x86_64__ || defined __arm64__
+	#elif defined __x86_64__ || defined __arm64__ || defined __ppc__
 	#define CC_BUILD_COCOA
 	#define CC_BUILD_MACOS
 	#define CC_BUILD_CURL
 	#else
+	#error
 	#define CC_BUILD_CARBON
 	#define CC_BUILD_MACOS
 	#define CC_BUILD_CURL
make CC=gcc-4.9

Caveat: I didn't link against a modern libcurl in this build.

cellularmitosis avatar Aug 31 '23 02:08 cellularmitosis

Tiger also builds as well, the one additional patch:

macuser@imacg52(tiger)$ diff -urN src/interop_cocoa.m.orig src/interop_cocoa.m
--- src/interop_cocoa.m.orig	2023-08-28 17:52:14.000000000 -0500
+++ src/interop_cocoa.m	2023-08-30 21:50:09.000000000 -0500
@@ -7,6 +7,11 @@
 #include <Cocoa/Cocoa.h>
 #include <ApplicationServices/ApplicationServices.h>
 
+#ifndef CGFloat
+/* CGFloat was introduced in Leopard. */
+typedef float CGFloat;
+#endif
+
 static int windowX, windowY;
 static NSApplication* appHandle;
 static NSWindow* winHandle;

I built and tested both GL1.5 and GL1.1 on Tiger.

cellularmitosis avatar Aug 31 '23 02:08 cellularmitosis

Ah, actually that's not right, typedefs don't work with ifdef that way. Should be this instead:

#include <AvailabilityMacros.h>
#if !defined(MAC_OS_X_VERSION_10_5)
typedef float CGFloat;
#endif

cellularmitosis avatar Aug 31 '23 02:08 cellularmitosis

I went with an alternative approach in 0a4f26c296dd6bc7a2e245e3fc24610ba77b7430 so that the cocoa backend can be compiled all the way back to Panther

My main concern with completely switching to Cocoa was ensuring that the potentially different OpenGL context selection did not result in any performance impact - and although I didn't see any difference on my mac mini, I am unable to test much older macOS versions on actual hardware to verify that

UnknownShadow200 avatar Sep 03 '23 13:09 UnknownShadow200

Is there any sort of repeatable performance benchmark in classicube, similar to a “timedemo” from quake 3?

I could use that to compare the performance of carbon vs cocoa OpenGL contexts on a few older machines (e.g. iMac G3)

cellularmitosis avatar Sep 03 '23 20:09 cellularmitosis

There isn't a formal test suite due to it so rarely being needed, but I will write up some instructions on what I do when comparing performance between two builds

I still need to fixup fullscreen in the cocoa build for older macOS versions first though

UnknownShadow200 avatar Sep 05 '23 13:09 UnknownShadow200

I still need to fixup fullscreen in the cocoa build for older macOS versions first though

Whoops, forgot I already fixed that

Is there any sort of repeatable performance benchmark in classicube, similar to a “timedemo” from quake 3?

I could use that to compare the performance of carbon vs cocoa OpenGL contexts on a few older machines (e.g. iMac G3)

In this case, since the goal is to compare performance by rendering as many frames as possible, what I would probably do is:

  • Escape > Options > Misc options, turn off Block Physics
  • Escape > Options > Graphics options, change the FPS Mode to LimitNone
  • Escape > Generate new level > Flatgrass

Then without moving around or moving the camera at all, measure average FPS over some time (this should minimise the other factors that could influence performance)

UnknownShadow200 avatar Sep 07 '23 13:09 UnknownShadow200

@cellularmitosis Sorry to bother you, but is this something you still plan on testing?

UnknownShadow200 avatar Nov 06 '23 11:11 UnknownShadow200