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

LibGfx+LibGUI: Small grabbag of jpeg2000-related tweaks #25671

Merged
merged 4 commits into from
Jan 31, 2025

Conversation

nico
Copy link
Contributor

@nico nico commented Jan 30, 2025

LibGfx+LibGUI: Add .jpf to known image extensions

This should have been part of f2e381a in #24016.

The Bitmap.h change has the effect that FileManager now shows a
preview of buggie-gray.jpf when opening
/usr/Tests/LibGfx/test-inputs/jpeg2000.


LibGfx/JPEG2000: Dump ISOBMFF boxes in JPEG2000_DEBUG builds


LibGfx/ISOBMFF/JPEG2000: Add a few known values

...for compression_type and enumerated_color_space values.

(Since these are open fields, the field type can't be an enum.)


LibGfx/JPEG2000: Reject images that do not use JPEG2000 compression

nico added 4 commits January 30, 2025 05:47
This should have been part of f2e381a in SerenityOS#24016.

The Bitmap.h change has the effect that FileManager now shows a
preview of buggie-gray.jpf when opening
/usr/Tests/LibGfx/test-inputs/jpeg2000.
...for compression_type and enumerated_color_space values.

(Since these are open fields, the field type can't be an enum.)
@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jan 30, 2025
Copy link
Member

@LucasChollet LucasChollet left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM!

@@ -25,6 +25,43 @@ struct JPEG2000ImageHeaderBox final : public Box {
u8 compression_type { 0 };
u8 is_colorspace_unknown { 0 };
u8 contains_intellectual_property_rights { 0 };

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why you are adding these enums if we can't use them 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Also don't enums allow not listed values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use them to compare against, see 4th commit. But we "can't" use them as the type of the field. (Since it's just an enum, not an enum class, comparing ints against them works without casts.)

enums do allow not listed values, especially with an explicit underlying type, but that's something we don't usually use. I think clang doesn't warn if you switch over an enum and handle all listed values, and for gcc the kinda automatic fix for the diag is to add a VERIFY_NOT_REACHED() after the switch, which here would be wrong.

@nico nico merged commit 93ecf84 into SerenityOS:master Jan 31, 2025
16 checks passed
@nico nico deleted the jpeg2000-grabbag branch January 31, 2025 00:08
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Jan 31, 2025
@nico
Copy link
Contributor Author

nico commented Jan 31, 2025

Thanks for taking a look 🙂

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