DIRECTOR: Fixes for The Dark Eye #5404
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
moralrecordings
force-pushed
the
darkeye
branch
from
ad394ba
to
b4a1299
Compare
Previously, BITD bitmaps were pre-converted to match the ScummVM colour depth. This caused issues in 32-bit mode; you are allowed to change the palette of an 8-bit image at any time, so pre-converting all 8-bit images to 32-bit leaves you frozen to whatever palette was in use when the image was loaded. The correct way to deal with this is to keep the original bitmap pixel format, and perform the conversion at widget creation time.
moralrecordings
force-pushed
the
darkeye
branch
from
b4a1299
to
6085c78
Compare
|
@coderabbitai review |
WalkthroughWalkthroughThe changes primarily focus on enhancing the handling of bitmap images in the Director engine. This includes modifications to the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
+216
to
+217
| // ScummVM using 8-bit video | ||
|
|
There was a problem hiding this comment.
The comment here is a bit confusing. It's not clear what "early directors" refers to. Consider revising for clarity.
Comment on lines
+263
to
+352
| Graphics::Surface *BitmapCastMember::getDitherImg() { | ||
| Graphics::Surface *dither = nullptr; | ||
|
|
||
| // Convert indexed image to indexed palette | ||
| Movie *movie = g_director->getCurrentMovie(); | ||
| Cast *cast = movie->getCast(); | ||
| Score *score = movie->getScore(); | ||
| // Get the current score palette. Note that this is the ID of the palette in the list, not the cast member! | ||
| CastMemberID currentPaletteId = score->getCurrentPalette(); | ||
| if (currentPaletteId.isNull()) | ||
| currentPaletteId = cast->_defaultPalette; | ||
| PaletteV4 *currentPalette = g_director->getPalette(currentPaletteId); | ||
| if (!currentPalette) { | ||
| currentPaletteId = CastMemberID(kClutSystemMac, -1); | ||
| currentPalette = g_director->getPalette(currentPaletteId); | ||
| } | ||
| CastMemberID castPaletteId = _clut; | ||
| // It is possible for Director to have saved an invalid ID in _clut; | ||
| // if this is the case, do no dithering. | ||
| if (castPaletteId.isNull()) | ||
| castPaletteId = currentPaletteId; | ||
|
|
||
| // Check if the palette is in the middle of a color fade event | ||
| bool isColorCycling = score->isPaletteColorCycling(); | ||
|
|
||
| // First, check if the palettes are different | ||
| switch (_bitsPerPixel) { | ||
| // 1bpp - this is preconverted to 0x00 and 0xff, change nothing. | ||
| case 1: | ||
| break; | ||
| // 2bpp - convert to nearest using the standard 2-bit palette. | ||
| case 2: | ||
| { | ||
| const PaletteV4 &srcPal = g_director->getLoaded4Palette(); | ||
| dither = _picture->_surface.convertTo(g_director->_wm->_pixelformat, srcPal.palette, srcPal.length, currentPalette->palette, currentPalette->length, Graphics::kDitherNaive); | ||
| } | ||
| break; | ||
| // 4bpp - if using a builtin palette, use one of the corresponding 4-bit ones. | ||
| case 4: | ||
| { | ||
| const auto pals = g_director->getLoaded16Palettes(); | ||
| // in D4 you aren't allowed to use custom palettes for 4-bit images, so uh... | ||
| // I guess default to the mac palette? | ||
| CastMemberID palIndex = pals.contains(castPaletteId) ? castPaletteId : CastMemberID(kClutSystemMac, -1); | ||
| const PaletteV4 &srcPal = pals.getVal(palIndex); | ||
| dither = _picture->_surface.convertTo(g_director->_wm->_pixelformat, srcPal.palette, srcPal.length, currentPalette->palette, currentPalette->length, Graphics::kDitherNaive); | ||
| } | ||
| break; | ||
| // 8bpp - if using a different palette, and we're not doing a color cycling operation, convert using nearest colour matching | ||
| case 8: | ||
| // Only redither 8-bit images if we have the flag set, or it is external | ||
| if (!movie->_remapPalettesWhenNeeded && !_external) | ||
| break; | ||
| if (_external || (castPaletteId != currentPaletteId && !isColorCycling)) { | ||
| const auto pals = g_director->getLoadedPalettes(); | ||
| CastMemberID palIndex = pals.contains(castPaletteId) ? castPaletteId : CastMemberID(kClutSystemMac, -1); | ||
| const PaletteV4 &srcPal = pals.getVal(palIndex); | ||
|
|
||
| // If it is an external image, use the included palette. | ||
| // For BMP images especially, they'll often have the right colors | ||
| // but in the wrong palette order. | ||
| const byte *palPtr = _external ? _picture->_palette : srcPal.palette; | ||
| int palLength = _external ? _picture->getPaletteSize() : srcPal.length; | ||
| dither = _picture->_surface.convertTo(g_director->_wm->_pixelformat, palPtr, palLength, currentPalette->palette, currentPalette->length, Graphics::kDitherNaive); | ||
| } | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| if (dither) { | ||
| // Save the palette ID so we can check if a redraw is required | ||
| _ditheredTargetClut = currentPaletteId; | ||
|
|
||
| if (!_external) { | ||
| // Finally, the first and last colours in the palette are special. No matter what the palette remap | ||
| // does, we need to scrub those to be the same. | ||
| const Graphics::Surface *src = &_picture->_surface; | ||
| for (int y = 0; y < src->h; y++) { | ||
| for (int x = 0; x < src->w; x++) { | ||
| const int test = *(const byte *)src->getBasePtr(x, y); | ||
| if (test == 0 || test == (1 << _bitsPerPixel) - 1) { | ||
| *(byte *)dither->getBasePtr(x, y) = test == 0 ? 0x00 : 0xff; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| return dither; | ||
|
|
There was a problem hiding this comment.
The getDitherImg() function is quite long and complex. Consider breaking it down into smaller, more manageable functions. This will improve readability and maintainability.
|
Thank you! |
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.
No description provided.