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

report(thumbnails): increase res and display, reduce number #14679

Merged
merged 2 commits into from
Jan 23, 2023

Conversation

connorjclark
Copy link
Collaborator

@connorjclark connorjclark commented Jan 13, 2023

fixes #14677

  • 10 -> 8 thumbnails
  • Remove down-scaling and re-encoding, resulting in ~2.2x larger images in terms of dimensions; and less quality degradation
  • Increase the display width in the report by 50%
  • Made the various knobs here an option for the advanced user to play with (yo @benschwarz); also allowed the test artifacts not need to be updated

For cnn.com, this roughly doubles the size of the screenshot-thumbnails. Using json-size.js:

screenshot-thumbnails              11%   159895 bytes
->
screenshot-thumbnails              18%   292192 bytes

before:
image

after:
image

@connorjclark connorjclark requested a review from a team as a code owner January 13, 2023 21:10
@connorjclark connorjclark requested review from brendankenny and removed request for a team January 13, 2023 21:10
@benschwarz
Copy link
Contributor

@connorjclark thanks for tagging me, this is a great improvement. Image quality looks much better 🤩

Will context configuration be passed through from global lighthouse config?

A potential optimisation you may want to explore is calculating a checksum for each image. With a checksum, you can use it to de-duplicate the screenshot array. As you'll note, a lot of the screenshots have the exact same content, and therefore the same checksum.

At Calibre we found that by converting to webp and de-duping the image set we were able to reduce screenshot timeline filesize by up to/around 65%. In many cases, de-duping reduced the image set from 10 images to 3-5. Food for thought! 🎂

@connorjclark
Copy link
Collaborator Author

@connorjclark thanks for tagging me, this is a great improvement. Image quality looks much better 🤩

Will context configuration be passed through from global lighthouse config?

You set these options on the config. See #14686

A potential optimisation you may want to explore is calculating a checksum for each image. With a checksum, you can use it to de-duplicate the screenshot array. As you'll note, a lot of the screenshots have the exact same content, and therefore the same checksum.

At Calibre we found that by converting to webp and de-duping the image set we were able to reduce screenshot timeline filesize by up to/around 65%. In many cases, de-duping reduced the image set from 10 images to 3-5. Food for thought! 🎂

It's a good idea, especially if it really is 65%. Only hesitation is that it'd change the shape of the audit result. It's a pretty quick change tho and with 10.0 coming up, I guess we might as well do it.

@connorjclark connorjclark changed the title report(thumbnails): increase res and display size, reduce number report(thumbnails): increase res and display, reduce number Jan 23, 2023
@connorjclark connorjclark merged commit 421844c into main Jan 23, 2023
@connorjclark connorjclark deleted the bigger-thumbnails branch January 23, 2023 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase resolution of filmstrip thumbnails
5 participants