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.

Sign up for GitHub

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

Jump to bottom

Next/future/image blur placeholder have undefined to view box SVG width & height #41393

Closed
1 task done
Balkeo opened this issue Oct 13, 2022 · 5 comments · Fixed by #41573
Closed
1 task done

Next/future/image blur placeholder have undefined to view box SVG width & height #41393

Balkeo opened this issue Oct 13, 2022 · 5 comments · Fixed by #41573
Assignees
@styfle
Labels
Image (next/image) Related to Next.js Image Optimization.

Comments

@Balkeo
Copy link

Balkeo commented Oct 13, 2022 •
edited

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: linux
  Arch: x64
  Version: #25-Ubuntu SMP Wed Mar 30 15:54:22 UTC 2022
Binaries:
  Node: 18.9.0
  npm: 8.19.1
  Yarn: 1.22.10
  pnpm: N/A
Relevant packages:
  next: 12.3.1
  eslint-config-next: 12.3.1
  react: 18.2.0
  react-dom: 18.2.0

What browser are you using? (if relevant)

Brave Version 1.44.108 Chromium: 106.0.5249.103

How are you deploying your application? (if relevant)

Vercel

Describe the Bug

Wrong SVG viewBox generated when using placeHolder='blur' with image filling his parent

  • The blur base64

  • The SVG using the blur base64

As we can see in the src/blur.svg the viewBox have weird values

<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 undefined undefined'>
    <filter id='b' color-interpolation-filters='sRGB'>
        <feGaussianBlur stdDeviation='20'/>
        <feComponentTransfer>
            <feFuncA type='discrete' tableValues='1 1'/>
        </feComponentTransfer>
    </filter>
    <image filter='url(#b)' x='0' y='0' height='100%' width='100%' href='data:image/jpeg;base64[...]'/>
</svg>

So the svg have this weird artifacts and the black borders

Expected Behavior

I expected the placeHolder to be rendered without artefacts and with a corect viewBox for the SVG not undefined in width & height

Link to reproduction

https://next-future-image-issue-sigma.vercel.app/

To Reproduce

A repo with all the need to reproduce
https://github.com/Balkeo/next-future-image-issue
@Balkeo Balkeo added the bug Issue was opened via the bug report template. label Oct 13, 2022
@balazsorban44 balazsorban44 added the Image (next/image) Related to Next.js Image Optimization. label Oct 13, 2022
@balazsorban44
Copy link
Member

Looks like a duplicate of #40419

@shouze
Copy link

shouze commented Oct 13, 2022

@balazsorban44 maybe, not sure... but probably related 😉

@trevorpfiz
Copy link

trevorpfiz commented Oct 15, 2022 •
edited

next-future-image-blur-bug
Similar issue. Here is a look at JPG images from Sanity loading in. Seems that the aspect ratio of the image can make it worse than another image? blurDataURL is coming from lqip from Sanity. Using Chrome.

Also, for whatever reason, when I am refreshing with next/future/image on localhost I am not able to see the blur effect. Has made troubleshooting tough because I have had to view attempted fixes on production.

@styfle styfle added kind: bug and removed bug Issue was opened via the bug report template. labels Oct 19, 2022
@styfle styfle self-assigned this Oct 19, 2022
@styfle styfle mentioned this issue Oct 19, 2022
Merged
@styfle
Copy link
Member

styfle commented Oct 19, 2022

Thanks for reporting! We'll get this fixed in #41573

ijjk pushed a commit that referenced this issue Oct 19, 2022
@styfle
Fix next/image svg blur placeholder with fill (#41573)
fe762d3 
This PR fixes two bugs:

- Fixes #40419 
- Fixes #41393 

The first is when the aspect ratio of the `width` and `height` does not
match the aspect ratio of the `blurDataURL` provided. This can result in
artifacts around the edges. The solution is to add
`preserveAspectRatio="none"`.

The second is when there is no `width` or `height` provided (which is
normal when using `fill`) so the viewBox was undefined. This can also
cause artifacts around the edges. The solution is to change the blur
technique from gaussian to css filter, similar to `next/legacy/image`.

Note: css blur might be [slower in
firefox](https://bugzilla.mozilla.org/show_bug.cgi?id=925025) which is
why we'll only use it for this corner case.
Kikobeats pushed a commit to Kikobeats/next.js that referenced this issue Oct 24, 2022
@styfle @Kikobeats
Fix next/image svg blur placeholder with fill (vercel#41573)
6ab2036 
This PR fixes two bugs:

- Fixes vercel#40419 
- Fixes vercel#41393 

The first is when the aspect ratio of the `width` and `height` does not
match the aspect ratio of the `blurDataURL` provided. This can result in
artifacts around the edges. The solution is to add
`preserveAspectRatio="none"`.

The second is when there is no `width` or `height` provided (which is
normal when using `fill`) so the viewBox was undefined. This can also
cause artifacts around the edges. The solution is to change the blur
technique from gaussian to css filter, similar to `next/legacy/image`.

Note: css blur might be [slower in
firefox](https://bugzilla.mozilla.org/show_bug.cgi?id=925025) which is
why we'll only use it for this corner case.
@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@styfle styfle

Labels
Image (next/image) Related to Next.js Image Optimization.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix next/image svg blur placeholder with fill vercel/next.js
5 participants
@shouze @styfle @Balkeo @balazsorban44 @trevorpfiz