Fix LWJGL2 crashes due to incomplete arm64 patches (OptiFine on 1.17.10, etc.)

[rinOfTheStars]

See: r58Playz/lwjgl2-m1@2320dbd

[ViRb3]

Many thanks for linking this project. I took a look at the code changes and I can confirm that they work to fix OptiFine on 1.17.10, but unfortunately, I don't think that this is the way to go. As far as my limited understanding goes, forcing all these functions to render on the main thread will incur an unnecessary performance penalty, and that will apply across all Minecraft versions. This is not a good thing to force upon people who don't play 1.7.10 and below, and those who don't use OptiFine. I think the fix would need to be applied to OptiFine instead, so it behaves properly even on these older versions.

[r58Playz]

Can we add this as an option in the launcher? Like "Use LWJGL2 that has been patched for Optifine"?

EDIT:
Also, this fix is only going to be on versions that use LWJGL2 (1.12 and below).

[ViRb3]

I would really prefer to find a proper solution to the problem instead of confusing people with more versions of libraries. I reached out to one of the OptiFine moderators and they said that they are willing to look at the problem and potentially release fixed versions, but they are currently busy. Will keep this issue updated as soon as I know more.

[ViRb3]

@r58Playz Could you please join the community Discord (https://discord.gg/CcFxPaDnjv) and message me (same username)? I would like to discuss the LWJGL2 patches a bit more with you. Many thanks.

[sp614x]

@r58Playz what exactly is the bugfix doing and can it be integrated in OptiFine?

[ViRb3]

@sp614x I rebased @r58Playz's patches (minus the HiDPI stuff) on top of upstream lwjgl2 here: https://github.com/MinecraftMachina/lwjgl. You can check the last two commits for the exact changes. It appears that a bunch of drawing methods have been explicitly synchronized to run on the main thread. I believe this specific line is what fixes Optifine. However, there's more to the picture.

Until now I was using tanmayb123's build of lwjgl2 in ManyMC, which I assumed was just vanilla lwjgl2 compiled for arm64. Since Minecraft 1.7.10 ran and Optifine crashed, I thought that Optifine is simply drawing on the wrong thread and needs to be updated for that. However, after compiling lwjgl2 for arm64 myself without any synchronization patches, it turns out that even vanilla 1.7.10 crashes on boot. I decompiled tanmayb123's lwjgl2 and surely enough, it also includes some of the synchronization patches.

At this point I am inclined to believe that the fix should go in lwjgl2 and not Optifine, but if somebody here is more versed in how safe and/or performance degrading these changes are, please let me know.

[sp614x]

I can check if there are some methods called outside of the main thread.
What are the problematic methods in the patch, so far I see lockFocus, resizeWindow , wasResized, update and setView?
What would be the corresponding Java methods for these?

[JJTech0130]

So this is a problem with LWJGL2's native component on macOS? Why does it work on x86-x64? Does Mojang patch it, or does threading/synchronization work differently?
(We had none of these issues on Linux ARM64 iirc)

[ViRb3]

After enduring excruciating pain debugging this issue, I have concluded that this is absolutely NOT a problem in OptiFine.

MacOS provides an SDK with XCode, which includes platform-specific bindings and implementations for various frameworks. Up until SDK 10.15, an implementation was included for both NSView, and CALayer rendering in JAWT. Unfortunately, in the header NSView is clearly marked as deprecated, and surely enough, in SDK 11.0 and above, the NSView implementation is gone. As you probably guessed, this is what LWJGL2 uses, so compiling it on M1 which only supports SDK 12+ becomes impossible. @r58Playz's initial patches and @tanmayb123's patches I used in ManyMC's LWJGL2 attempt to bring back this renderer. I reckon the new NSView system-wide renderer is now async and this is why the crashes started happening. These patches work around the problem by forcefully synchronizing critical methods from LWJGL2, but they missed some used by OptiFine. Even with the latest patches which fix OptiFine, there's no guarantee that more LWJGL2 methods won't be broken and unspotted just because they so happen to be unused.

The next steps here would be to correctly re-implement the NSView renderer without the whack-a-mole approach. Any takers? :)

[JJTech0130]

So, they both patched much more than just those functions, and the synchronization patches were because of bugs in their reimplementations?

[ViRb3]

No, the patches above do not re-implement anything. I'm not too sure about the specifics, but it appears that the NSView renderer was only hidden from the new SDK by removing the structure that exposes it. The patches above simply re-introduce that old structure. This leads to the synchronization crashes, so the patches manually synchronize each call that causes a crash.

Ideally, we should move away from the NSView renderer. It might get completely removed or even more broken with any MacOS update...

[JJTech0130]

Ah… I see. Is NSView tightly coupled to LWJGL? How different is CALayer? How hard would it be to adapt it? Can we take code from LWJGL3?

[r58Playz]

It seems that LWJGL has already made the switch to CALayer and we are compiling an older version.

See this line.
Edit: My lwjgl source has the CALayer backend as well. Maybe LWJGL isn't triggering it?

[ViRb3]

