Add a SDL12COMPAT_MAX_BPP hint, and use it to restrict Hyperspace Delivery Boy to 16bpp (fixes #317) #321
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
Some games (e.g. Hyperspace Delivery Boy) only work on a 16-bit display, but request the highest bit-depth available. Add a hint which makes all queries for a mode (including the current mode, and the implicit format chosen by providng SDL_SetVideoMode() a bpp of 0) report a bit depth less than or equal to the value of SDL12COMPAT_FORCE_BPP. For example: SDL12COMPAT_FORCE_BPP=16 ./hdb There are a few interesting "features" of this implementation: - All video modes are now clamped to 32bit, which is the default value of the hint. This seems like something we'd probably want anyway, as many SDL 1.2 apps would have trouble with >32bit video modes anway (and we never properly supported them). - This is our first proper integer hint, so add an SDL_GetHintInt() function. - Split the BPP -> format conversion into its own helper function. - This doesn't clamp the bit depth of user-created surfaces, or of the screen if the application specifically requests a bit depth. It only changes defaults and reported modes.
The LGP port of Hyperspace Delivery Boy has broken colour keys if run in 32-bpp mode (see bug libsdl-org#317). This is because it relies heavily on the imprecise RGB565->RGB888 conversion in earlier SDL 1.2 versions, when running in 32-bpp mode. The game's assets are all in 565 format, and the game converts these to the screen's format on load. It then sets a colour key. This presents a problem, because: - The generic BlitNToN implementation in SDL 1.2 just shifted the values, so the resulting image was not at full range. Magenta became (F800F8). - Early versions of SDL 1.2 fell back to the BlitNToN blitter very frequently: libsdl-org/SDL-1.2@6f4a75d - So, Hyperspace Delivery Boy calls SDL_MapRGB(0xF8, 0, 0xF8) to get the colour key, then sets it on the converted surface. - In SDL 2.0, the blitters now properly do a full-range conversion, so the magenta becomes (FF00FF), which now doesn't match the hardcoded (F800F8). - That being said, in general, it's not guaranteed that SDL_MapRGB() will do the same format conversion as SDL_CovertSurface(), so the "correct" way of handling this is to set the colour key before converting, which works (albeit slowly) in SDL2: libsdl-org/SDL#1854 - Since the conversion behaviour is different even between SDL 1.2 versions, it's not worth trying to imitate it here, so we just force the game to run in 16-bpp mode, which works fine. - (And the game's README recommends it, too.)
|
Looks good, thanks! |
|
diff --git a/src/SDL12_compat.c b/src/SDL12_compat.c
index f3ecd3b..f089a69 100644
--- a/src/SDL12_compat.c
+++ b/src/SDL12_compat.c
@@ -2049 +2049 @@ static SDL_PixelFormatEnum
-BPPToPixelFormat(int bpp)
+BPPToPixelFormat(unsigned bpp)
@@ -2247 +2247 @@ Init12VidModes(void)
- const int max_bpp = SDL12Compat_GetHintInt("SDL12COMPAT_MAX_BPP", 32);
+ const unsigned max_bpp = SDL12Compat_GetHintInt("SDL12COMPAT_MAX_BPP", 32);
@@ -2416 +2416 @@ Init12Video(void)
- const int max_bpp = SDL12Compat_GetHintInt("SDL12COMPAT_MAX_BPP", 32);
+ const unsigned max_bpp = SDL12Compat_GetHintInt("SDL12COMPAT_MAX_BPP", 32);OK, pushing that myself (noticed late that it's already merged) |
Whoops — nice catch, thanks! |
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.
Some games (e.g. Hyperspace Delivery Boy) only work on a 16-bit display,
but request the highest bit-depth available. Add a hint which makes all
queries for a mode (including the current mode, and the implicit format
chosen by providng SDL_SetVideoMode() a bpp of 0) report a bit depth
less than or equal to the value of SDL12COMPAT_FORCE_BPP.
We then add a quirk to force Hyperspace Delivery Boy to 16-bit, as it uses
the current video mode's bpp, but for 32-bit modes relies heavily on the
exact way format conversion was broken in some SDL 1.2 versions.
There are a few interesting "features" of this implementation:
of the hint. This seems like something we'd probably want anyway, as
many SDL 1.2 apps would have trouble with >32bit video modes anway
(and we never properly supported them).
function.
screen if the application specifically requests a bit depth. It only
changes defaults and reported modes.
Regardless, this seems, at least to me, to be a better solution than trying
to emulate all of the ways different versions of SDL 1.2 could get format
conversion wrong. And, if you're bored, you can force games to run at 8bpp
and enjoy the horrible colour banding.