search

fonttools / fontbakery

🧁 A font quality assurance tool for everyone
https://fontbakery.readthedocs.io
Apache License 2.0
548 stars 99 forks source link

New check: article/images (location, resolution and filesize) #4594

Closed m4rc1e closed 5 months ago

m4rc1e commented 6 months ago

We currently don't have any checks for family articles. I'm opening this issue so we can discuss possible checks we'd like to include in our CI for families that have articles.

The following have been proposed:

@nathan-williams for the /images dir. Do we expect it in /ofl/family/article/images or /ofl/family/images?

cc @chrissimpkins @davelab6 @emmamarichal @vv-monsalve @nathan-williams

m4rc1e commented 6 months ago

Nathan also noticed that Ojuju had a massive 1.5mb .gif. These should be converted to mp4.

felipesanches commented 6 months ago

Here's a list of image resolutions sampled from the google/fonts repo and sorted (largest first). Where should we put the max resolution threshold?

felipesanches commented 6 months ago

@m4rc1e, I've renamed the issue to focus on this specific article/images new check proposal.

For additional check proposals, please open separate issues.

felipesanches commented 6 months ago

Look at that!!!

Roboto Flex's massive image has 17.7 mega bytes!

Screenshot from 2024-03-19 23-55-37

m4rc1e commented 6 months ago

@nathan-williams do we care about resolution or just size?

felipesanches commented 6 months ago

@nathan-williams do we care about resolution or just size?

Given the example of Roboto Flex above, yes, I think we should have a max resolution rule.

felipesanches commented 6 months ago

I've merged the PR with the new check implementation so that people can try it (it is marked as an experimental check).

But I'll keep this issue open for additional discussion on proposed checking criteria.

I've set the experimental check to:

vv-monsalve commented 6 months ago
  • Check images/videos are included in an /images directory

Currently, we do not use an /images directory but add the images directly to the article directory next to the ARTICLE.en_us.html file. Do we need this directory, or could we continue adding them to the root of the /article directory?

Will there be a limit on the number of images? Therefore, what is the maximum weight limit for the sum of all images?

nathan-williams commented 6 months ago

Thank you for the summary, @felipesanches. All of that looks good to me.

@vv-monsalve I don't think we need a check for total image load at this time. As for the /images directory, it looks like that isn't necessary after all; Onest is a working example. My suggestion to include that check was based on some images checks in the knowledge_graph.py workflow, which are made redundant by these FontBakery checks. I'm actually okay with dropping that requirement if it makes things easier. No strong preference either way, so whatever the team feels is better for the author.

vv-monsalve commented 6 months ago

I don't think we need a check for total image load at this time.

Okay. In that case, there will also be no limit on the number of images, right?

About the limit for vector Vs. raster images. Couldn't it be the opposite, bigger for raster and smaller for vector? The firsts are usually heavier, and 800kb could be a really low limit for animated GIFs or videos. Or would there be a different limit for them?

Edit: below are the values that we have in the GF-Guide, which would make more sense for each format.

=> MAXSIZE_VECTOR = 800kb
=> MAXSIZE_RASTER = 1.75Mb
felipesanches commented 6 months ago

I agree with @vv-monsalve It seems that we should accept larger rasters, rather than larger vectors.

As for the /images directory, it looks like that isn't necessary after all;

But it could be useful to at least WARN, so that we could slightly nudge the whole library towards some consistency

nathan-williams commented 6 months ago

I can add more context here. The MAXSIZE_RASTER of 1.75MB corresponds to the following types: PNG, JPG/JPEG, JXL, and GIF. MAXSIZE_VECTOR should apply to any other media. These constraints are loosely based on infrastructure limitations under default configurations.

vv-monsalve commented 6 months ago

The MAXSIZE_RASTER of 1.75MB corresponds to the following types: PNG, JPG/JPEG, JXL, and GIF

Thanks, @nathan-williams, for the added info. However, this value is the opposite of what you specified in the chat where you said 1,75 were for vector images. Could you confirm it is 1.75MB for the above formats? and in that case 800 kb for VECTORS?

felipesanches commented 6 months ago

@nathan-williams Could you please check whether it is actually VECTORS that have the 800kb limit? We're suspecting that the numbers may be mixed, because vectors are usually smaller than raster images.

davelab6 commented 6 months ago

Nate was/is out this week so I'll follow up with him on Monday :)

Wow yes, that Roboto Flex hero.png is 3840 × 2160 and optipng makes it 17,658,525 bytes (a 15,415 byte, 0.09% decrease...) So I added a downscaled version to https://github.com/google/fonts/pull/7518 which I started to deal with the 1.5Mb GIF in Ojuju.

I made a few and in macOS Chrome Inspector I found the image is only rendered just under 900px wide anyway due the page layout, which came it at 900kb, but a JPEG (80% compression) version of the same resolution is ~166kb optimized:

