Skip to content

Conversation

@DanielKinsman
Copy link
Contributor

Fixes #45523 #103951 #91600

Uses libjpeg from the Independent JPEG Group, which is pretty much the reference implementation. Might have been nice to use libjpeg-turbo instead for the speed improvements but it seemed too complicated to shoe-horn into Godot's scons system.

Doesn't appear to be any impact on speed or binary size as a result of the changes, but the images from those linked issues load correctly now.

  • implement saving (compression)
  • test on different platforms
  • add more error handling
  • replace the jpg module entirely and migrate thorvg and basis_universal to use this module?

Before I start on those I thought I'd see what you all think of the approach?

@fire
Copy link
Member

fire commented Mar 18, 2025

I thought there’ll be more benefits to using jpeg turbo. How bad is it?

@DanielKinsman
Copy link
Contributor Author

Turbo compiles the same source files several times for different bit depths (8, 12, 16), puts the resulting object files into subdirs specific for that bit depth, before linking them all together.

@fire
Copy link
Member

fire commented Mar 18, 2025

I think libturbo already has integration with thorvg. Those negatives about making the scons doesn’t seem that bad.

@akien-mga
Copy link
Member

Turbo compiles the same source files several times for different bit depths (8, 12, 16), puts the resulting object files into subdirs specific for that bit depth, before linking them all together.

The PCRE2 library does something similar to this, here's how we handle it in SCons:
https://github.com/godotengine/godot/blob/master/modules/regex/SCsub#L58-L66

So something like this could possibly be done for libjpeg-turbo.

If you want to explore it I'd suggest making it a separate PR so we can evaluate both implementations.

In general I'm in favor of switching to a more complete and maintained library, but we need to make sure we strike the right balance in terms of added thirdparty code, complexity, and binary size.

@akien-mga
Copy link
Member

akien-mga commented Mar 18, 2025

I checked the impact of this change on compilation speed and binary size for a Linux release template:

master (fc827bb) This PR (rebased on fc827bb) Difference
Compilation time 2m39.430s / 2m39.581s 2m40.609s / 2m39.223s Within error margin
Size 66.346 MiB 66.495 MiB +59.9 KiB

Seems reasonable.

@DanielKinsman
Copy link
Contributor Author

Declining, will go with libjpeg-turbo instead in #104347

@akien-mga akien-mga removed this from the 4.x milestone Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Valid JPG file fails to load using Image::load_jpg_from_buffer()

4 participants