Remap ColorMatrix

[elijah-semyonov]

Proposed Changes

Remap 4th column of ColorMatrix to from 0..255 to 0..1 range before passing to skiko.

Testing

Test: GraphicsLayerTest.correctColorMatrix

Issues Fixed

Fixes: JetBrains/compose-multiplatform#3461

[MatkovIvan]

fiddle example doesn't look like valid proof

[MatkovIvan]

Android does it here - https://cs.android.com/android/platform/superproject/+/master:frameworks/base/libs/hwui/jni/ColorFilter.cpp;l=46-54

[MatkovIvan]

Let's do it like there - unwrap loop and use multiply instead of division

[elijah-semyonov]

I'm fine with unrolling loop, but multiply on (1.0 / 255.0) - why? And considering proof, you can just type in the numbers in fiddle and see the result

[MatkovIvan]

you can just type in the numbers in fiddle and see the result

Let's put official docs/spec instead. Or link to actual android implementation at least.

but multiply on (1.0 / 255.0) - why?

I had two reasons here: repeat original implementation and flashbacks from low-level C world. Since here we're inside managed env, I don't think that it's really matter. The old rule in my head is "multiplication usually faster than division, never slower". Again, it's not really matter here, not a blocker at all.

[elijah-semyonov]

The ColorMatrix documentation in code itself describes the expected values, and generated doc is linked in the beginning of function:

compose-multiplatform-core/compose/ui/ui-graphics/src/commonMain/kotlin/androidx/compose/ui/graphics/ColorMatrix.kt

Lines 24 to 62 in f516176

	/**
	* 4x5 matrix for transforming the color and alpha components of a source.
	* The matrix can be passed as single array, and is treated as follows:
	*
	* ```
	* [ a, b, c, d, e,
	* f, g, h, i, j,
	* k, l, m, n, o,
	* p, q, r, s, t ]
	* ```
	*
	* When applied to a color <code>[[R, G, B, A]]</code>, the resulting color
	* is computed as:
	*
	* ```
	* R' = a*R + b*G + c*B + d*A + e;
	* G' = f*R + g*G + h*B + i*A + j;
	* B' = k*R + l*G + m*B + n*A + o;
	* A' = p*R + q*G + r*B + s*A + t;</pre>
	*
	* ```
	* That resulting color <code>[[R', G', B', A']]</code>
	* then has each channel clamped to the <code>0</code> to <code>255</code>
	* range.
	*
	* The sample ColorMatrix below inverts incoming colors by scaling each
	* channel by <code>-1</code>, and then shifting the result up by
	* `255` to remain in the standard color space.
	*
	* ```
	* [ -1, 0, 0, 0, 255,
	* 0, -1, 0, 0, 255,
	* 0, 0, -1, 0, 255,
	* 0, 0, 0, 1, 0 ]
	* ```
	*
	* This is often used as input for [ColorFilter.colorMatrix] and applied at draw time
	* through [Paint.colorFilter]
	*/