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

Convert all documentation example images to webp using sharp #4329

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

YohanSciubukgian
Copy link

@YohanSciubukgian YohanSciubukgian commented Jun 26, 2024

Convert all documentation example images to WebP format using sharp library
The sharp library has an Apache-2.0 license

Add the following npm run command:

  • npm run convert-images to convert all existing .png images
  • npm run convert-images <example-file-name> to convert a specific image

Global size optimization for all example images:
From 26.9 MB (28,248,974 bytes) in .png
To 9.54 MB (10,007,586 bytes) in .webp

Specifications:

  • Compression is set to -quality 90 which is usually a good tradeoff between size and image quality for standard web usage.
  • This is a Lossy compression as per @HarelM recommendation

These are "thumbnails" for the examples, I don't need it pixel perfect and use a lossless compression, on the contrary, I need the image to be small and represent the example well, much like the addition of lazy.

Originally posted by @HarelM in #4189 (comment)

Note: The integration could be improved by merging the convert-images command to the npm run generate-images command so it generates both PNR and WebP at the same time (or just WebP if we consider that PNG will not be used anymore).

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality.
  • Document any changes to public APIs.
  • Post benchmark scores.
  • Add an entry to CHANGELOG.md under the ## main section.

PS: It's my first PR to this repository, please feel free to guide me on doing it the right way :)

…name>` to convert documentation example images to webp (lossy, quality=90) using sharp library
@codecov-commenter
Copy link

codecov-commenter commented Jun 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.93%. Comparing base (6612f1b) to head (5c4dc9a).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4329   +/-   ##
=======================================
  Coverage   87.92%   87.93%           
=======================================
  Files         246      246           
  Lines       33361    33361           
  Branches     2167     2200   +33     
=======================================
+ Hits        29334    29336    +2     
+ Misses       3044     3024   -20     
- Partials      983     1001   +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -0,0 +1,44 @@
import fs from 'fs';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this script is needed as is, but it would be great if you could add it to the image generation script.
Note that I have made a few changes to this script in the sky branch.

@@ -41,12 +41,12 @@
"pbf": "^3.2.1",
"potpack": "^2.0.0",
"quickselect": "^2.0.0",
"sharp": "^0.33.4",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a devDependency

@@ -155,6 +156,7 @@
"generate-typings": "dts-bundle-generator --export-referenced-types --umd-module-name=maplibregl -o ./dist/maplibre-gl.d.ts ./src/index.ts",
"generate-docs": "typedoc && node --no-warnings --loader ts-node/esm build/generate-docs.ts",
"generate-images": "node --no-warnings --loader ts-node/esm build/generate-doc-images.ts",
"convert-images": "node --no-warnings --loader ts-node/esm build/convert-doc-images.ts",
Copy link
Collaborator

Choose a reason for hiding this comment

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

See my initial comment.

@HarelM
Copy link
Collaborator

HarelM commented Jun 26, 2024

Hmm... seems like github doesn't support showing the content of webp images :-( this creates a problem when one wants to review how the image will look before merging.
Is there a solution to this?
Maybe we can run this script as part of the documentation build instead?

@HarelM
Copy link
Collaborator

HarelM commented Jun 27, 2024

I've merged the sky PR, which created a conflict, no surprise here...

@YohanSciubukgian
Copy link
Author

I have found an issue talking about the .webp limitation in github (still unresolved) Native WebP image support #5470

Hmm... seems like github doesn't support showing the content of webp images :-( this creates a problem when one wants to review how the image will look before merging. Is there a solution to this? Maybe we can run this script as part of the documentation build instead?


Someone found a workarround by just renaming the file extension .webp to any Github supported image format
isaacs/github#976 (comment)

If you rename your .webp file to .gif, GitHub will allow you to upload it. And since browsers parse the images header first, it will automagically work. I just confirmed this on Firefox and Chrome, both on Linux and Android. This works well for issues, pull requests and READMEs.

Originally posted by @AlxHnr in isaacs/github#976 (comment)


@HarelM What is your implementation recommendation based on these feedbacks?

@YohanSciubukgian
Copy link
Author

As another alternative,
Depending on the hosting services you are using, you could rely on a CDN to convert / compress / resize automatically images at GET time to optimize image size and optimize delivery (bandwidth & latencies) capabilities.
it's usually managed via querystring where you could have somehting like this:

/folder/image.png?format=web&quality=90

The response will be the webp converted on the fly by the CDN.

@HarelM
Copy link
Collaborator

HarelM commented Jun 27, 2024

I think the best approach would be to update the generate-docs script to only do that as part of the build process, this way it won't be important to commit images that are optimized, although it will save git storage it will be less important.
That's my 2 cents.

@HarelM
Copy link
Collaborator

HarelM commented Jun 27, 2024

We are using github pages basically. So it doesn't have good CDN type of capabilities nor we plan to do something more complicated. I think keeping it simple is a good approach.

@HarelM
Copy link
Collaborator

HarelM commented Sep 12, 2024

@YohanSciubukgian any updates on this?

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.

3 participants