Gatsby: [gatsby-plugin-sharp] Density should be ignored for non-SVG images

[jlengstorf]

We saw a few reports of itty bitty images come in from @joelhooks and @kentcdodds. The max-width from gatsby-remark-images was coming out with tiny values like 38px, which, of course, is bad.

Digging into this, @ChristopherBiscardi and I (with an assist from the all-knowing @pieh) traced the issue back to gatsby-plugin-sharp and this code:
https://github.com/gatsbyjs/gatsby/blob/74e1063f57916f04b853f7e5aae466ce72a2ff73/packages/gatsby-plugin-sharp/src/index.js#L380-L383

Further sleuthing from Chris found a couple comments from @lovell (https://github.com/lovell/sharp/issues/967#issuecomment-333180519 and https://github.com/lovell/sharp/issues/823#issuecomment-332742955) that say the density option isn't supported for JPG/PNG.

And... since we only process JPG/PNG with gatsby-plugin-sharp, we should probably drop the sizeByPixelDensity option altogether.

At minimum, we should ignore the option when format is non-vector.

For now we've opened PRs for egghead (https://github.com/eggheadio/how-to-egghead/pull/46) and Kent (https://github.com/kentcdodds/kentcdodds.com/pull/108) to disable the option, but we should probably come up with a better solution that "don't use it". 😅

[jlengstorf]

I _think_ the kludgy fix here would be to replace lines 380–383 with:

const pixelRatio = 1

😄

[paschalidi]

@jlengstorf can you please describe how one could reproduce this so that I could have a closer look please?

[jlengstorf]

@paschalidi hey! I don't know of a reliable reproduction (that was what made this such a confusing bug for people) but your note was a reminder to open a PR to turn this option into a no-op. See #13852

Thanks!

[paschalidi]

@jlengstorf well glad I helped somehow 🤔 👍

[zubko]

@jlengstorf @KyleAMathews

Hi, sorry for posting into closed issue, but I really need a way to render small images nicely on a retina screen. (imageWidth < options.maxWidth) and I think it's an important feature for the whole system of automatically managing all images - if the result of the automation isn't the best then the whole automation will loose to the manual handling.

If sharp's metadata cannot provide pixelRatio reliably, can pixelRatio be decided from the file name for example? (like it's done on iOS) If the filename ends with @2x.png, @3x.png etc that means its pixel ratio is 2, 3 etc. So if my file name is '[email protected]' and the size is 500px then its max-width should be set to 250px. (or these @2x @3x could break some SEO ? i'm not an expert. but users for sure don't care much what is the name of the file)

The use case: take for example this website made with Gatsby:
https://howtoegghead.com/create-your-30second-lesson
The images here could look much nicer on retina.

[zubko]

Here is a crude version of that idea. It works for my case. If you agree I can improve it to PR.

diff --git a/node_modules/gatsby-plugin-sharp/index.js b/node_modules/gatsby-plugin-sharp/index.js
index 0d7b880..466e167 100644
--- a/node_modules/gatsby-plugin-sharp/index.js
+++ b/node_modules/gatsby-plugin-sharp/index.js
@@ -335,11 +335,17 @@ async function fluid({

   let presentationWidth, presentationHeight;

+  let pixelRatio = 1;
+  const match = file.name.match(/@(\d+)x$/);
+  if (match) {
+    pixelRatio = Number(match[1]);
+  }  
+
   if (fixedDimension === `maxWidth`) {
-    presentationWidth = Math.min(options.maxWidth, width);
+    presentationWidth = Math.min(options.maxWidth, Math.floor(width / pixelRatio));
     presentationHeight = Math.round(presentationWidth * (height / width));
   } else {
-    presentationHeight = Math.min(options.maxHeight, height);
+    presentationHeight = Math.min(options.maxHeight, Math.floor(height / pixelRatio));
     presentationWidth = Math.round(presentationHeight * (width / height));
   } // If the users didn't set default sizes, we'll make one.