Upon further investigation, I can confirm @r58Playz you're right. LWJGL2 actually supports both backends and automatically chooses CALayer unless Java 1.7 is used. I tested by removing all support for NSView mode, leaving only CALayer, and rendering works exactly like before. Sadly, the synchronization patches are still necessary. It appears that rendering on main thread was not enforced until MacOS 10.15, according to this reference. I believe that this change came in the MacOS SDK 10.15, though, as I tested LWJGL2 compiled on SDK 10.8 and it runs even on MacOS 12.

I wonder why Minecraft 1.8+ does not run into this crash. @sp614x would you be able to help us track down any rendering changes in Minecraft 1.8.9 that may be synchronizing the rendering code? Since the same LWJGL2 library is used, the "magic" must be happening in plain Minecraft Java code.

[ViRb3]

Alright, after some careful consideration, I decided that the synchronization patches are the best choice we have right now. I slightly improved the code by checking whether the thread is not main first, avoiding overhead on 1.8.9+. These changes now live in https://github.com/MinecraftMachina/lwjgl, and I have updated ManyMC to use this version of LWJGL2. So, OptiFine 1.7.10 and before now works on ManyMC. Still, any thoughts here are very welcome. @r58Playz many thanks for your help to get this working!

[sp614x]

@ViRb3 how exactly is it crashing (stack trace, crash report)?
Is there a specific MC/OF version (snapshot) that starts crashing (or stops crashing)?

[ViRb3]

@r58Playz Can I please ask for some details on your High DPI patches? For convenience, I isolated them here. You appear to be disabling High DPI support on NSWindow level via setWantsBestResolutionOpenGLSurface:NO, but at the same time, you are setting the Java code's enableHighDPI to true and multiplying some widths and heights. In my tests, merely disabling High DPI support via the first method is enough and fixes the windowed mode scaling issues on all versions of Minecraft. What is more, with only this patch, fullscreen works on MC 1.12.2, but if I add your patches, it starts covering half the screen. Sadly, neither of these patches work on fullscreen on MC 1.11 and below, not sure why.

[r58Playz]

I was testing multiple ways of fixing fullscreen mode and I forgot to clean up.
I'll remove everything except the NSWindow's disable High DPI.

Thanks for letting me know. I completely forgot I committed the Java code changes!

EDIT: It seems that the scaling issue is still there. I am using build 7 of ManyMC.
EDIT 2: I am running Minecraft Java 1.12.2 with Azul Zulu Java v1.8.0_275 for arm64.

In comparison (m1-multimc-hack):
Minecraft Java 1.12.2 with Azul Zulu Java v1.8.0_275 for arm64

[ViRb3]

@r58Playz can you please check your logs and ensure you're using LWJGL2-mmachina.2, like so:

Native libraries:
...
  /Users/user/Library/Application Support/ManyMC/libraries/org/lwjgl/lwjgl/lwjgl-platform/2.9.4-nightly-20150209-mmachina.2/lwjgl-platform-2.9.4-nightly-20150209-mmachina.2-natives-osx.jar

Maybe post logs actually? Works wonders here on both external normal DPI and internal high DPI monitor:

P.S. If you use Discord, please join the ManyMC guild, it will be much faster to test there. Happy to also add you on Telegram, etc.

[ViRb3]

@sp614x After a lot of experimentation, it seems like 1.8.9 is actually the only single LWJGL2 release that doesn't crash, even within snapshots. Maybe this was actually a bug, lol. Here's a graphic of what I tried, the instances with a red X are what crashed:

Regarding the crash, here's full logs, I hope you can see something helpful:

https://paste.gg/p/anonymous/08075dd2cec74bddb7043fa706b1d7eb

Maybe we need to diff 1.8.9 with the snapshot right before or after it (15w49b or 15w49a)? They were released on the same day, so hopefully would be minimal changes.

[r58Playz]

I checked my logs and I'm using mmachina.1. How do I update?

@sp614x After a lot of experimentation, it seems like 1.8.9 is actually the only single LWJGL2 release that doesn't crash, even within snapshots. Maybe this was actually a bug, lol. Here's a graphic of what I tried, the instances with a red X are what crashed:

Regarding the crash, here's full logs, I hope you can see something helpful:

https://paste.gg/p/anonymous/08075dd2cec74bddb7043fa706b1d7eb

Maybe we need to diff 1.8.9 with the snapshot right before or it (15w49b or 15w49a)? They were released on the same day, so hopefully would be minimal changes.

We need to run [NSOpenGLContext setView] from the main thread.

[ViRb3]

@r58Playz if you restart ManyMC, it should pick up the new update automatically, although that depends on how long GitHub caches the manifest for. In my experience, it takes seconds for an update to propagate, so I'm not sure what's wrong in your case. Maybe try deleting ~/Library/Application Support/ManyMC/libraries?

Regarding the crash, yes, I am completely aware that we need to run setView from main thread, but we are trying to figure out what MC 1.8.9 does different which already runs everything necessary on the main thread :) We may want to copy that approach instead of the current hack.