FEX-Emu
ThunkLibs: Add support for GLX/GL without thunking X11, thunked as FGL
Overview
This includes only the required bits from #1735 to support GLX thunking without X11 thunks.
The GL linking fixes in #1734 are not included.
A new entry is added to ThunksDB.json, "FGL" that does not depend on X11. The same library is used.
GL only thunk in nvidia AGX ORIN
Rationale
Our X11 thunks are (very) minimal. This allows us to thunk GL for most applications,
Depends
#1805 For custom hostcall support
Details
See #1735
Follow Up
- Support for EGL, GLESv2, GLESv1
~For the thunked xcb + unthunked libX11 case, https://xcb.freedesktop.org/MixingCalls/, XGetXCBConnection & XOpenDisplay might offer a solution there.~ It was an issue with the hostcall support, after all.
Change this so we have libGL thunk and libFGL so the two implementations can live side by side and I don't see any reason why it can't be tested at the same time.
We don't need to disrupt the previous implementation while this new path is being tested.
They are the same code, the mode of operation is detected by whenever X11 is thunked or not. FGL only exists as a json alias.
Except that it is changing a wackload of code from passthrough to custom implementations. Which I'm not sure if we agree is the right path forward yet. Which is why having the two implementations side-by-side allows easier testing without breaking one or the other.
Only GLX is affected, and sadly, due to how libGL works on linux, GLX is also in libGL, not only libGLX.
So to keep separate impls, we'd have to also keep two libGLs with is easily 100x GLX in entrypoints
(The original PR /did/ have two libs, this is the rewrite that merged them for less code)
I'm fine with more code to keep the implementations separate, or if you want to save some code duplication add some GL_Common folder or something then have each implementation inline a .inl file from that folder. Just for code that is identical.
As a general remark, I'll quote my comment from #1734:
As mentioned in other channels, what I need to review your ThunkLibs PRs is better separation of changes and documentation. The bare minimum needed is a rudimentary explanation of what problem each of the patches solves. Otherwise, we have no solid basis for discussion and I won't be able to gauge how the changes will impact my ongoing work.
I won't repeat another review like https://github.com/FEX-Emu/FEX/pull/1809 where I had to spend a full week until I understood what you're doing and actually got it into a working state. Since I'm continuing to drive forward the bigger-scheme thunking work, I don't have capacity to carry early-state PRs out of prototype state.
As for this PR, I don't know if we critically need to merge it right now since you haven't outlined your specific future plans about it. Currently, I see no drawback from leaving this unmerged until some specific later date so we can continue evaluating limitations of "regular" GL thunking. This will save us unnecessary refactoring and feature creep in the generator which would come back and haunt us at a later point. If the chosen date passes or if we hit a hard blocker in the regular GL thunks, we can still integrate FGL at that point.
As for how to integrate FGL, @Sonicadvance1's proposal sounds reasonable. Since this is more of a stopgap solution it makes sense to minimize code sharing between FGL and the other thunk libraries. (If we find regular-GL hits a blocker it would be preferable to have stronger integration, but again that's something we would be in a better position to decide later.) The separation must be strong enough not to overlap with any of my generator changes, and it must be possible to safely make changes to one code path without having to retest the other all the time. The last thing we need is two separate configurations where at least one is constantly broken.
Finally, I need to stress that until #1611 is merged, I'm not in a position where I can review generator or build system changes unless they are clearly communicated and well-motivated. This includes massively-copy-pasted code that would need to be updated manually if changes to the generator are made, as such generator changes are still happening frequently.
I'm fine with more code to keep the implementations separate, or if you want to save some code duplication add some GL_Common folder or something then have each implementation inline a
.inlfile from that folder. Just for code that is identical.
Sure, we can go this way. I just don't want to duplicate the code all over.
As a general remark, I'll quote my comment from #1734:
As mentioned in other channels, what I need to review your ThunkLibs PRs is better separation of changes and documentation. The bare minimum needed is a rudimentary explanation of what problem each of the patches solves. Otherwise, we have no solid basis for discussion and I won't be able to gauge how the changes will impact my ongoing work.
One can read code too, apart from documentation, I presume. I've offered endless time to discuss/call/text w/e about it.
I won't repeat another review like #1809 where I had to spend a full week until I understood what you're doing and actually got it into a working state. Since I'm continuing to drive forward the bigger-scheme thunking work, I don't have capacity to carry early-state PRs out of prototype state.
Yeah, so instead these have been pending for months now. Seems an improvement.
As for this PR, I don't know if we critically need to merge it right now since you haven't outlined your specific future plans about it.
That's your opinion, and I strongly disagree there. I have explained during several past meetups, and over discord, exactly why this is needed and is blocking progress. Last week I even offer to discuss how you want it structured, and I do the work.
Currently, I see no drawback from leaving this unmerged until some specific later date so we can continue evaluating limitations of "regular" GL thunking.
This has nothing to do with GL, and only with GLX.
This will save us unnecessary refactoring and feature creep in the generator which would come back and haunt us at a later point. If the chosen date passes or if we hit a hard blocker in the regular GL thunks, we can still integrate FGL at that point.
What feature creep? Nothing is special about this.
If you think the gen can get too complex, someone else can also work on it or refactor it.
As for how to integrate FGL, @Sonicadvance1's proposal sounds reasonable. Since this is more of a stopgap solution it makes sense to minimize code sharing between FGL and the other thunk libraries. (If we find regular-GL hits a blocker it would be preferable to have stronger integration, but again that's something we would be in a better position to decide later.)
Again, this is not about GL, but GLX.
The separation must be strong enough not to overlap with any of my generator changes, and it must be possible to safely make changes to one code path without having to retest the other all the time. The last thing we need is two separate configurations where at least one is constantly broken.
My view here is, the generator needs to serve the needs of the project, not the other way around.
Finally, I need to stress that until #1611 is merged, I'm not in a position where I can review generator or build system changes unless they are clearly communicated and well-motivated. This includes massively-copy-pasted code that would need to be updated manually if changes to the generator are made, as such generator changes are still happening frequently.
That's fine, we discussed yesterday with @Sonicadvance1 and he'll be taking over the review on this one, from what I understand.
After today's discussion,
- This will not affect the non-FGL path in general, and users should take note that things may break with FGL.
- Apart from getting thunks up-and-running fast, this will enable differential debugging (X11 thunks on vs X11 thunks off)
- It will be much easier to do passthrough GL + FGLX rather than X11 thunking for 32-bits
I see nothing has moved on this still.
Yes, as I've been blocked doing noop work on other tickets, and of course this now has to be needlessly rebased, and there's been no progress in resolving disagreements in https://github.com/FEX-Emu/FEX/pull/1805.
I looked a bit more into separating GLX away from GL today, and it involves handling a lot of things that we don't currently handle.
I don't think it makes sense to separate them over such a small (~ 100 lines in total, excluding https://github.com/FEX-Emu/FEX/pull/1805) change.
Again, I'm re-iterating the value of this to make it to (some) release, as it
- Removes X11 as a thunk dependency - we keep running into X11 bugs and issues
- Enables a path towards 32-bit GL thunking with minimal effort (see the RFC/PoC)
- The GL/GLX (and GLdispatch) splits are far bigger than this change is, and can be worked on as a follow up as we formalize our thunking and GL/GLX/GLdispatch infra.
(Closing this as there is an ongoing powergrab by @Sonicadvance1, I will migrate my work to https://github.com/skmp/fex-emu-ng.git)