Myndex / SAPC-APCA

APCA (Accessible Perceptual Contrast Algorithm) is a new method for predicting contrast for use in emerging web standards (WCAG 3) for determining readability contrast. APCA is derived form the SAPC (S-LUV Advanced Predictive Color) which is an accessibility-oriented color appearance model designed for self-illuminated displays.
https://git.apcacontrast.com/documentation/APCAeasyIntro
Other
435 stars 14 forks source link

inconsistent value with really low contrast in `fontLookupAPCA()` #113

Open jonathanpoelen opened 9 months ago

jonathanpoelen commented 9 months ago

Describe the bug

In fontLookupAPCA() from apca-w3, a contrast less than 5 returns 500, 600, 700, 800 and 900 instead of 999

To Reproduce

for (let i = 0; i < 6; ++i)
  console.log(fontLookupAPCA(i).join(', '));
0.00, 999, 999, 999, 999, 999, 999, 999, 999, 999
1.00, 999, 999, 999, 999, 500, 600, 700, 800, 900
                          ~~~  ~~~  ~~~  ~~~  ~~~
2.00, 999, 999, 999, 999, 500, 600, 700, 800, 900
                          ~~~  ~~~  ~~~  ~~~  ~~~
3.00, 999, 999, 999, 999, 500, 600, 700, 800, 900
                          ~~~  ~~~  ~~~  ~~~  ~~~
4.00, 999, 999, 999, 999, 500, 600, 700, 800, 900
                          ~~~  ~~~  ~~~  ~~~  ~~~
5.00, 999, 999, 999, 999, 999, 999, 999, 999, 999

Expected behavior

All values should be 999.

Additional context

The bug is here: https://github.com/Myndex/apca-w3/blob/master/src/apca-w3.js#L571 which selects index 0 corresponding to a header.

  const factor = 0.2; // 1/5 as LUT is in increments of 5
  const index = (contrast == 0) ?
                 1 : (contrast * factor) | 0 ; // LUT row... n|0 is bw floor

The formula should probably look like

  const index = Math.max(1, (contrast * factor) | 0); // LUT row... n|0 is bw floor

Or else everything is out of sync and it should be

  const index = ((contrast * factor) | 0) + 1; // LUT row... n|0 is bw floor

This does not happen using APCAcontrast since the minimum value returned is greater than 7.3 and less than -7.3.

Myndex commented 9 months ago

Contrasts under $L^c ±10$ are invalid, and fonts cut off at $L^c ±30$.

I'm not sure what you mean when you say "expected value is 999" ?

I think what is going on is there is no $L^c\ 5$ row in the LUT, so the $L^c\ 0$ row is being returned... but values under $L^c ±10$ are to be disregarded in the current -w3 version.

In order to use values under $L^c ±10$ there is a code extention that must be installed.

jonathanpoelen commented 9 months ago

My aim was to plot the values -108 to 106 using the result of fontLookupAPCA(). Except that in the -5 to 5 range, the curve doesn't make any sense when it could easily be flat:

0.00, 999, 999, 999, 999, 999, 999, 999, 999, 999
1.00, 999, 999, 999, 999, 999, 999, 999, 999, 999
2.00, 999, 999, 999, 999, 999, 999, 999, 999, 999
3.00, 999, 999, 999, 999, 999, 999, 999, 999, 999
4.00, 999, 999, 999, 999, 999, 999, 999, 999, 999
5.00, 999, 999, 999, 999, 999, 999, 999, 999, 999
Myndex commented 9 months ago

Hi @jonathanpoelen

I see, though for apca-w3, there are no valid values under Lc ±7.

But if you are just taking the iterated output of the fontLookupAPCA() function, I understand the issue.

Fix A

The interim fix is either to set the first element of the

const fontMatrixAscend = [
    ['Lc',100,200,300,400,500,600,700,800,900],
    [....

To

const fontMatrixAscend = [
    ['Lc',999,999,999,999,999,999,999,999,999],
    [....

Fix B

Or add a 5 line to all the related arrays.

const fontMatrixAscend = [
    ['Lc',100,200,300,400,500,600,700,800,900],
    [0,999,999,999,999,999,999,999,999,999],
    [5,999,999,999,999,999,999,999,999,999],  //  Insert this line
    [10,999,999,999,999,999,999,999,999...
;

const fontDeltaAscend = [
    ['∆Lc',100,200,300,400,500,600,700,800,900],
    [0,0,0,0,0,0,0,0,0,0],
    [5,0,0,0,0,0,0,0,0,0],  //  Insert this line
    [10,0,0,0,0,0,0,0,0,0],
;

  const contrastArrayAscend = ['lc',0,5,10,15,20,25,30,35,40,45,50,55,60,65,70,75,80,85,90,95,100,105,110,115,120,125];
//                                    ^ insert the 5

Thank you for reporting, it will be corrected in the next update. Please let me know if I can be of further assistance.

Myndex commented 9 months ago

Also just FYI, the lookups will be replaced with a pure algorithmic approach in the not too distant future. The LUTs are for developmental purposes.