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

"Properly size image" report is too strict? #11593

Open
bakura10 opened this issue Oct 24, 2020 · 19 comments
Open

"Properly size image" report is too strict? #11593

bakura10 opened this issue Oct 24, 2020 · 19 comments

Comments

@bakura10
Copy link

bakura10 commented Oct 24, 2020

Summary

Hi !

I am trying to get rid of the "Properly image size" but I am facing a wall here. Here are the kind of errors I get:

image

After checking, it appears that I serve a 800px image, the actual displayed size is 362px and as the DPI is 2, so to serve a perfect image I would need to serve 362x2 = 724px images. But as the design is fluid, it depends of course of the screen width.

In my srcset, I have images that range from 300px to 1000px by increment of 100px. The issue is that it seems the tolerance is super small, and I would need to serve much more images (maybe by increment of 20 or 30px) ?

I have two problems with that:

  • The document size would grow quite a lot (especially on product listing pages where I can have up to 50 products, so adding that many srcset will likely outgrows the minor win you can have at serving an exact size.
  • This will make a pretty poor usage of CDN: we are using Shopify to serve image, and whenever you ask for a given size, it generates on the fly a new copy at each edges. This initial resize can take a bit of time (especially if the original image is super large) which means that by having way too many srcset, a lot more of users will face the initial resizing, and the cache will fill much faster and therefore lead again to more resizing.

I would like to know what is the recommended practice here, and if being too strict here actually does not encourage a bad practice that would like to extremely large document size, and poor usage of CDN.

Thanks :)

@patrickhulce
Copy link
Collaborator

Thanks for filing @bakura10!

This audit could do with a bit of loosening, but a few other things to keep in mind too...

  • To avoid explosion of sizes and best utilize a CDN for fluid images it's recommended to select breakpoints that scale with the area of the image (aka the file size) rather than with a single dimension. This would probably solve your case too (though currently might get flagged when the image is smaller).
  • The document size increase per image typically pales in comparison to the savings on the image (~33KB here). If you're finding individual <picture> elements are even just 1KB after gzip, then something is seriously wrong, and they should probably be dynamically generated instead.

For Lighthouse team, inline with file size breakpoint approaches, I think we should ignore up to ~10-20KB of savings even on smaller images to encourage this best practice (if there is picture or srcset already detected, if they didn't use either, then we can keep the lower threshold 😃 ).

@bakura10
Copy link
Author

@patrickhulce Thanks for the feedback. The GZip compression is a good point, I will try to do some test to check the impact.

For the first one I am not sure to follow you there. We have a fluid design so possibly an infinity of different sizes for the image based on the width of the screen. Do you mean maybe that we should provide fixed sizes in the srcset based on the 4-5 most popular sizes?

@patrickhulce
Copy link
Collaborator

Do you mean maybe that we should provide fixed sizes in the srcset based on the 4-5 most popular sizes?

No, for fluid sized situations such as yours I'm suggesting generating sizes in increments according to the area of the image rather than a single dimension (you mentioned that they "range from 300px to 1000px by increment of 100px" on its longest dimension).

Instead have breakpoints for every 0.1 megapixel increase in resolution (width * height, not just width or just height) or by file size like the method described in this post. If we decide to loosen our thresholds this is the sort of strategy we would respect.

@paulirish
Copy link
Member

For Lighthouse team, inline with file size breakpoint approaches, I think we should ignore up to ~10-20KB of savings even on smaller images to encourage this best practice (if there is picture or srcset already detected, if they didn't use either, then we can keep the lower threshold 😃 ).

👍

@sahilcode17
Copy link

is this issue still available to work on?

@patrickhulce
Copy link
Collaborator

Yes go for it @sahilcode17! You'll want to look at the uses-responsive-images audit for this one.

for (const image of images) {
// Give SVG a free pass because creating a "responsive" SVG is of questionable value.
// Ignore CSS images because it's difficult to determine what is a spritesheet,
// and the reward-to-effort ratio for responsive CSS images is quite low https://css-tricks.com/responsive-images-css/.
if (image.mimeType === 'image/svg+xml' || image.isCss) {
continue;
}
// Skip if we couldn't collect natural image size information.
if (!image.naturalDimensions) continue;

@priyang12
Copy link

can i work on this issue if no one is working on this???

@adamraine
Copy link
Member

can i work on this issue if no one is working on this???

Yes

@Mbellsudteen
Copy link

Mbellsudteen commented Oct 2, 2021 via email

@O2Graphics
Copy link

Hello! :-)

Does anyone is working to fix this problem?
I have exactly the same issue as @bakura10. In my case, it's with "Moto G4", and DPI is 3 in Chrome's Lighthouse (or 2.625 for some strange reasons: https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/config/constants.js#L60-L63 and it looks like a hack: #10741 ).

@TripleEquals
Copy link
Contributor

I believe I have the bandwidth to take this on, just looking for some clarification on the intent of the potential fix.

The intent as I understand it, is to allow the audit to pass even if the wasted bytes exceed the IGNORE_THRESHOLD_IN_BYTES threshold so long as the developer used scrcset in an attempt to pass the audit; but also the image should pass only if the wasted bytes is below a new, sliding threshold that is based on the image size. Is that correct?

@connorjclark
Copy link
Collaborator

The intent as I understand it, is to allow the audit to pass even if the wasted bytes exceed the IGNORE_THRESHOLD_IN_BYTES threshold so long as the developer used scrcset in an attempt to pass the audit; but also the image should pass only if the wasted bytes is below a new, sliding threshold that is based on the image size. Is that correct?

Almost–although we want to encourage breakpoints by image area (not width or height), we weren't thinking of making that explicit in the audit. By introducing a IGNORE_THRESHOLD_IN_BYTES_BREAKPOINTS_PRESENT (which is ~10-20KB as @patrickhulce suggested), the audit would fail less often for folks whose breakpoints are based on area (which means that the delta between each image would be larger than if doing by just one dimension).

So no sliding threshold. Just a different threshold, if srcset/was in a picture element (these properties are already on the ImageElement artifact).

To avoid explosion of sizes and best utilize a CDN for fluid images it's recommended to select breakpoints that scale with the area of the image (aka the file size) rather than with a single dimension.

I couldn't find any reference to this in our docs, so we should probably add one. Anyone know the origin of this "best practice"?

@robots4life
Copy link

To avoid explosion of sizes and best utilize a CDN for fluid images it's recommended to select breakpoints that scale with the area of the image (aka the file size) rather than with a single dimension.

I couldn't find any reference to this in our docs, so we should probably add one. Anyone know the origin of this "best practice"?

I checked in https://web.dev/uses-responsive-images/ and https://web.dev/serve-responsive-images/ and see no mention of such Image File Size Breakpoints or how to create them. An official documentation outlining the technique and how to achieve a good test result would be super nice to have.

@bakura10
Copy link
Author

I did not write any follow up on this but I am still having a lot of reports on PageSpeed for this, without any clear way on how to solve the issue. We try to limit the number of srcset image we generate, we tried a lot of different sizes/steps, but those issues always happen. It would be very nice to have some example algorithm on how to solve this.

@goranhorvathr
Copy link

goranhorvathr commented Nov 30, 2022

I second this and raise it 100%. The report does not make any sense. Even with perfect srcset and sizes this thing flags certain images. There is somewhere in official documentation that I dug up" If the rendered/displayed image is at least 4 KB smaller than the actual image size, it will be flagged as improperly sized". I raise you with this screenshot.

buggedBehaviour

Is this 4kib smaller ? I investigated this in depth gents you have bugs. I created my own linter that checks all of the images and all of the properties I assign inside srcset and sizes and even does custom calculations in the background if sizes are incorrect and checks from 165px width to 3000px width. And we developers even when we do a good job or excellent job get questions hey why even though the desktop score is 99 why is this 0.67sec showing there we should fix this this is slowing our website down. Yea right how do I explain this to a client this absolutely has no performance impact??? Additionally besides that I found other bugs which pretty much means that everyone and by that I mean clients respect lighthouse and think oh its 100% accurate but the inconsistency of the tool is horrible. And we all know that by that I mean contributors, google, developers and etc.

@aditya2049
Copy link

Hey! Can I work on this issue?

@bakura10
Copy link
Author

bakura10 commented Mar 4, 2024

Hello,

I would like to do a follow-up on this as I really feel there is a problem in this report. You can try this page: https://prestige-theme-allure.myshopify.com/

I get this error from Lighthouse:

image

The issue happens for this image:

image

However the size is correct. I give the browsers images up to 1600w, and the image rendered on my MacBook as 830px. Considering the 2 screen density, the best image would be an image of 1660px, but as I don't have such an image, the browser picks the best possible match, so the 1600px image (which is actually smaller than the optimal size).

There is definitely no way I could size it better and I don't understand why Lighthouse reports this error here. @patrickhulce can you please explain me what is the error here?

@robots4life
Copy link

I have a similar issue here. https://davidmorrison.ch/

https://ausi.github.io/respimagelint/ reports the sizes and srcset to be correct.

image

However PageSpeed for Desktop flags it.

image

The sizes is (min-width: 2140px) 1984px, calc(94.29vw - 15px) and this is the HTML with the srcset.

<img
	loading="eager"
	fetchpriority="high"
	src="https://media.graphassets.com/g5WFNGErQaydakaw9hJw"
	sizes="(min-width: 2140px) 1984px, calc(94.29vw - 15px)"
	srcset="
	https://media.graphassets.com/resize=width:2048/g5WFNGErQaydakaw9hJw 2048w,
	https://media.graphassets.com/resize=width:1984/g5WFNGErQaydakaw9hJw 1984w,
	https://media.graphassets.com/resize=width:1810/g5WFNGErQaydakaw9hJw 1810w,
	https://media.graphassets.com/resize=width:1630/g5WFNGErQaydakaw9hJw 1630w,
	https://media.graphassets.com/resize=width:1420/g5WFNGErQaydakaw9hJw 1420w,
	https://media.graphassets.com/resize=width:1200/g5WFNGErQaydakaw9hJw 1200w,
	https://media.graphassets.com/resize=width:1170/g5WFNGErQaydakaw9hJw 1170w,
	https://media.graphassets.com/resize=width:850/g5WFNGErQaydakaw9hJw 850w,
	https://media.graphassets.com/resize=width:768/g5WFNGErQaydakaw9hJw 768w,
	https://media.graphassets.com/resize=width:700/g5WFNGErQaydakaw9hJw 700w,
	https://media.graphassets.com/resize=width:650/g5WFNGErQaydakaw9hJw 650w,
	https://media.graphassets.com/resize=width:600/g5WFNGErQaydakaw9hJw 600w,
	https://media.graphassets.com/resize=width:550/g5WFNGErQaydakaw9hJw 550w,
	https://media.graphassets.com/resize=width:500/g5WFNGErQaydakaw9hJw 500w,
	https://media.graphassets.com/resize=width:425/g5WFNGErQaydakaw9hJw 425w,
	https://media.graphassets.com/resize=width:370/g5WFNGErQaydakaw9hJw 370w,
	https://media.graphassets.com/resize=width:320/g5WFNGErQaydakaw9hJw 320w,
	https://media.graphassets.com/resize=width:270/g5WFNGErQaydakaw9hJw 270w,
	https://media.graphassets.com/resize=width:240/g5WFNGErQaydakaw9hJw 240w,
	https://media.graphassets.com/resize=width:180/g5WFNGErQaydakaw9hJw 180w,
	https://media.graphassets.com/resize=width:120/g5WFNGErQaydakaw9hJw 120w"
	width="1984"
	height="1582"
	alt="Hippomania 2011 Oil on cotton"
/>

PageSpeed does not flag it on Mobile though, so there the rules for "Properly size images" are met.

Does anyone know what could possibly cause this ?

https://pagespeed.web.dev/analysis/https-davidmorrison-ch/aw29646ovc?form_factor=desktop

@rgpublic
Copy link

I second this. I'm also quite confused by the report. Please also note that it's just not viable for everyone to use a CDN which creates any resolution on the fly. Even a CDN at some points need to cache all those generated images. And not everyone can create an arbitrary number of breakpoints either. IMHO that's too strict. We currently serve up to 8 different breakpoints. If someone tells me that 12 or 16 breakpoints are better - so be it. Unfortunately, at the moment PSI doesn't even tell me how the supposed savings come to be. If at the very least it showed which image for which specific window size it'd deem more suitable, things would become much clearer.

BTW: Does it really respect the retina resolution? We are currently serving e.g. a 900px image for a full-width 450px window size to get decent quality.

Another related thing PSI gets wrong in my opinion: If we are say somewhere in the middle of a range associated with a certain breakpoint, we should always serve a size according to the upper end of the range. The docs claim we should never serve more pixels than necessary because it's a waste of bandwidth. But if we don't serve perfectly sized images via CDN, we'll always have the problem that we don't want to scale up to avoid quality problems. In summary, I think the policy should be relaxed/resonsidered. Right now it's not really in line with what's actually practical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment