w3c / fxtf-drafts

Mirror of https://hg.fxtf.org/drafts
https://drafts.fxtf.org/
Other
69 stars 49 forks source link

[filter-effects-1] inconsistent number of decimal places for hueRotate matrix #542

Open Artoria2e5 opened 9 months ago

Artoria2e5 commented 9 months ago

In https://drafts.fxtf.org/filter-effects/#attr-valuedef-type-huerotate (actually scroll down a little), we see:

For type="hueRotate", values is a single one real number value (degrees). A hueRotate operation is equivalent to the following matrix operation: MML where the terms a00, a01, etc. are calculated as follows: feColorMatrix03.mml Thus, the upper left term of the hue matrix turns out to be: feColorMatrix04.mml

The issue is that the two MMLs, https://github.com/w3c/fxtf-drafts/blob/1c75a54c68e937275f55ce3990def0ddc4b3b7e4/filter-effects/mathml/feColorMatrix03.mml and https://github.com/w3c/fxtf-drafts/blob/1c75a54c68e937275f55ce3990def0ddc4b3b7e4/filter-effects/mathml/feColorMatrix04.mml, disagree on their number of decimal places. 03 says "0.213, 0.787", while 04 gives "0.2127, 0.7873".

Artoria2e5 commented 8 months ago

The MMLs are in general a little messed up. The saturation thing also says 0.213. LuminanceToAlpha below says 0.2126.

The saturation thing also doesn’t italicize the "s".

It’s also not luminance but "luma", but that’s wayyyy too late. A note might be added to appease pedants like me though.

svgeesus commented 8 months ago

In general the level of imprecision reflects the age of the specification, and there is little attempt to minimize the effects of cumulative round-off error.

It’s also not luminance but "luma", but that’s wayyyy too late. A note might be added to appease pedants like me though.

Yup, all these calculations are being done on gamma-encoded values instead of in the linear-light domain, which is incorrect, but changing that would alter existing content in very visible ways. I agree that this should at least be noted in the specification.

Given that, there is limited utility in increasing the precision of the constants used in these calculations.

However, if there is interest in doing so, CSS Color 4 defines the conversion in terms of rational numbers, so (looking at the second row of the matrix in lin_sRGB_to_XYZ:

function lin_sRGB_to_XYZ(rgb) {
    // convert an array of linear-light sRGB values to CIE XYZ
    // using sRGB's own white, D65 (no chromatic adaptation)

    var M = [
        [ 506752 / 1228815,  87881 / 245763,   12673 /   70218 ],
        [  87098 /  409605, 175762 / 245763,   12673 /  175545 ],
        [   7918 /  409605,  87881 / 737289, 1001167 / 1053270 ],
    ];
    return multiplyMatrices(M, rgb);
}

and thus:

so it would be easily possible to round these to a consistent 4 significant figures, at least.

Artoria2e5 commented 8 months ago

About that un-italicized "s" in saturation, what editor was used to write those MMLs? I saw the StarMath code and thought "ah, must be LibreOffice", but my LibreOffice 7.0 correctly turned it into a <mi>. It also did some other changes.

image


I have a little idea about writing those four numbers. Maybe we can give them names like Kr, Kg, Kb and just refer to them (and one-minus-them) in the equations? Well, doing so makes it neater, but doesn't do much else because they shouldn't be changed. But I like neat...

svgeesus commented 8 months ago

Maybe we can give them names like Kr, Kg, Kb and just refer to them (and one-minus-them) in the equations?

I agree, that would be clearer.

Artoria2e5 commented 8 months ago

Hmmmmm, there's one bit that can't be replaced directly by K or 1-K: the 0.143 0.140 -0.283 row on the sine part of hueRotate. It should ultimately be derived from them, but to be honest I've never been with this rotation matrix-by-an-axis thing.

Looking at the file history, the three numbers also got left alone when the <pre> values got updated to four decimal places. It sure seems like we've forgotten how we got these numbers.