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

[gatsby-plugin-sharp] Fix PNG generation when using the "duotone" option #1506

Merged

Conversation

fk
Copy link
Contributor

@fk fk commented Jul 14, 2017

Options for PNG and JPEG output need to be set prior to raw().toBuffer() so that toFormat() in duotone.js actually works.

Things were working for JPEGs before because they are written toFile() as JPEGs explicitly. PNGs get processed with imagemin before writing them to disk, which requires a valid (PNG) buffer.

When authoring the original feature, I moved the PNG/JPEG output settings after the actual duotone processing (for whatever reason 😬 – I guess I thought they get dropped when doing raw().toBuffer()) – this PR reverts that, and cleans up duotone.js.

Florian Kissling added 5 commits July 14, 2017 18:26
Options for PNG and JPEG output need to be set so that "toFormat" in duotone.js actually works.
This was working for JPEGs before because we are directly writing those "toFile".
PNGs get processed with imagemin before writing them to disk, and require a "correct" (PNG) buffer content.
@KyleAMathews
Copy link
Contributor

@gatsbybot
Copy link
Collaborator

gatsbybot commented Jul 14, 2017

Deploy preview ready!

Built with commit dd9e94b

https://deploy-preview-1506--using-drupal.netlify.com

@gatsbybot
Copy link
Collaborator

gatsbybot commented Jul 14, 2017

Deploy preview ready!

Built with commit dd9e94b

https://deploy-preview-1506--gatsbyjs.netlify.com

@gatsbybot
Copy link
Collaborator

gatsbybot commented Jul 14, 2017

Deploy preview ready!

Built with commit dd9e94b

https://deploy-preview-1506--gatsbygram.netlify.com

}
// @todo remove once we upgrade to sharp v0.18 which
// adds an alias for "jpg" to toFormat
// @see http://sharp.dimens.io/en/stable/changelog/#v0180-30th-may-2017
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like the main blocker for upgrading as we "attention" is the default mode for us for cropping https://github.com/jcupitt/libvips/issues/672#issuecomment-307365416

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the heads up! Didn't have the time to look into the changelog for 0.18 yet, merely stumbled upon that note regarding the alias. "attention" is all I had to mention to the people that I showed plugin-sharp to instantly win them over btw. <3

@KyleAMathews
Copy link
Contributor

Everything looks happy 👍 https://596ebdce8ebdd96db650f3e0--image-processing.netlify.com/

@KyleAMathews KyleAMathews merged commit 27f55de into gatsbyjs:master Jul 19, 2017
@fk
Copy link
Contributor Author

fk commented Jul 19, 2017

👍 @KyleAMathews I actually should have added a PNG-and-duotone example to the image-processing example … :-/. Will do! I wish I could get my head around writing tests for this.

@fk fk deleted the fix-gatsby-plugin-sharp-duotone-with-png branch July 19, 2017 23:49
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.

None yet

3 participants