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

Use libplum for RGBGFX #1214

Open
pinobatch opened this issue Nov 4, 2023 · 7 comments
Open

Use libplum for RGBGFX #1214

pinobatch opened this issue Nov 4, 2023 · 7 comments
Labels
refactoring This PR is intended to clean up code more than change functionality rgbgfx This affects RGBGFX

Comments

@pinobatch
Copy link
Member

RGBGFX currently depends on zlib and libpng. These dependencies have proven to cause inconvenience, especially when building for Windows. When zlib or libpng maintainers issue certain kinds of update, they upload the zipfile of the new version's source code, update the web page, and delete the zipfile of the old insecure version from the server. This causes all CI statuses on all PRs to come to a crashing halt when Actions tries and fails to download zlib and libpng while building the Windows executables. See (for example) #92, #173, #469, #783, #1082, #1163, and #1202.

In a recent steering meeting, it was suggested to migrate from libpng to libplum, an image loader maintained by server member ax6 (@aaaaaa123456789). I don't know to what extent libplum depends on POSIXisms that Windows doesn't follow.

@Rangi42
Copy link
Contributor

Rangi42 commented Nov 4, 2023

Other alternatives to weigh: libspng, lodepng, stb_image, fpng. (Haven't investigated yet, there may be reasons why one or all of these are non-starters.)

@Rangi42 Rangi42 added rgbgfx This affects RGBGFX refactoring This PR is intended to clean up code more than change functionality labels Nov 4, 2023
@aaaaaa123456789
Copy link
Member

I don't know to what extent libplum depends on POSIXisms that Windows doesn't follow.

It does not. It's designed to be portable. However, it does not build under MSVC, because it requires C17, and the only C standard that MSVC properly supports is C89. (That being said, it can be built for Windows using any conformant compiler.)

@Rangi42
Copy link
Contributor

Rangi42 commented Nov 4, 2023

It also needs bash to build, since it relies on merge.sh.

@aaaaaa123456789
Copy link
Member

It also needs bash to build, since it relies on merge.sh.

Only from source. The releases already contain the merged files.

@Rangi42
Copy link
Contributor

Rangi42 commented Nov 29, 2023

If RGBGFX gets rewritten in Rust, it may end up using an existing Rust library like image. But if we do want libplum, I was looking at https://github.com/alexcrichton/bzip2-rs as an example of how to write a Rust wrapper around a C library.

@ISSOtm
Copy link
Member

ISSOtm commented Jan 2, 2024

The plan is indeed to RIIR (#1213), and also to switch to libplum since it provides the functionality we are interested in (in particular, palette-aware image loading) but also support for more image formats than PNG.

I had begun writing a higher-level wrapper (the low-level bindings were trivial to write thanks to bindgen), but I got stuck on how to design that crate's API to wrap around libplum. I think @eievui5 had some interest in writing such a wrapper, but am not sure if she'd be interested in designing and/or developing it after I admittedly stole her thunder... :/ (My apologies for that, if you're reading this.)

@LIJI32
Copy link
Member

LIJI32 commented Jan 2, 2024

Replacing libpng with libplum has the side-effect of removing all of the runtime dependencies that are downloaded from Homebrew on macOS, which has these nice side-effects:

  1. It simplifies the release build process by removing the need to hack around pkg-config, brew, etc to statically link against libpng
  2. It will allow to trivially add fat-binary support in the macOS releases, so RGBDS could have readily-downloadable ARM64 builds rather than having to emulate it via Rosetta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring This PR is intended to clean up code more than change functionality rgbgfx This affects RGBGFX
Projects
None yet
Development

No branches or pull requests

5 participants