Fix visibility of city circles and names #131
Conversation
Currently, cities are not drawn until at least half of it is onscreen, meaning they "pop in" suddenly. This fixes that problem. Also, the check for city name was too aggressive, meaning the name of small cities (e.g. Motel in Sonora) would fail to render near the bottom of the screen.
|
🚀 WebAssembly PR Preview for 982c5fc is available at: https://fallout2-ce.github.io/fallout2-ce/PR-131/ |
|
Tried this in game. I have been doing a bunch of coding with the worldmap, so seeing the difference was great. Very nice. Big improvement. I haven't looked at the code yet, but after scrolling around with mouse and keyboard, I clicked on a town and may have got a ctd. This was on the web version, so I'm not sure. It suddenly shifted to a 'page cannot be found' screen, so I am assuming CTD. |
Hmm, not great. I'll do more testing before merging. Yeah, definitely a crash! Will continue testing |
|
Well, I couldn't reproduce that in my local build. I will keep trying, but it seems solid so far. |
|
I can reproduce pretty reliably. The problem is that we're trying to render the circle outside the actual buffer size, and overwriting memory. I'll have to think about potential solutions tomorrow. Btw, do you know how to view |
|
Ah, my build has a buffer that fills the screen I believe, so no crash for me. (Part of stretching for World Map) You might be able to do that then, just make the buffer large enough to fill the screen, and the world map border/interface (which is blitted over a second time) hides everthing. No idea on debug messages. @sherief likely know, as I think he has been using them. |
|
I spoke too soon. I am getting more CTD on my version too. How are you reproducing? Mine seem random so far. |
|
@cambragol are you testing by integrating a PR's changes with your local changes? That must be finicky. I just switch to the PR branch, build, (~3s) then drag the .app to my Sonora folder to test. (Stashing first if necessary.) That way, I'm testing exactly what the build is proposing to merge as well. (So far, I've found that easier than using the web debug builds since uploading the data takes >1min for me.) |
|
I am using the web builds. This was the first CTD I experienced on the web interface, so I wanted to be sure. However, my own build is getting quite distant from this whole effort. I don't know how I will be able to consolidate at this point. I have thousands of lines of code for all the stretching functions built over multiple files, and they are all based off Alex's, not this. Also, I can't build from this branch, since I updated to Sequoia latest. I will have to prevent some of make files updating in order to do that. See #106 |
|
Thinking this one through, you could: Make an off screen buffer, which is larger than the window buffer (like, maybe the size of a large city icon larger in each direction) Then draw all the city icons onto this buffer. Then clip this buffer to the bounds of the window buffer. Then blit this clipped section to the window buffer via blitBufferToBufferTrans |
|
I would prefer to go with clipping rather than having a bigger buffer. In my But it is just my opinion |
|
Actually the idea of having an offscreen buffer is pretty good |
|
what are the callstacks for ctd? |
|
@sherief Surprisingly no callstack, just a crash. I don't think it's too important to understand the crash since the cause is clear and avoidable (blitting outside the bounds of the buffer) |
|
I have a working fix for this. Should I just add it to this pull request @mikeklaas ? |
|
@cambragol sounds good! I'm happy to review/test |
|
🚀 WebAssembly PR Preview for 5995592 is available at: https://fallout2-ce.github.io/fallout2-ce/PR-131/ |
|
Cool, give it a try. I haven't had any issues yet. |
|
🚀 WebAssembly PR Preview for 3e57488 is available at: https://fallout2-ce.github.io/fallout2-ce/PR-131/ |
|
A bit busy for a few days, but I will! |
| int bufferWidth = viewportWidth + PADDING * 2; | ||
| int bufferHeight = viewportHeight + PADDING * 2; | ||
|
|
||
| unsigned char* paddedBuf = (unsigned char*)malloc(bufferWidth * bufferHeight); |
There was a problem hiding this comment.
It's a pretty significant change from the current code that allocates the screen buffer once, vs this which allocates a new ~500kB buffer every frame, and copies the whole screen onto it. Maybe that isn't super significant for modern computers, though this'll be yet bigger when we have hires support for the world map
|
@cambragol I think #178 is a better solution here. If you agree, we can close this PR. |
|
I'm no expert, so I'll trust your judgement and experience here. But, maybe we should hold off deleting this until the other is confirmed. |
Description
Currently, cities are not drawn until at least half of it is onscreen, meaning they "pop in" suddenly. This fixes that problem.
Also, the check for city name was too aggressive, meaning the name of small cities (e.g. Motel in Sonora) would fail to render near the bottom of the screen.
Reproduction Steps and Savefile (if available)
Scroll around worldmap until a city is halfway obscured. It will disappear (see #130)
Screenshots
Example from #130 :
Fixes #130