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
Add: make modal windows update more smooth and fix "closing the app" issues closely related to this. #8830
Conversation
227302d
to
422c9ec
Compare
6728f5e
to
deb94ec
Compare
Some stuff I noticed after testing (some may not be related to this pr but whatever):
|
Tnx for testing!
Good! That is the important bit :D
Are you sure it is doing the scan at 100%? How do you tell? For me it is working as expected, but one important observation: at the end of the NewGRF Scan, it stalls for a bit of time. There is a pretty hefty function being called when the Scan is complete, which stalls for anywhere between 300ms up to 2seconds for me.
The first 5 steps are indeed laggy. This is because TGP does a massive calculation at once, and prints that on the map. There are no in-betweens there, and it doesn't allow mouse movement while doing. I think future-extensions can be to allow mouse movements, even during times game-tick has a lock on the game-state. But we are a bit away from that :)
shrug. Not disagreeing, but indeed out of scope of this PR. Maybe worth an issue on its own?
Haha, I noticed that too :D I will fix this one, as it is really silly :P |
deb94ec
to
655898e
Compare
Basically, the game remembers how many NewGRFs you had last round, and assumes you will have that again. So the 100% is at the last known amount you had ( |
655898e
to
47e3689
Compare
47e3689
to
9397abb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the look of the code in general, all sorts of mutex & global removals. As for it's correctness...
Basically, modal windows had their own thread-locking for what drawing was possible. This is a bit nonsense now we have a game-thread. And it makes much more sense to do things like NewGRFScan and GenerateWorld in the game-thread, and not in a thread next to the game-thread. This commit changes that: it removes the threads for NewGRFScan and GenerateWorld, and just runs the code in the game-thread. On regular intervals it allows the draw-thread to do a tick, which gives a much smoother look and feel. It does slow down NewGRFScan and GenerateWorld ever so slightly as it spends more time on drawing. But the slowdown is not measureable on my machines (with 700+ NewGRFs / 4kx4k map and a Debug build). Running without a game-thread means NewGRFScan and GenerateWorld are now blocking.
Otherwise the numbers are all over the place when a modal window just closed.
This prevents the window from "freezing" when you close it during the scanning of NewGRFs, as it first would continue the action.
This prevents the window from "freezing" when you close it during world generation, as it first would continue the action.
Otherwise that might cause calls to the video-driver, which are already shut down by now. This causes, depending on the video-driver crashes or weird effects.
9397abb
to
0415a33
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been tested reasonably thoroughly, and removing mutexes & global variables is always nice
YOLO
(okay, this might be growing a bit too much; if it is easier if I PR them separately (a few depend on each other, but I can deal with that), just let me know!)
Closes #8828
Fixes #8760
Fixes #8833
Motivation / Problem
With the gameloop now threaded, we can get ride of the ugliness that the NewGRFScan and GenerateWorld threads are. They are a huge hack dating back when threads were still scary and not many people had more than one core. Pretty sure those days are over :P
By removing the special code for those two modal windows, the code becomes a lot simpler. Having such massive chunks of code not running in different threads makes things easier to debug and understand.
Purely by accident, this solves most issues mentioned in #8712, with the exception of the palette.
But mostly ... it now looks so much more smooth to generate a world ...
genworld.mp4
Description
All the extra
MarkDirty
calls are because formally it just made the whole screen dirty every time, but that is very expensive. So now only that what changed is marked dirty, which is a lot nicer on the CPU.Limitations
-vwin32-opengl:no_threads
means you don't see those two modals; similar to running without threads before this commit. I think this is reasonable, to make it work this way.Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.