Only call GLUT functions from the GLUT thread. #270
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Will this also allow us some better control over the DPI Awareness settings? People keep asking about them, and if we can enable/disable them. The last time I was playing around with them, they seemed to be interwoven with the same glut processes that you're moving around and tweaking here for us. |
|
@SteveMcNeill I haven't looked into it that closely but I wouldn't expect these changes to make it any easier. Really I didn't change a whole lot here, we call things slightly differently to avoid the threading issues from #66 but all the GLUT logic is the same. |
a740g
approved these changes
Nov 29, 2022
RhoSigma-QB64
approved these changes
Nov 29, 2022
These tests cover all the commands that generally interact with GLUT. The ensure that these functions can be used at the very beginning of a program with no issues. Additionally they verify the behavior of these functions in the presence of `$SCREENHIDE`, and also `_ScreenHide`.
Xvfb is being used to give us an X server implementation on the Linux build agents. A running X server is necessary for graphics to function (which we have so far avoided testing).
This fixes all the code so that all the calls to glut functions happen on the same thread that is running GLUT. We achieve this by creating a queue of GLUT commands to execute. Commands can be added to the queue anywhere in the code, and then the queue is processed on the GLUT thread via it's idle func or timer func. The command is run and if necessary the result is provided in the message queue object. Each object contains a completion which can be waited on to block until the GLUT thread has processed the command. Fixes: QB64-Phoenix-Edition#66
With the recent changes to libqb.cpp to pull out some of the GLUT logic, the only actual Objective-C in libqb.cpp was pulled out. That being the case, it's no longer necessary to have libqb.mm for compiling libqb.cpp, so we're removing it to simplify the compliation logic a bit.
Fairly straightfowrad, programs were randomly seg faulting on exit. This was happening due to GLUT registering a cleanup function via atexit(), which then gets called when exit() is called. The issue happens when exit() is called on a thread other than the GLUT thread, which leads to the exit() call then attempting to cleanup GLUT while the other thread is still using it, which randomly leads to seg faults. Fixing this is slightly annoying. We cannot stop the GLUT thread, as the basic GLUT API (which is used on Mac OS) simply does not offer a way to exit the glutMainLoop() call. Thus the simplest solution is to simply make sure we call exit() on the GLUT thread, which we can fairly easily due via the message queue. That being the case, a new libqb_exit() API was added, which simply regsiters the GLUT exit message and then waits for the program to end. The atexit() handler then runs on the GLUT thread and everything works out fine. In the future we probably should redo the exit logic a bit so that all the threads are actually stopped/joined to ensure the exit process is consistent, however this is good enough for now. Also, there's plenty of error states which call exit() which I did not address.
The commands _ScreenX and _ScreenY got significantly slower due to the need to wait for the GLUT thread to wake up and execute the glutGet() command for them. We've already seen a few programs (including the IDE) where this behavior completely grinds the program to a halt, so we definitely can't keep it. The simple solution here is to not call glutGet() on every _ScreenX/Y command. Instead every time the idle/timer function runs we get the current values for the relevant glutGet() variables and store them. libqb_glut_get() then checks if the value being read is one of the ones we read in the idle/timer functionand if so just returns the last read value. By doing it this way the commands no longer has to wait on the GLUT thread for the result.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main goal of this PR is fixing #66, which was necessary to fix to get my GLUT test suite working consistently (as without it being fixed, programs simply explode too often and fail the tests). This required the addition of some new code, which I placed into the
libqb/src. This gave a good opportunity for moving all the GLUT initialization (which is fairly self-contained) intolibqb/srcas well, which what a lot of the changes here are about. The commits contain fairly detailed explanations of the changes so I would refer to those, but here's an overview:To fix #66 we needed a way of telling the GLUT thread to perform GLUT actions when the program wants to do them, which I implemented in
glut-message.h,glut-message.cpp, andglut-msg-queue.cpp. It's fairly simple, we have astd::queueprotected with a mutex, and the GLUT idle or timer function (depending on platform) calls a function to process the queue. Additionally there arelibqb_glut_*functions which can be called in the same way as the originalglutfunctions and simply add commands to the queue. For most commands we don't wait for them to be completed, but forglutGet()we use acompletionto wait for the command to be executed and then get the result.The rest of the changes are refactoring the GLUT stuff into
libqb/src. For the most part it's just a copy-paste into new functions and then wiring those back up tolibqb.cpp.$Console:Onlyvs. GLUT have different cpp files for linking (replacing the#ifdeflogic inlibqb.cpp).Additionally I was very happy that this refactor removed the little bit of Objective-C++ that was living in
libqb.cpp, so I put that in a.mmfile inlibqb/src/and was able to remove thelibqb.mmfile.Fixes: #66