Skip to content

New check: com.google.fonts/check/color_cpal_brightness#3908

Merged
felipesanches merged 1 commit intofonttools:mainfrom
yanone:new-check-CPAL-brightness
Sep 30, 2022
Merged

New check: com.google.fonts/check/color_cpal_brightness#3908
felipesanches merged 1 commit intofonttools:mainfrom
yanone:new-check-CPAL-brightness

Conversation

@yanone
Copy link
Copy Markdown
Collaborator

@yanone yanone commented Sep 22, 2022

Description

    Layers of a COLRv0 font should not be too dark or too bright. When layer colors are
    set explicitly, they can't be changed and they may turn out illegible against dark or bright backgrounds.
    While traditional color-less fonts
    can be colored in design apps or CSS, a black color definition in a COLRv0
    font actually means that that layer will be rendered in black regardless of the
    background color. This leads to text becoming invisible against a dark background,
    for instance when using a dark theme in a web browser or operating system.

    This check ensures that layer colors are at least 10% bright and at most 90% bright, when not 
    already set to the current color (0xFFFF).

To Do

  • update CHANGELOG.md
  • wait for all checks to pass
  • request a review

@yanone yanone force-pushed the new-check-CPAL-brightness branch from ff482ef to d769575 Compare September 30, 2022 14:00
@yanone yanone mentioned this pull request Sep 30, 2022
3 tasks
Copy link
Copy Markdown
Collaborator

@felipesanches felipesanches left a comment

Choose a reason for hiding this comment

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

All good, except for a bad URL on proposal field.

Comment thread Lib/fontbakery/profiles/googlefonts.py Outdated
This check ensures that layer colors are at least 10% bright and at most 90% bright, when not
already set to the current color (0xFFFF).
""",
proposal = 'https://github.com/googlefonts/fontbakery/issues/3886'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This URL must be fixed.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't worry. I'm fixing it myself right now.

felipesanches pushed a commit to yanone/fontbakery that referenced this pull request Sep 30, 2022
@felipesanches felipesanches force-pushed the new-check-CPAL-brightness branch from 0915ae7 to b50b697 Compare September 30, 2022 17:19
@felipesanches felipesanches force-pushed the new-check-CPAL-brightness branch from b50b697 to 17985b1 Compare September 30, 2022 19:02
@felipesanches felipesanches merged commit 9b6a0b7 into fonttools:main Sep 30, 2022
@anthrotype
Copy link
Copy Markdown
Member

This whole check is misguided and confusing (what's "color brightness" anyway, how's that defined?) -- there's two problems:

  1. whether black or white end up being illegible depends on the background, and CPAL version 1 allows specific palettes to be designed for dark or light backgrounds; besides, CPAL can contain more than one palette; this check completely ignores that, just takes the first default palette, assuming it's the only one, and simply goes about complaining that some glyphs are "too dark or too bright" (verbatim). But it's wrong to assume that a given palette entry index maps to only one given color, as there could be several palettes, and some may intentionally be designed for light/dark background.
  2. the check only considers this to be a problem for COLR version 0; why not also COLR version 1? How's that different from the former in the need to "not be too dark or too bright"? I'm pointing this out not much to recommend you include COLR version 1 in this check as well, but to highlight its arbitrariness.

FWIW, I would just remove this check altogether and remind designers about the possibility of using the 0xFFFF special foreground color palette entry index somewhere else (e.g. documentation about creating color fonts).

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