vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.82k stars 26.96k forks source link

NextImage always taking width to shrink images produces blurry result on wide images #37500

Closed MonstraG closed 1 year ago

MonstraG commented 2 years ago

Verify canary release

Provide environment information

Operating System:
  Platform: win32
  Arch: x64
  Version: Windows 10 Home Single Language
Binaries:
  Node: 17.9.0
  npm: N/A
  Yarn: N/A
  pnpm: N/A
Relevant packages:
  next: 12.1.7-canary.31
  react: 18.1.0
  react-dom: 18.1.0

What browser are you using? (if relevant)

Version 104.0.5105.0 (Official Build) canary (64-bit)

How are you deploying your application? (if relevant)

No response

Describe the Bug

image This is a result of hovering over the image link in chrome devtools.

NextImage takes the width of the image, and resizes it to fit 72x72. But because the image is, approx, 2w:1h, resulting height get squished more, producing a blurry image (96x45, 45 is less then rendered 72)

Expected Behavior

Both dimenions of the requested image should be bigger than the required size.

In this case, I would expect something like 205x96.

To Reproduce

    <NextImage src={url} height={72} width={72} objectFit="cover" />

no additional config in next.config.js, other than the cdn.

MonstraG commented 2 years ago

I've added sizes="256px" in an attempt to make sure that fetched image will still be big enough even if its 3:1, but it didn't work, for some reason "rendered size" became 0x0, which might be a bug?

timfuhrmann commented 2 years ago

The default next/image layout option is intrinsic, which means scale down to fit width of container, up to image size. In this case both width or height represent the rendered width/height in pixels, hence it will affect how large the image appears. Since 72 x 72 is a different aspect ratio than 96 x 45, it's squished.

Additionally it's recommended to use objectFit="cover" in combination with layout="fill".

MonstraG commented 2 years ago

The default next/image layout option is intrinsic, which means scale down to fit width of container, up to image size. In this case both width or height represent the rendered width/height in pixels, hence it will affect how large the image appears. Since 72 x 72 is a different aspect ratio than 96 x 45, it's squished.

Additionally it's recommended to use objectFit="cover" in combination with layout="fill".

If I add layout="fill", it'll just ignore width and height, and download full 100vw image.

timfuhrmann commented 2 years ago

Yes fill grows to fill the container in both axes.

Hard to tell what your desired outcome is, but I recommend to read the docs, they are pretty well written.

MonstraG commented 2 years ago

Okay, I have a big non-square image. I want to display it in a small 72x72 box.

I've tried to do this in two ways: with layout="fill" and with width\height, on a random stock photo. In both cases I get an image with resolution 96x35. Height ends up being less than the rendered image, and I get a blurry result.

import { FC } from "react";
import NextImage from "next/image";

const Test: FC = () => {
    return (
        <>
            <div style={{ width: "72px", height: "72px", position: "relative" }}>
                <NextImage
                    src="https://image.shutterstock.com/image-photo/couple-little-daughter-sit-on-600w-1751378789.jpg"
                    layout="fill"
                    objectFit="cover"
                    sizes="72px"
                />
            </div>
            <div style={{ width: "72px", height: "72px", position: "relative" }}>
                <NextImage
                    src="https://image.shutterstock.com/image-photo/couple-little-daughter-sit-on-600w-1751378789.jpg"
                    objectFit="cover"
                    width={72}
                    height={72}
                />
            </div>
        </>
    );
};

export default Test;

(you will need to add shutterstock to the domains)

module.exports = {
    swcMinify: true,
    reactStrictMode: true,
    images: {
        domains: ["image.shutterstock.com"]
    }
};

image

Instead, I want for next/image to give me an image, where both sides are bigger (or equal) than the size I specified, so that image is never blurry.

timfuhrmann commented 2 years ago

Your first layout fill version should be the correct choice in that case. The problem is sizes="72px" - it's too much downsizing for an image of that ratio to fill a square, use something like sizes="200px" and you should be fine.

https://nextjs.org/docs/api-reference/next/image#sizes

MonstraG commented 2 years ago

Your first layout fill version should be the correct choice in that case. The problem is sizes="72px" - it's too much downsizing for an image of that ratio to fill a square, use something like sizes="200px" and you should be fine.

