Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix plJPEG image channels order #127

Merged
merged 4 commits into from
Mar 6, 2024
Merged

Fix plJPEG image channels order #127

merged 4 commits into from
Mar 6, 2024

Conversation

Hoikas
Copy link
Member

@Hoikas Hoikas commented Aug 20, 2018

Plasma's plMipmaps store image data in BGR(A) for all uncompressed image types. However, JPEGs are stored as RGB on disk, so we must flip the channels for images that we read in. RLE images are unaffected because they operate on the pixel level. Indeed, the previous behavior would be inconsistent in that Cyan's lossy JPEG mipmaps would appear in RGB while Cyan's RLE mipmaps would appear in BGR.

@zrax
Copy link
Member

zrax commented Aug 20, 2018

This one probably needs updates in PlasmaShop as well, for JPEG rendering and also JPEG decal alpha masking.

@Hoikas
Copy link
Member Author

Hoikas commented Aug 20, 2018

That should be handled by H-uru/PlasmaShop#44

@Hoikas
Copy link
Member Author

Hoikas commented Mar 5, 2024

We've had to explain why certain mipmaps have the blue/red channels reversed when viewed in PRPShop on the OU discord. Perhaps it is time to revisit this and H-uru/PlasmaShop#44, and @Deledrius's successor H-uru/PlasmaShop#69?

@@ -189,6 +189,15 @@ void plJPEG::DecompressJPEG(hsStream* S, void* buf, size_t size) {
offs += out_stride;
}

// Data stored as RGB on disk but Plasma uses BGR
uint32_t* dp = reinterpret_cast<uint32_t*>(buf);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in the PlasmaShop PR, we should probably add an assert to ensure the buf is properly aligned for the cast...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libHSPlasma doesn't seem to have a standard assertion macro like Plasma does, so I added a throw hsBadParamException if the alignment doesn't smell right.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's reasonable...

@Deledrius
Copy link
Member

We've had to explain why certain mipmaps have the blue/red channels reversed when viewed in PRPShop on the OU discord. Perhaps it is time to revisit this and H-uru/PlasmaShop#44, and @Deledrius's successor H-uru/PlasmaShop#69?

Yeah, this fix has been ready for years now. :(

Plasma's plMipmaps store image data in BGR(A) for all uncompressed image
types. However, JPEGs are stored as RGB on disk, so we must flip the
channels for images that we read in. RLE images are unaffected because
they operate on the pixel level.
core/Util/plJPEG.cpp Outdated Show resolved Hide resolved
Co-authored-by: Michael Hansen <zrax0111@gmail.com>
@zrax
Copy link
Member

zrax commented Mar 6, 2024

Ugh, MSVC 2013 doesn't support alignof() apparently

@zrax zrax merged commit 82a6b6c into H-uru:master Mar 6, 2024
8 of 9 checks passed
@Hoikas Hoikas deleted the bgra branch March 6, 2024 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants