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

Enhancement: ensure metadata.background is compatible with color package for single-channel images #4090

Open
Zirafnik opened this issue Apr 29, 2024 · 3 comments

Comments

@Zirafnik
Copy link

Zirafnik commented Apr 29, 2024

When using sharp(input).metadata() an object with properties, which includes background is returned.

background: Default background colour, if present, for PNG (bKGD) and GIF images, either an RGB Object or a single greyscale value

It can return RGB Object or greyscale number.

When using sharp(input).flatten() we can pass in an options object which includes the background color property. This option expects an RGB Object or a CSS type color string (e.g. 'white').

[options.background] string | Object "{r: 0, g: 0, b: 0}"

What I did, was use the background color returned by .metadata(), directly as an option in .flatten(). This however, produced an error in cases where the metadata returned a greyscale number, which is not a valid input for flatten background option.

Current workaround is simple: just repeat the greyscale number value for all the RGB Object fields.

const image = sharp(input)

const data = await image.metadata();

// THIS CAN FAIL: when background is greyscale number
const image
    .flatten({background: data.background ?? 'white'}) // include fallback when no background is returned
    .toBuffer();

// WORKAROUND:
const background =
        typeof data.background === 'object'
            ? data.background
            : { r: data.background, g: data.background, b: data.background };

const image
    .flatten({ background }) // need a separate check for fallback (not included here)
    .toBuffer();

However, while the "fix" is simple, it is not the expected behavior and should be adjusted. I propose that the .flatten() background option accepts a greyscale number as possible input.

@lovell
Copy link
Owner

lovell commented Apr 29, 2024

The simplest solution here is probably to always return an RGB Object in metadata.background regardless of the number of input channels.

Happy to accept a PR for this, if you're able. We should add a test case for this too as it doesn't appear to be covered by any of the scenarios in test/unit/metadata.js at the moment.

@lovell lovell added enhancement and removed triage labels Apr 29, 2024
@lovell lovell changed the title .flatten() not accepting background greyscale options Enhancement: ensure metadata.background always returns RGB Object Apr 29, 2024
@Zirafnik
Copy link
Author

Removing greyscale number return type from .metadata() would be a breaking change for anybody that confidently relies on receiving just the number.

Adding additional parameter type for .flatten() background to include a number, is not a breaking change.

@lovell
Copy link
Owner

lovell commented Apr 29, 2024

I'm OK with this being a breaking change, as part of a future possible v0.34.0, as it simplifies the API so that the background metadata can always be parsed by the color package and therefore operations that use it such as flatten().

Perhaps a single-channel input could return something like { gray: value } instead? The color package accepts values in the range 0-100 so we would need to scale from libvips' 0-255 range.

@lovell lovell changed the title Enhancement: ensure metadata.background always returns RGB Object Enhancement: ensure metadata.background is compatible with color package for single-channel images Apr 30, 2024
@lovell lovell added this to the v0.34.0 milestone Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants