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

feat: use exponential increase for srcset widths #130

Merged
merged 7 commits into from
Nov 26, 2018

Conversation

frederickfogerty
Copy link
Contributor

Description

This PR updates the srcset width-generation logic. The widths now grow exponentially, rather than by a fixed amount. This means that the dimensions of a rendered image are at most 8% from the "pixel-perfect" dimensions.

Steps to test

Ensure that tests still pass.

Copy link
Contributor

@jayeb jayeb left a comment

Choose a reason for hiding this comment

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

Looks good to me, but will look again once tests land.

@frederickfogerty
Copy link
Contributor Author

@jayeb seems like the tests have completed - is this good to go?

@jayeb
Copy link
Contributor

jayeb commented Nov 20, 2018

Sorry, I read a line about tests in the description and thought you were saying that you intended to add tests for this. Would you mind?

@frederickfogerty
Copy link
Contributor Author

The tests currently check whether the number of srcsets generated is correct - I'm not sure what more tests I could add without becoming too coupled to the implementation. Is there anything you had in mind?

@frederickfogerty
Copy link
Contributor Author

Hey @jayeb I've added some more tests as discussed offline - can you have a look please?

package.json Outdated Show resolved Hide resolved
* master:
  chore: add package-lock.json and prettier (#131)
  chore: update travis node version to stable (#133)

# Conflicts:
#	package.json
#	spec/ImgixTagSpec.js
#	src/targetWidths.js
@frederickfogerty frederickfogerty merged commit d18a85f into master Nov 26, 2018
@frederickfogerty frederickfogerty deleted the fred/update-srcset-widths branch November 26, 2018 23:02
@tedw
Copy link

tedw commented May 29, 2019

Hi all,

Just checking out the source code and was curious why 8192 was chosen as the max size. Did anyone notice a performance decrease when adding more then 30 widths to srcset? If so, I’m wondering if it would be worthwhile to add a config option to set a custom MAX_SIZE. I’m happy to open an issue and take a stab at a PR for that if others think it would be useful.

I’m also not totally clear on why exponential growth is preferred. Was this done to save bandwidth for users with smaller screens at the expense of those with larger screens? I guess that makes sense, but was just wondering if there’s another reason I’m missing.

Lastly, is there a performance benefit to rounding the widths up to an even number? I haven’t come across that before so wondering what the reason is.

Really enjoying Imgix so far and appreciate the team’s responsiveness!

Thanks,
Ted

@frederickfogerty
Copy link
Contributor Author

Hey @tedw! I'm on mobile so forgive any spelling mistakes.

8192px is a restriction based on the imgix backend. This is the maximum resolution that will be rendered, so creating srcsets above this width doesn't make much sense.

As for the exponential increase: our reasoning is visual performance is based on how far away the source image is away from the rendered image in percentage points. e.g. a 7200px image displayed at 7272px will look better than a 100px image displayed at 172px. Therefore it was decided to design it so that the displayed image was never more than 8% away from the source image, rather than using a fixed pixel difference. This also has the side effect of reducing the number of image variants, which improves load performance as there will be more frequent cache hits! I hope this makes sense, I can explain more when I'm back at my keyboard if needed.

IIRC the rounding up was to match a backend implementation we had at imgix.

Feel free to follow up with any more questions you might have!

Fred

@jayeb
Copy link
Contributor

jayeb commented May 29, 2019

Great questions, @tedw.

Everything Fred says is true, but I want to offer some clarity on the last bit. There's no imgix backend restriction about rendering images at even widths, but we figured that since screen dimensions are ~always even, and the majority of website multi-column layouts end up with even-pixel column widths, rounding all of the URLs in a srcset to even numbers would slightly increase the number of "direct hits" when browsers were deciding which URL in to choose from the set.

@tedw
Copy link

tedw commented May 30, 2019

@frederickfogerty @jayeb Wow, thanks for the quick replies and background info! Really appreciate you all taking the time to answer my random questions 😄


8192px is a restriction based on the imgix backend

Cool, good to know 👍


7200px image displayed at 7272px will look better than a 100px image displayed at 172px

Sure, that makes sense. FWIW, Firefox and Safari choose the smallest source file that results in a pixel density >= screen resolution, while Chrome and Edge will let the image to stretch ~10–25% of its natural size (e.g. up to 0.8 dpi on 1x displays). I built a demo page a couple years ago to test this . Either way, 8% seems like a reasonable default. It’s too bad browsers can’t allow larger images to stretch a little more before swapping out for the next size up.

I wonder, though, if it would be worth adding a few more large sizes to reduce the total page weight. Since increasing the width of a large image by a certain amount adds more bytes than increasing a small image by that same amount, it seems like it would be better to err on the side of loading a larger image than necessary for small images while trying to match large image size as closely as possible.

I’ll try to run some tests using exponential and linear progressions once I’ve finished the current site I’m working on to see if it actually makes any difference. I suspect the difference it slight unless you have a lot of images on the page. Will keep you posted!


rounding…to even numbers would slightly increase the number of "direct hits"

That’s interesting, I hadn’t thought of that. I wonder if many desktop users resize their browser to < full width? I also know in IE the scrollbar takes up 17px, so images set to 100% could be an odd number even when the browser is maximized (notably setting 100vw does include the scrollbar, demo).

That said, I did a quick check of browser size in Google Analytics using these steps and it seems the overwhelming majority do in fact have an even viewport size. Though, it also showed some viewport were larger than the screens, so not sure how they are calculating those numbers.

Update: Looks like the iPhone 6, 6S, 7, 8, X, and XS are all 375px wide, so perhaps on mobile even numbers are not as common. Thought, I guess they all have 2x resolution so would request an image with an even width.

@tedw
Copy link

tedw commented May 30, 2019

Sorry to bug you again, but I just noticed some of the images on imgix.com seem to be using completely different sizes. Any idea why?

View Sizes

100
200
300
320
400
500
576
600
640
700
720
750
768
800
900
940
1000
1024
1080
1100
1140
1152
1200
1242
1300
1400
1440
1442
1500
1536
1600
1700
1800
1880
1900
1920
2000
2048
2100
2200
2208
2280
2300
2400
2415
2500
2560

@jayeb
Copy link
Contributor

jayeb commented May 30, 2019

Ah, looks like we're still on version 3.1.0 for imgix.com, which had a different strategy for generating srcset widths. Sort of a "cobbler's children have no shoes" situation, I suppose.

@tedw
Copy link

tedw commented May 30, 2019

Ah, gotcha. Can definitely relate to that 😉 Thanks for the info!

@frederickfogerty
Copy link
Contributor Author

I’ll try to run some tests using exponential and linear progressions once I’ve finished the current site I’m working on to see if it actually makes any difference.

One thing I don't think I explained very well before is that even though we may be delivering images that are too large (as you've outlined) and thus the page weight might not be optimal, the page load performance might actually be better. This is because when there are less variants (i.e. less srcset widths), the more likely a particular request is going to be a cache hit. Cache misses are costly and take a long time (relatively) to render. Combine this with the fact that higher resolution devices are more likely to have a better internet connection (someone loading an image at 5000px wide is probably on a desktop with WiFi or Ethernet), and it may be found that the heavier images may load faster compared to have more fine-grained widths.

Anyway, thank you @tedw for all the thought you've outlined here, it's definitely been an interesting read and some food for thought!

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.

3 participants