$ sips -Z 800 hero.png -o hero-800.png
$ sips -Z 800 -s format jpeg -s formatOptions 80 hero.png -o hero-800.jpg
$ ls -l
  899,639 hero-800.png
  176,465 hero-800.jpg
$ jpegoptim hero-800.jpg 
hero-800.jpg 800x450 24bit N JFIF,Exif [OK] 176,465 --> 166,258 bytes (5.78%), optimized.

So... probably we should have a "gf content" profile that has a dependency on imagemagick (or sips if running on macOS, a proprietary image converter) to empirically test all the images in the repo, convert them to/from png/jpg and test if the image compression format makes a big difference, and rescale the article images to 900px width proportionally.

davelab6 commented 6 months ago

From Felipe's list of resolutions, I looked up the corresponding list of file sizes currently on main in PNG format, and ran a couple of simple conversions to JPEG, resized to 800px wide, to 1,200px wide, and without a resizing; I put the data in a Google Sheet, and here it is raw:

Article image PNG Filesize PNG Ratio Filesize JPG resized to 800 Ratio Filesize JPG resized to 1200 Ratio Filesize JPG not resized Ratio
brygada1918/article/handwritten_note.png 678,215 100% 95,151 14% 158,158 23% 95,151 14%
pushster/article/suspect.png 164,927 100% 71,757 44% 133,337 81% 26,109 16%
afacad/article/afacad_01_promo.png 2,039,263 100% 49,205 2% 117,482 6% 347,407 17%
andika/article/hero.png 1,326,699 100% 86,786 7% 173,076 13% 226,725 17%
mingzat/article/hero.png 1,326,699 100% 86,786 7% 173,076 13% 226,725 17%
taiheritagepro/article/hero.png 1,326,699 100% 86,786 7% 173,076 13% 226,725 17%
lexend/article/hero.png 3,191,416 100% 72,554 2% 156,507 5% 566,867 18%
lexenddeca/article/hero.png 3,191,416 100% 72,554 2% 156,507 5% 566,867 18%
lexendexa/article/hero.png 3,191,416 100% 72,554 2% 156,507 5% 566,867 18%
lexendgiga/article/hero.png 3,191,416 100% 72,554 2% 156,507 5% 566,867 18%
lexendmega/article/hero.png 3,191,416 100% 72,554 2% 156,507 5% 566,867 18%
lexendpeta/article/hero.png 3,191,416 100% 72,554 2% 156,507 5% 566,867 18%
lexendtera/article/hero.png 3,191,416 100% 72,554 2% 156,507 5% 566,867 18%
lexendzetta/article/hero.png 3,191,416 100% 72,554 2% 156,507 5% 566,867 18%
afacad/article/afacad_03_kapitael.png 1,938,051 100% 60,844 3% 133,182 7% 368,192 19%
climatecrisis/article/hero.png 7,725,616 100% 205,199 3% 470,736 6% 1,743,601 23%
afacad/article/afacad_04_promo.png 1,369,377 100% 63,426 5% 122,851 9% 309,228 23%
afacad/article/afacad_02_promo.png 1,260,195 100% 59,791 5% 123,465 10% 317,012 25%
spacemono/article/hero.png 148,735 100% 39,787 27% 65,295 44% 54,906 37%
spectral/article/hero.png 594,402 100% 55,946 9% 92,400 16% 222,975 38%
bizudgothic/article/hero.png 251,069 100% 134,012 53% 231,472 92% 103,227 41%
bizudmincho/article/hero.png 251,069 100% 134,012 53% 231,472 92% 103,227 41%
bizudpgothic/article/hero.png 251,069 100% 134,012 53% 231,472 92% 103,227 41%
bizudpmincho/article/hero.png 251,069 100% 134,012 53% 231,472 92% 103,227 41%
atkinsonhyperlegible/article/hero.png 234,075 100% 49,868 21% 88,988 38% 146,578 63%
readexpro/article/Readex_Pro.png 240,180 100% 50,425 21% 83,676 35% 152,599 64%
fraunces/article/fraunces.png 31,158 100% 20,194 65% 31,713 102% 20,194 65%
urbanist/article/hero.png 93,894 100% 75,197 80% 123,941 132% 61,988 66%
robotoserif/article/hero.png 205,771 100% 71,964 35% 119,310 58% 155,478 76%
shantellsans/article/Shantell_Sans.png 288,922 100% 65,827 23% 112,142 39% 219,188 76%
tirobangla/article/hero.png 306,144 100% 60,203 20% 102,101 33% 250,093 82%
tirodevanagarihindi/article/hero.png 306,144 100% 60,203 20% 102,101 33% 250,093 82%
tirodevanagarimarathi/article/hero.png 306,144 100% 60,203 20% 102,101 33% 250,093 82%
tirodevanagarisanskrit/article/hero.png 306,144 100% 60,203 20% 102,101 33% 250,093 82%
tirogurmukhi/article/hero.png 306,144 100% 60,203 20% 102,101 33% 250,093 82%
tirokannada/article/hero.png 306,144 100% 60,203 20% 102,101 33% 250,093 82%
tirotamil/article/hero.png 306,144 100% 60,203 20% 102,101 33% 250,093 82%
dynapuff/article/hero.png 313,318 100% 66,154 21% 111,462 36% 292,007 93%
gulzar/article/hero.png 254,802 100% 53,475 21% 92,914 36% 252,195 99%
playpensans/article/PlaypenSans-about1a.png 199,724 100% 91,377 46% 177,063 89% 219,336 110%
questrial/article/hero.png 156,686 100% 51,912 33% 87,857 56% 172,768 110%
edunswactfoundation/article/hero.png 262,931 100% 66,756 25% 117,354 45% 327,138 124%
eduqldbeginner/article/hero.png 262,931 100% 66,756 25% 117,354 45% 327,138 124%
edusabeginner/article/hero.png 262,931 100% 66,756 25% 117,354 45% 327,138 124%
edutasbeginner/article/hero.png 262,931 100% 66,756 25% 117,354 45% 327,138 124%
eduvicwantbeginner/article/hero.png 262,931 100% 66,756 25% 117,354 45% 327,138 124%
fraunces/article/hero.png 258,822 100% 89,147 34% 151,527 59% 360,682 139%
kalnia/article/KalniaTypeface-01.png 86,147 100% 29,729 35% 48,648 56% 122,362 142%
atkinsonhyperlegible/article/circular_forms.png 72,830 100% 27,625 38% 45,060 62% 108,928 150%
playpensans/article/PlaypenSans-about2a.png 69,908 100% 61,792 88% 99,862 143% 124,145 178%
kalnia/article/KalniaTypeface-02.png 79,428 100% 35,188 44% 59,870 75% 142,436 179%
kalnia/article/KalniaTypeface-03.png 121,519 100% 59,466 49% 94,669 78% 218,070 179%
flowblock/article/hero.png 46,703 100% 27,530 59% 46,270 99% 87,090 186%
flowcircular/article/hero.png 46,703 100% 27,530 59% 46,270 99% 87,090 186%
flowrounded/article/hero.png 46,703 100% 27,530 59% 46,270 99% 87,090 186%
redacted/article/hero.png 46,703 100% 27,530 59% 46,270 99% 87,090 186%
redactedscript/article/hero.png 46,703 100% 27,530 59% 46,270 99% 87,090 186%
gulzar/article/gulzar.png 75,911 100% 34,299 45% 59,627 79% 150,872 199%
anekbangla/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
anekdevanagari/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
anekgujarati/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
anekgurmukhi/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
anekkannada/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
aneklatin/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
anekmalayalam/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
anekodia/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
anektamil/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
anektelugu/article/Anek_Allscripts_Heights.png 64,056 100% 52,148 81% 93,233 146% 144,830 226%
zenantique/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zenantiquesoft/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zendots/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zenkakugothicantique/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zenkakugothicnew/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zenkurenaido/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zenloop/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zenmarugothic/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zenoldmincho/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
zentokyozoo/article/hero.png 110,645 100% 88,694 80% 163,512 148% 342,286 309%
playpensans/article/PlaypenSans-about4a.png 49,401 100% 107,273 217% 183,341 371% 215,272 436%
kalnia/article/KalniaTypeface-04.png 33,125 100% 43,862 132% 74,482 225% 173,540 524%
Screenshot 2024-04-04 at 2 11 54 AM

As you can see, playpensans/article/PlaypenSans-about4a.png never benefits from JPG format, even when reduced to 800px the file size more than doubles; but afacad/article/afacad_01_promo.png is 2mb and without resizing JPEG brings it to 347kb, 17% of the original size.

Resizing some images doesn't make any difference as they don't have any sharp shapes (like small text labels) but for some the JPEG format makes it overall a little fuzzy even without resizing. Its only ~80 families so I'll manually review visually and make a PR to update them as needed.

nathan-williams commented 5 months ago

@felipesanches though surprising, the thresholds should indeed be set that way today. These are inferred constraints based on where we encounter timeouts in the processing infrastructure. I'm interested in adjusting timeouts where we can to shift those thresholds, but we aren't going to pursue that right now.

felipesanches commented 5 months ago

@nathan-williams, thanks for the clarification!

felipesanches commented 5 months ago

The initial implementation of this check will be included in today's 0.12.0 release. And any further improvements can be made for the v0.12.1 later this month.

davelab6 commented 5 months ago

@felipesanches I think this can be closed then? :)