https://nextjs.org/docs/api-reference/next/image#sizes

That will depend on the ratio, and maybe I have a 10:1 image, then I would need 720px (which is of course will be bad for actually square images), but I do not know it beforehand.

So I want next image to consider both dimentions, instead of only width.

timfuhrmann commented 2 years ago

sizes is optional and doesn't define the actual displayed width, but is used to load smaller versions of an image from srcset. So either no value or 720px won't have any effect on square or any other images except of performance and sometimes quality.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-sizes

So I want next image to consider both dimentions, instead of only width.

Next/image can't automatically do that for you. - You won't be able to handle every image perfectly without knowing their width or height. Besides that 10:1 images should be very rare and cut beforehand to fit a square.

github-actions[bot] commented 1 year ago

Please verify that your issue can be recreated with next@canary.

Why was this issue marked with the please verify canary label?

We noticed the provided reproduction was using an older version of Next.js, instead of canary.

The canary version of Next.js ships daily and includes all features and fixes that have not been released to the stable version yet. You can think of canary as a public beta. Some issues may already be fixed in the canary version, so please verify that your issue reproduces by running npm install next@canary and test it in your project, using your reproduction steps.

If the issue does not reproduce with the canary version, then it has already been fixed and this issue can be closed.

How can I quickly verify if my issue has been fixed in canary?

The safest way is to install next@canary in your project and test it, but you can also search through closed Next.js issues for duplicates or check the Next.js releases. You can also use the GitHub template (preferred), or the CodeSandbox or StackBlitz templates to create a reproduction with canary from scratch.

My issue has been open for a long time, why do I need to verify canary now?

Next.js does not backport bug fixes to older versions of Next.js. Instead, we are trying to introduce only a minimal amount of breaking changes between major releases.

What happens if I don't verify against the canary version of Next.js?

An issue with the please verify canary that receives no meaningful activity (e.g. new comments that acknowledge verification against canary) will be automatically closed and locked after 30 days.

If your issue has not been resolved in that time and it has been closed/locked, please open a new issue, with the required reproduction, using next@canary.

I did not open this issue, but it is relevant to me, what can I do to help?

Anyone experiencing the same issue is welcome to provide a minimal reproduction following the above steps. Furthermore, you can upvote the issue using the :+1: reaction on the topmost comment (please do not comment "I have the same issue" without repro steps). Then, we can sort issues by votes to prioritize.

I think my reproduction is good enough, why aren't you looking into it quicker?

We look into every Next.js issue and constantly monitor open issues for new comments.

However, sometimes we might miss one or two due to the popularity/high traffic of the repository. We apologize, and kindly ask you to refrain from tagging core maintainers, as that will usually not result in increased priority.

Upvoting issues to show your interest will help us prioritize and address them as quickly as possible. That said, every issue is important to us, and if an issue gets closed by accident, we encourage you to open a new one linking to the old issue and we will look into it.

Useful Resources

MonstraG commented 1 year ago

Still reproducible: image

"next": "13.1.7-canary.2",

import Image from "next/image";

<Image
    src={"hide-the-pain-harold.webp"}
    alt="kek"
    height={128}
    width={128}
    style={{ objectFit: "cover", width: 128, height: 128 }}
/>

I guess I may have explained myself badly, but when I put in a value in height and width, I expect the image to be at least this size, so it's never blurry.

expected: both sides of intrinsic size are >= 128.

actual: both sides of intrinsic size are <= 128

balazsorban44 commented 1 year ago

This issue has been automatically closed because it wasn't verified against next@canary. If you think it was closed by accident, please leave a comment. If you are running into a similar issue, please open a new issue with a reproduction. Thank you.

MonstraG commented 1 year ago

Wait what? I did verify that it is still reporducable.

MonstraG commented 1 year ago

Bump?

florianwalther-private commented 1 year ago

I agree with @MonstraG and was also hoping that NextJS would adjust to the smallest dimension, rather than only the width.

In my case, I have a blog where users can upload images which will be shown in a fixed size with center-crop applied. If they upload an ultra-wide image, the resized height is too small and the image becomes blurry (even though the original image was big enough).

Right now, I'm resizing images manually on my server before showing them in next/Image. It would be great if next/Image would handle this better tho.

github-actions[bot] commented 1 year ago

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.