Fix #8774: Black screenshots when using 40bpp-blitter. #8791
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
This affected all screenshot types that render to an off-screen buffer and don't copy the actual screen contents.
|
How much longer, approximately speaking? |
|
Really hard to properly test and measure. Its probably going to highly depend on things like cache hits/misses, but something like 5-10% it seems. There isn't any real alternative though, the whole point of |
|
What part is 5-10% slower, the normal operation of the 40bpp-anim blitter, or the screenshots? (or both?) :D |
|
These three specific broken screenshot types. As long as your sprite cache size is normally big (i.e. default), the sprite encoding change should not be measureable. |
TrueBrain
approved these changes
Mar 2, 2021
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.
This affected all screenshot types that render to an off-screen
buffer and don't copy the actual screen contents.
Motivation / Problem
Screenshot types that take their contents from an newly rendered off-screen buffer were black when using 8bpp graphics with the 40bpp-anim blitter.
Description
The 40bpp-blitter does not resolve palette values when encoding a sprite as palette resolving is done by OpenGL in the end. Off-screen screenshots render using 32bpp-optimized though, which expects resolved RGB values.
We fix this by adding a special mode to the 32bpp drawing code that resolves the palette during blitting.
Limitations
Rendering these screenshots will take longer with 40bpp-anim than with a 32bpp blitter, as it incurs an additional palette lookup for each pixel. The 40bpp-anim blitter also takes a slight performance hit during sprite encoding as it has to calculate the max brightness value just for the screenshots.
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.