GRAPHICS: Add functions for blitting with transparent masks #5409
Conversation
WalkthroughWalkthroughThe changes introduce new functionalities to the blitting operations in the graphics library. New functions have been added to support blitting with transparent color masks and color mapping. Existing functions have been modified to accommodate these new features. The changes expand the library's capabilities to handle various pixel formats, pitch parameters, and color format conversions. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- graphics/blit.h (3 hunks)
- graphics/blit/blit.cpp (20 hunks)
Additional comments: 6
graphics/blit.h (3)
85-104: The new function
maskBlithas been added to support blitting with a transparent color mask. Ensure that the mask buffer is correctly initialized and that the mask is applied correctly in the implementation.156-185: The new function
crossMaskBlithas been added to support blitting with a transparent color mask while converting between graphical formats. Ensure that the mask buffer is correctly initialized and that the mask is applied correctly in the implementation.194-200: The new function
crossMaskBlitMaphas been added to support blitting with a transparent color mask and a color map while converting between graphical formats. Ensure that the mask buffer and color map are correctly initialized and that they are applied correctly in the implementation.graphics/blit/blit.cpp (3)
112-134: The
maskBlitLogicfunction seems to be correctly implemented. It checks if the mask is non-zero before copying the source to the destination. It also correctly increments the pointers for source, destination, and mask. However, it's important to ensure that the mask, source, and destination buffers are correctly allocated and have the expected sizes to prevent buffer overflows.198-234: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [168-223]
The
crossBlitLogicfunction seems to be correctly implemented. It correctly handles the cases where the blitting should be done backward. It also correctly increments or decrements the pointers for source, destination, and mask based on whether the blitting is done backward or not. However, it's important to ensure that the mask, source, and destination buffers are correctly allocated and have the expected sizes to prevent buffer overflows.
- 239-264: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [225-261]
The
crossBlitLogic1BppSourcefunction seems to be correctly implemented. It correctly handles the cases where the blitting should be done backward. It also correctly increments or decrements the pointers for source, destination, and mask based on whether the blitting is done backward or not. However, it's important to ensure that the mask, source, and destination buffers are correctly allocated and have the expected sizes to prevent buffer overflows.
| // Function to blit a rect with a transparent color mask | ||
| bool maskBlit(byte *dst, const byte *src, const byte *mask, | ||
| const uint dstPitch, const uint srcPitch, const uint maskPitch, | ||
| const uint w, const uint h, | ||
| const uint bytesPerPixel) { | ||
| if (dst == src) | ||
| return true; | ||
|
|
||
| // Faster, but larger, to provide optimized handling for each case. | ||
| const uint srcDelta = (srcPitch - w * bytesPerPixel); | ||
| const uint dstDelta = (dstPitch - w * bytesPerPixel); | ||
| const uint maskDelta = (maskPitch - w); | ||
|
|
||
| if (bytesPerPixel == 1) { | ||
| maskBlitLogic<uint8, 1>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | ||
| } else if (bytesPerPixel == 2) { | ||
| maskBlitLogic<uint16, 2>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | ||
| } else if (bytesPerPixel == 3) { | ||
| maskBlitLogic<uint8, 3>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | ||
| } else if (bytesPerPixel == 4) { | ||
| maskBlitLogic<uint32, 4>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | ||
| } else { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
The maskBlit function correctly calls maskBlitLogic with the appropriate parameters based on the bytes per pixel. It also checks if the source and destination are the same, in which case it returns true without doing anything. However, it returns false if the bytes per pixel is not 1, 2, 3, or 4. It might be better to throw an exception or return an error code in this case, to make it clear that an unsupported pixel format was used.
- if (bytesPerPixel != 1 && bytesPerPixel != 2 && bytesPerPixel != 3 && bytesPerPixel != 4) {
- return false;
- }
+ if (bytesPerPixel != 1 && bytesPerPixel != 2 && bytesPerPixel != 3 && bytesPerPixel != 4) {
+ throw std::invalid_argument("Unsupported pixel format in maskBlit");
+ }Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| // Function to blit a rect with a transparent color mask | |
| bool maskBlit(byte *dst, const byte *src, const byte *mask, | |
| const uint dstPitch, const uint srcPitch, const uint maskPitch, | |
| const uint w, const uint h, | |
| const uint bytesPerPixel) { | |
| if (dst == src) | |
| return true; | |
| // Faster, but larger, to provide optimized handling for each case. | |
| const uint srcDelta = (srcPitch - w * bytesPerPixel); | |
| const uint dstDelta = (dstPitch - w * bytesPerPixel); | |
| const uint maskDelta = (maskPitch - w); | |
| if (bytesPerPixel == 1) { | |
| maskBlitLogic<uint8, 1>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 2) { | |
| maskBlitLogic<uint16, 2>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 3) { | |
| maskBlitLogic<uint8, 3>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 4) { | |
| maskBlitLogic<uint32, 4>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else { | |
| return false; | |
| } | |
| return true; | |
| } | |
| // Function to blit a rect with a transparent color mask | |
| bool maskBlit(byte *dst, const byte *src, const byte *mask, | |
| const uint dstPitch, const uint srcPitch, const uint maskPitch, | |
| const uint w, const uint h, | |
| const uint bytesPerPixel) { | |
| if (dst == src) | |
| return true; | |
| // Faster, but larger, to provide optimized handling for each case. | |
| const uint srcDelta = (srcPitch - w * bytesPerPixel); | |
| const uint dstDelta = (dstPitch - w * bytesPerPixel); | |
| const uint maskDelta = (maskPitch - w); | |
| if (bytesPerPixel == 1) { | |
| maskBlitLogic<uint8, 1>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 2) { | |
| maskBlitLogic<uint16, 2>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 3) { | |
| maskBlitLogic<uint8, 3>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 4) { | |
| maskBlitLogic<uint32, 4>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else { | |
| throw std::invalid_argument("Unsupported pixel format in maskBlit"); | |
| } | |
| return true; | |
| } |
There was a problem hiding this comment.
@CodeRabbit Please write a prompt which will exclude 2 things: usage of stdlib and exceptions.
There was a problem hiding this comment.
@coderabbitai Please write a prompt which will exclude 2 things: usage of stdlib and exceptions.
There was a problem hiding this comment.
@sev- Understood. If we want to avoid using exceptions and the standard library, we can modify the function to return an error code instead. Here's a version of the function that returns an integer error code:
// Function to blit a rect with a transparent color mask
// Returns 0 on success, -1 if the source and destination are the same,
// and -2 if the bytes per pixel is not 1, 2, 3, or 4.
int maskBlit(byte *dst, const byte *src, const byte *mask,
const uint dstPitch, const uint srcPitch, const uint maskPitch,
const uint w, const uint h,
const uint bytesPerPixel) {
if (dst == src)
return -1;
// Faster, but larger, to provide optimized handling for each case.
const uint srcDelta = (srcPitch - w * bytesPerPixel);
const uint dstDelta = (dstPitch - w * bytesPerPixel);
const uint maskDelta = (maskPitch - w);
if (bytesPerPixel == 1) {
maskBlitLogic<uint8, 1>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta);
} else if (bytesPerPixel == 2) {
maskBlitLogic<uint16, 2>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta);
} else if (bytesPerPixel == 3) {
maskBlitLogic<uint8, 3>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta);
} else if (bytesPerPixel == 4) {
maskBlitLogic<uint32, 4>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta);
} else {
return -2;
}
return 0;
}In this version, the function returns 0 if it completes successfully. If the source and destination are the same, it returns -1. If the bytes per pixel is not 1, 2, 3, or 4, it returns -2. This way, the caller can check the return value to see if the function completed successfully or if there was an error, and what type of error occurred.
ccawley2011
force-pushed
the
blit-masks
branch
from
e7ee868
to
36fc3eb
Compare
There was a problem hiding this comment.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files selected for processing (2)
- graphics/blit.h (3 hunks)
- graphics/blit/blit.cpp (20 hunks)
Additional comments: 6
graphics/blit.h (5)
85-86: The function
keyBlithas been updated to accept a new parameterkeyfor transparent color key. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.88-104: The new function
maskBlithas been added to support blitting with a transparent color mask. Ensure that the mask buffer is correctly initialized and used in the upstream code.156-157: The function
crossKeyBlithas been updated to accept a new parameterkeyfor transparent color key. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.159-185: The new function
crossMaskBlithas been added to support blitting with a transparent color mask while converting from one graphical format to another. Ensure that the mask buffer is correctly initialized and used in the upstream code.197-200: The new function
crossMaskBlitMaphas been added to support blitting with a transparent color mask and color mapping. Ensure that the mask buffer and color map are correctly initialized and used in the upstream code.graphics/blit/blit.cpp (1)
- 505-511: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [499-527]
The
crossBlitMapfunction
| template<typename Color, int Size> | ||
| inline void maskBlitLogic(byte *dst, const byte *src, const byte *mask, const uint w, const uint h, | ||
| const uint srcDelta, const uint dstDelta, const uint maskDelta) { | ||
| for (uint y = 0; y < h; ++y) { | ||
| for (uint x = 0; x < w; ++x) { | ||
| if (*mask) { | ||
| if (Size == sizeof(Color)) { | ||
| *(Color *)dst = *(const Color *)src; | ||
| } else { | ||
| memcpy(dst, src, Size); | ||
| } | ||
| } | ||
|
|
||
| src += Size; | ||
| dst += Size; | ||
| mask += 1; | ||
| } | ||
|
|
||
| src += srcDelta; | ||
| dst += dstDelta; | ||
| mask += maskDelta; | ||
| } | ||
| } |
There was a problem hiding this comment.
The maskBlitLogic function is well implemented. It correctly handles the masking operation for different pixel sizes. However, it assumes that the mask is a single byte, which might not always be the case. Consider adding a template parameter for the mask size if different mask sizes need to be supported.
| const uint bytesPerPixel) { | ||
| if (dst == src) | ||
| return true; | ||
|
|
||
| // Faster, but larger, to provide optimized handling for each case. | ||
| const uint srcDelta = (srcPitch - w * bytesPerPixel); | ||
| const uint dstDelta = (dstPitch - w * bytesPerPixel); | ||
| const uint maskDelta = (maskPitch - w); | ||
|
|
||
| if (bytesPerPixel == 1) { | ||
| maskBlitLogic<uint8, 1>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | ||
| } else if (bytesPerPixel == 2) { | ||
| maskBlitLogic<uint16, 2>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | ||
| } else if (bytesPerPixel == 3) { | ||
| maskBlitLogic<uint8, 3>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | ||
| } else if (bytesPerPixel == 4) { | ||
| maskBlitLogic<uint32, 4>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | ||
| } else { | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
The maskBlit function correctly calls maskBlitLogic with the appropriate parameters based on the bytes per pixel. It also checks if the source and destination are the same, in which case it returns true without doing anything. However, it returns false if the bytes per pixel is not 1, 2, 3, or 4. It might be better to throw an exception or return an error code in this case, to make it clear that an unsupported pixel format was used.
- if (bytesPerPixel != 1 && bytesPerPixel != 2 && bytesPerPixel != 3 && bytesPerPixel != 4) {
- return false;
- }
+ if (bytesPerPixel != 1 && bytesPerPixel != 2 && bytesPerPixel != 3 && bytesPerPixel != 4) {
+ throw std::invalid_argument("Unsupported pixel format in maskBlit");
+ }Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| const uint bytesPerPixel) { | |
| if (dst == src) | |
| return true; | |
| // Faster, but larger, to provide optimized handling for each case. | |
| const uint srcDelta = (srcPitch - w * bytesPerPixel); | |
| const uint dstDelta = (dstPitch - w * bytesPerPixel); | |
| const uint maskDelta = (maskPitch - w); | |
| if (bytesPerPixel == 1) { | |
| maskBlitLogic<uint8, 1>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 2) { | |
| maskBlitLogic<uint16, 2>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 3) { | |
| maskBlitLogic<uint8, 3>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 4) { | |
| maskBlitLogic<uint32, 4>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else { | |
| return false; | |
| } | |
| return true; | |
| } | |
| const uint bytesPerPixel) { | |
| if (dst == src) | |
| return true; | |
| // Faster, but larger, to provide optimized handling for each case. | |
| const uint srcDelta = (srcPitch - w * bytesPerPixel); | |
| const uint dstDelta = (dstPitch - w * bytesPerPixel); | |
| const uint maskDelta = (maskPitch - w); | |
| if (bytesPerPixel == 1) { | |
| maskBlitLogic<uint8, 1>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 2) { | |
| maskBlitLogic<uint16, 2>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 3) { | |
| maskBlitLogic<uint8, 3>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else if (bytesPerPixel == 4) { | |
| maskBlitLogic<uint32, 4>(dst, src, mask, w, h, srcDelta, dstDelta, maskDelta); | |
| } else { | |
| throw std::invalid_argument("Unsupported pixel format in maskBlit"); | |
| } | |
| return true; | |
| } |
| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | ||
| crossBlitLogic<uint16, 2, uint8, 3, true, false>(dst, src, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0); | ||
| crossBlitLogic<uint16, 2, uint8, 3, true, false, false>(dst, src, nullptr, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0, 0); | ||
| } else if (srcFmt.bytesPerPixel == 3) { | ||
| crossBlitLogic<uint8, 3, uint8, 3, false, false>(dst, src, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0); | ||
| crossBlitLogic<uint8, 3, uint8, 3, false, false, false>(dst, src, nullptr, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0, 0); | ||
| } else { | ||
| crossBlitLogic<uint32, 4, uint8, 3, false, false>(dst, src, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0); | ||
| crossBlitLogic<uint32, 4, uint8, 3, false, false, false>(dst, src, nullptr, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0, 0); | ||
| } | ||
| } else if (dstFmt.bytesPerPixel == 4) { | ||
| if (srcFmt.bytesPerPixel == 2) { |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [291-332]
The crossBlit function correctly calls crossBlitLogic with the appropriate parameters based on the bytes per pixel of the source and destination formats. It also correctly handles the cases where the blitting should be done backward. However, it returns false if the bytes per pixel is not 1, 2, 3, or 4. It might be better to throw an exception or return an error code in this case, to make it clear that an unsupported pixel format was used.
- if (dstFmt.bytesPerPixel != 1 && dstFmt.bytesPerPixel != 2 && dstFmt.bytesPerPixel != 3 && dstFmt.bytesPerPixel != 4) {
- return false;
- }
+ if (dstFmt.bytesPerPixel != 1 && dstFmt.bytesPerPixel != 2 && dstFmt.bytesPerPixel != 3 && dstFmt.bytesPerPixel != 4) {
+ throw std::invalid_argument("Unsupported pixel format in crossBlit");
+ }| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | ||
| crossBlitLogic<uint16, 2, uint8, 3, true, true>(dst, src, w, h, srcFmt, dstFmt, srcDelta, dstDelta, key); | ||
| crossBlitLogic<uint16, 2, uint8, 3, true, true, false>(dst, src, nullptr, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0, key); | ||
| } else if (srcFmt.bytesPerPixel == 3) { | ||
| crossBlitLogic<uint8, 3, uint8, 3, false, true>(dst, src, w, h, srcFmt, dstFmt, srcDelta, dstDelta, key); | ||
| crossBlitLogic<uint8, 3, uint8, 3, false, true, false>(dst, src, nullptr, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0, key); | ||
| } else { | ||
| crossBlitLogic<uint32, 4, uint8, 3, false, true>(dst, src, w, h, srcFmt, dstFmt, srcDelta, dstDelta, key); | ||
| crossBlitLogic<uint32, 4, uint8, 3, false, true, false>(dst, src, nullptr, w, h, srcFmt, dstFmt, srcDelta, dstDelta, 0, key); | ||
| } | ||
| } else if (dstFmt.bytesPerPixel == 4) { | ||
| if (srcFmt.bytesPerPixel == 2) { |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [363-407]
The crossKeyBlit function correctly calls crossBlitLogic with the appropriate parameters based on the bytes per pixel of the source and destination formats. It also correctly handles the cases where the blitting should be done backward. However, it returns false if the bytes per pixel is not 1, 2, 3, or 4. It might be better to throw an exception or return an error code in this case, to make it clear that an unsupported pixel format was used.
- if (dstFmt.bytesPerPixel != 1 && dstFmt.bytesPerPixel != 2 && dstFmt.bytesPerPixel != 3 && dstFmt.bytesPerPixel != 4) {
- return false;
- }
+ if (dstFmt.bytesPerPixel != 1 && dstFmt.bytesPerPixel != 2 && dstFmt.bytesPerPixel != 3 && dstFmt.bytesPerPixel != 4) {
+ throw std::invalid_argument("Unsupported pixel format in crossKeyBlit");
+ }| // Error out if conversion is impossible | ||
| if ((srcFmt.bytesPerPixel == 1) || (dstFmt.bytesPerPixel == 1) | ||
| || (!srcFmt.bytesPerPixel) || (!dstFmt.bytesPerPixel)) | ||
| return false; | ||
|
|
||
| // Don't perform unnecessary conversion | ||
| if (srcFmt == dstFmt) { | ||
| maskBlit(dst, src, mask, dstPitch, srcPitch, maskPitch, w, h, dstFmt.bytesPerPixel); | ||
| return true; | ||
| } | ||
|
|
||
| // Faster, but larger, to provide optimized handling for each case. | ||
| const uint srcDelta = (srcPitch - w * srcFmt.bytesPerPixel); | ||
| const uint dstDelta = (dstPitch - w * dstFmt.bytesPerPixel); | ||
| const uint maskDelta = (maskPitch - w); | ||
|
|
||
| // TODO: optimized cases for dstDelta of 0 | ||
| if (dstFmt.bytesPerPixel == 2) { | ||
| if (srcFmt.bytesPerPixel == 2) { | ||
| crossBlitLogic<uint16, 2, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else if (srcFmt.bytesPerPixel == 3) { | ||
| crossBlitLogic<uint8, 3, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else { | ||
| crossBlitLogic<uint32, 4, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } | ||
| } else if (dstFmt.bytesPerPixel == 3) { | ||
| if (srcFmt.bytesPerPixel == 2) { | ||
| // We need to blit the surface from bottom right to top left here. | ||
| // This is needed, because when we convert to the same memory | ||
| // buffer copying the surface from top left to bottom right would | ||
| // overwrite the source, since we have more bits per destination | ||
| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | ||
| crossBlitLogic<uint16, 2, uint8, 3, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else if (srcFmt.bytesPerPixel == 3) { | ||
| crossBlitLogic<uint8, 3, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else { | ||
| crossBlitLogic<uint32, 4, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } | ||
| } else if (dstFmt.bytesPerPixel == 4) { | ||
| if (srcFmt.bytesPerPixel == 2) { | ||
| // We need to blit the surface from bottom right to top left here. | ||
| // This is neeeded, because when we convert to the same memory | ||
| // buffer copying the surface from top left to bottom right would | ||
| // overwrite the source, since we have more bits per destination | ||
| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | ||
| crossBlitLogic<uint16, 2, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else if (srcFmt.bytesPerPixel == 3) { | ||
| // We need to blit the surface from bottom right to top left here. | ||
| // This is neeeded, because when we convert to the same memory | ||
| // buffer copying the surface from top left to bottom right would | ||
| // overwrite the source, since we have more bits per destination | ||
| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | ||
| crossBlitLogic<uint8, 3, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else { | ||
| crossBlitLogic<uint32, 4, uint32, 4, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); |
There was a problem hiding this comment.
The crossMaskBlit function correctly calls crossBlitLogic with the appropriate parameters based on the bytes per pixel of the source and destination formats. It also correctly handles the cases where the blitting should be done backward. However, it returns false if the bytes per pixel is not 1, 2, 3, or 4. It might be better to throw an exception or return an error code in this case, to make it clear that an unsupported pixel format was used.
- if (dstFmt.bytesPerPixel != 1 && dstFmt.bytesPerPixel != 2 && dstFmt.bytesPerPixel != 3 && dstFmt.bytesPerPixel != 4) {
- return false;
- }
+ if (dstFmt.bytesPerPixel != 1 && dstFmt.bytesPerPixel != 2 && dstFmt.bytesPerPixel != 3 && dstFmt.bytesPerPixel != 4) {
+ throw std::invalid_argument("Unsupported pixel format in crossMaskBlit");
+ }Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| // Error out if conversion is impossible | |
| if ((srcFmt.bytesPerPixel == 1) || (dstFmt.bytesPerPixel == 1) | |
| || (!srcFmt.bytesPerPixel) || (!dstFmt.bytesPerPixel)) | |
| return false; | |
| // Don't perform unnecessary conversion | |
| if (srcFmt == dstFmt) { | |
| maskBlit(dst, src, mask, dstPitch, srcPitch, maskPitch, w, h, dstFmt.bytesPerPixel); | |
| return true; | |
| } | |
| // Faster, but larger, to provide optimized handling for each case. | |
| const uint srcDelta = (srcPitch - w * srcFmt.bytesPerPixel); | |
| const uint dstDelta = (dstPitch - w * dstFmt.bytesPerPixel); | |
| const uint maskDelta = (maskPitch - w); | |
| // TODO: optimized cases for dstDelta of 0 | |
| if (dstFmt.bytesPerPixel == 2) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| crossBlitLogic<uint16, 2, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| crossBlitLogic<uint8, 3, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } | |
| } else if (dstFmt.bytesPerPixel == 3) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is needed, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint16, 2, uint8, 3, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| crossBlitLogic<uint8, 3, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } | |
| } else if (dstFmt.bytesPerPixel == 4) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is neeeded, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint16, 2, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is neeeded, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint8, 3, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint32, 4, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| // Error out if conversion is impossible | |
| if ((srcFmt.bytesPerPixel == 1) || (dstFmt.bytesPerPixel == 1) | |
| || (!srcFmt.bytesPerPixel) || (!dstFmt.bytesPerPixel)) | |
| return false; | |
| // Don't perform unnecessary conversion | |
| if (srcFmt == dstFmt) { | |
| maskBlit(dst, src, mask, dstPitch, srcPitch, maskPitch, w, h, dstFmt.bytesPerPixel); | |
| return true; | |
| } | |
| // Faster, but larger, to provide optimized handling for each case. | |
| const uint srcDelta = (srcPitch - w * srcFmt.bytesPerPixel); | |
| const uint dstDelta = (dstPitch - w * dstFmt.bytesPerPixel); | |
| const uint maskDelta = (maskPitch - w); | |
| // TODO: optimized cases for dstDelta of 0 | |
| if (dstFmt.bytesPerPixel == 2) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| crossBlitLogic<uint16, 2, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| crossBlitLogic<uint8, 3, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } | |
| } else if (dstFmt.bytesPerPixel == 3) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is needed, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint16, 2, uint8, 3, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| crossBlitLogic<uint8, 3, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } | |
| } else if (dstFmt.bytesPerPixel == 4) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is neeeded, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint16, 2, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is neeeded, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint8, 3, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint32, 4, false, false, true>(dst, src, mask, w, h, srcFmt, dst | |
| </details> | |
| <!-- suggestion_end --> | |
| <!-- This is an auto-generated comment by CodeRabbit --> |
ccawley2011
force-pushed
the
blit-masks
branch
from
36fc3eb
to
0d36215
Compare
| if (backward) { | ||
| src -= SrcSize; | ||
| dst -= DstSize; | ||
| if (hasMask) | ||
| mask -= 1; | ||
| } else { | ||
| src += SrcSize; | ||
| dst += DstSize; | ||
| if (hasMask) | ||
| mask += 1; | ||
| } | ||
| } | ||
|
|
||
| if (backward) { | ||
| src -= srcDelta; | ||
| dst -= dstDelta; | ||
| if (hasMask) | ||
| mask -= maskDelta; | ||
| } else { | ||
| src += srcDelta; | ||
| dst += dstDelta; | ||
| if (hasMask) | ||
| mask += maskDelta; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| template<typename DstColor, int DstSize, bool backward, bool hasKey> | ||
| inline void crossBlitLogic1BppSource(byte *dst, const byte *src, const uint w, const uint h, | ||
| const uint srcDelta, const uint dstDelta, const uint32 *map, const uint32 key) { | ||
| template<typename DstColor, int DstSize, bool backward, bool hasKey, bool hasMask> | ||
| inline void crossBlitLogic1BppSource(byte *dst, const byte *src, const byte *mask, const uint w, const uint h, | ||
| const uint srcDelta, const uint dstDelta, const uint maskDelta, const uint32 *map, const uint32 key) { | ||
| for (uint y = 0; y < h; ++y) { | ||
| for (uint x = 0; x < w; ++x) { | ||
| const byte color = *src; | ||
| if (!hasKey || color != key) { | ||
| if ((!hasKey || color != key) && (!hasMask || *mask != 0)) { | ||
| if (DstSize == sizeof(DstColor)) { | ||
| *(DstColor *)dst = map[color]; | ||
| } else { |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [168-223]
The crossBlitLogic function is complex and handles many cases. It might be beneficial to break it down into smaller, more manageable functions. This would improve readability and maintainability.
| if (backward) { | ||
| src -= 1; | ||
| dst -= DstSize; | ||
| if (hasMask) | ||
| mask -= 1; | ||
| } else { | ||
| src += 1; | ||
| dst += DstSize; | ||
| if (hasMask) | ||
| mask += 1; | ||
| } | ||
| } | ||
|
|
||
| if (backward) { | ||
| src -= srcDelta; | ||
| dst -= dstDelta; | ||
| if (hasMask) | ||
| mask -= maskDelta; | ||
| } else { | ||
| src += srcDelta; | ||
| dst += dstDelta; | ||
| if (hasMask) | ||
| mask += maskDelta; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [226-261]
The crossBlitLogic1BppSource function is well implemented. It correctly handles the blitting operation for different pixel sizes. However, it assumes that the mask is a single byte, which might not always be the case. Consider adding a template parameter for the mask size if different mask sizes need to be supported.
| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - 1; | ||
| crossBlitLogic1BppSource<uint16, 2, true, true>(dst, src, w, h, srcDelta, dstDelta, map, key); | ||
| crossBlitLogic1BppSource<uint16, 2, true, true, false>(dst, src, nullptr, w, h, srcDelta, dstDelta, 0, map, key); | ||
| } else if (bytesPerPixel == 3) { | ||
| // We need to blit the surface from bottom right to top left here. | ||
| // This is needed, because when we convert to the same memory |
There was a problem hiding this comment.
Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [547-575]
These sections of code are repetitive and could be simplified by creating a helper function that takes the bytes per pixel as a parameter and calls the appropriate crossBlitLogic or crossBlitLogic1BppSource function.
| // Function to blit a rect from one color format to another with a transparent color mask | ||
| bool crossMaskBlit(byte *dst, const byte *src, const byte *mask, | ||
| const uint dstPitch, const uint srcPitch, const uint maskPitch, | ||
| const uint w, const uint h, | ||
| const Graphics::PixelFormat &dstFmt, const Graphics::PixelFormat &srcFmt) { | ||
| // Error out if conversion is impossible | ||
| if ((srcFmt.bytesPerPixel == 1) || (dstFmt.bytesPerPixel == 1) | ||
| || (!srcFmt.bytesPerPixel) || (!dstFmt.bytesPerPixel)) | ||
| return false; | ||
|
|
||
| // Don't perform unnecessary conversion | ||
| if (srcFmt == dstFmt) { | ||
| maskBlit(dst, src, mask, dstPitch, srcPitch, maskPitch, w, h, dstFmt.bytesPerPixel); | ||
| return true; | ||
| } | ||
|
|
||
| // Faster, but larger, to provide optimized handling for each case. | ||
| const uint srcDelta = (srcPitch - w * srcFmt.bytesPerPixel); | ||
| const uint dstDelta = (dstPitch - w * dstFmt.bytesPerPixel); | ||
| const uint maskDelta = (maskPitch - w); | ||
|
|
||
| // TODO: optimized cases for dstDelta of 0 | ||
| if (dstFmt.bytesPerPixel == 2) { | ||
| if (srcFmt.bytesPerPixel == 2) { | ||
| crossBlitLogic<uint16, 2, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else if (srcFmt.bytesPerPixel == 3) { | ||
| crossBlitLogic<uint8, 3, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else { | ||
| crossBlitLogic<uint32, 4, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } | ||
| } else if (dstFmt.bytesPerPixel == 3) { | ||
| if (srcFmt.bytesPerPixel == 2) { | ||
| // We need to blit the surface from bottom right to top left here. | ||
| // This is needed, because when we convert to the same memory | ||
| // buffer copying the surface from top left to bottom right would | ||
| // overwrite the source, since we have more bits per destination | ||
| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | ||
| crossBlitLogic<uint16, 2, uint8, 3, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else if (srcFmt.bytesPerPixel == 3) { | ||
| crossBlitLogic<uint8, 3, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else { | ||
| crossBlitLogic<uint32, 4, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } | ||
| } else if (dstFmt.bytesPerPixel == 4) { | ||
| if (srcFmt.bytesPerPixel == 2) { | ||
| // We need to blit the surface from bottom right to top left here. | ||
| // This is neeeded, because when we convert to the same memory | ||
| // buffer copying the surface from top left to bottom right would | ||
| // overwrite the source, since we have more bits per destination | ||
| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | ||
| crossBlitLogic<uint16, 2, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else if (srcFmt.bytesPerPixel == 3) { | ||
| // We need to blit the surface from bottom right to top left here. | ||
| // This is neeeded, because when we convert to the same memory | ||
| // buffer copying the surface from top left to bottom right would | ||
| // overwrite the source, since we have more bits per destination | ||
| // color than per source color. | ||
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | ||
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | ||
| crossBlitLogic<uint8, 3, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | ||
| } else { | ||
| crossBlitLogic<uint32, 4, uint32, 4, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); |
There was a problem hiding this comment.
The crossMaskBlit function correctly calls crossBlitLogic with the appropriate parameters based on the bytes per pixel. It also checks if the source and destination are the same, in which case it returns true without doing anything. However, it returns false if the bytes per pixel is not 1, 2, 3, or 4. It might be better to throw an exception or return an error code in this case, to make it clear that an unsupported pixel format was used.
- if (bytesPerPixel != 1 && bytesPerPixel != 2 && bytesPerPixel != 3 && bytesPerPixel != 4) {
- return false;
- }
+ if (bytesPerPixel != 1 && bytesPerPixel != 2 && bytesPerPixel != 3 && bytesPerPixel != 4) {
+ throw std::invalid_argument("Unsupported pixel format in crossMaskBlit");
+ }Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| // Function to blit a rect from one color format to another with a transparent color mask | |
| bool crossMaskBlit(byte *dst, const byte *src, const byte *mask, | |
| const uint dstPitch, const uint srcPitch, const uint maskPitch, | |
| const uint w, const uint h, | |
| const Graphics::PixelFormat &dstFmt, const Graphics::PixelFormat &srcFmt) { | |
| // Error out if conversion is impossible | |
| if ((srcFmt.bytesPerPixel == 1) || (dstFmt.bytesPerPixel == 1) | |
| || (!srcFmt.bytesPerPixel) || (!dstFmt.bytesPerPixel)) | |
| return false; | |
| // Don't perform unnecessary conversion | |
| if (srcFmt == dstFmt) { | |
| maskBlit(dst, src, mask, dstPitch, srcPitch, maskPitch, w, h, dstFmt.bytesPerPixel); | |
| return true; | |
| } | |
| // Faster, but larger, to provide optimized handling for each case. | |
| const uint srcDelta = (srcPitch - w * srcFmt.bytesPerPixel); | |
| const uint dstDelta = (dstPitch - w * dstFmt.bytesPerPixel); | |
| const uint maskDelta = (maskPitch - w); | |
| // TODO: optimized cases for dstDelta of 0 | |
| if (dstFmt.bytesPerPixel == 2) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| crossBlitLogic<uint16, 2, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| crossBlitLogic<uint8, 3, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } | |
| } else if (dstFmt.bytesPerPixel == 3) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is needed, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint16, 2, uint8, 3, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| crossBlitLogic<uint8, 3, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } | |
| } else if (dstFmt.bytesPerPixel == 4) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is neeeded, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint16, 2, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is neeeded, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint8, 3, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint32, 4, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| // Function to blit a rect from one color format to another with a transparent color mask | |
| bool crossMaskBlit(byte *dst, const byte *src, const byte *mask, | |
| const uint dstPitch, const uint srcPitch, const uint maskPitch, | |
| const uint w, const uint h, | |
| const Graphics::PixelFormat &dstFmt, const Graphics::PixelFormat &srcFmt) { | |
| // Error out if conversion is impossible | |
| if ((srcFmt.bytesPerPixel == 1) || (dstFmt.bytesPerPixel == 1) | |
| || (!srcFmt.bytesPerPixel) || (!dstFmt.bytesPerPixel)) | |
| throw std::invalid_argument("Unsupported pixel format in crossMaskBlit"); | |
| // Don't perform unnecessary conversion | |
| if (srcFmt == dstFmt) { | |
| maskBlit(dst, src, mask, dstPitch, srcPitch, maskPitch, w, h, dstFmt.bytesPerPixel); | |
| return true; | |
| } | |
| // Faster, but larger, to provide optimized handling for each case. | |
| const uint srcDelta = (srcPitch - w * srcFmt.bytesPerPixel); | |
| const uint dstDelta = (dstPitch - w * dstFmt.bytesPerPixel); | |
| const uint maskDelta = (maskPitch - w); | |
| // TODO: optimized cases for dstDelta of 0 | |
| if (dstFmt.bytesPerPixel == 2) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| crossBlitLogic<uint16, 2, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| crossBlitLogic<uint8, 3, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint16, 2, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } | |
| } else if (dstFmt.bytesPerPixel == 3) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is needed, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint16, 2, uint8, 3, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| crossBlitLogic<uint8, 3, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else { | |
| crossBlitLogic<uint32, 4, uint8, 3, false, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } | |
| } else if (dstFmt.bytesPerPixel == 4) { | |
| if (srcFmt.bytesPerPixel == 2) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is neeeded, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| crossBlitLogic<uint16, 2, uint32, 4, true, false, true>(dst, src, mask, w, h, srcFmt, dstFmt, srcDelta, dstDelta, maskDelta, 0); | |
| } else if (srcFmt.bytesPerPixel == 3) { | |
| // We need to blit the surface from bottom right to top left here. | |
| // This is neeeded, because when we convert to the same memory | |
| // buffer copying the surface from top left to bottom right would | |
| // overwrite the source, since we have more bits per destination | |
| // color than per source color. | |
| dst += h * dstPitch - dstDelta - dstFmt.bytesPerPixel; | |
| src += h * srcPitch - srcDelta - srcFmt.bytesPerPixel; | |
| c | |
| </details> | |
| <!-- suggestion_end --> | |
| <!-- This is an auto-generated comment by CodeRabbit --> |
|
Thanks! |
These can be found in RISC OS sprite files, but may also be useful elsewhere.