Precompute and cache RGB/XYZ matrices in RgbWorkingSpace - #3181
Precompute and cache RGB/XYZ matrices in RgbWorkingSpace#3181stefannikolei wants to merge 1 commit into
Conversation
Move RgbToCieXyz and CieXyzToRgb matrix construction from Rgb into RgbWorkingSpace, computing both matrices once at construction. Pass the precomputed inverse adaptation matrix through all chromatic adaptation calls to eliminate repeated Matrix4x4.Invert invocations during color conversions.
| /// <param name="matrix">The chromatic adaptation matrix.</param> | ||
| public static void Transform( | ||
| /// <param name="inverseMatrix">The inverse of <paramref name="matrix"/>.</param> | ||
| internal static void Transform( |
There was a problem hiding this comment.
why was this made internal?
| /// <param name="matrix">The chromatic adaptation matrix.</param> | ||
| /// <param name="inverseMatrix">The inverse of <paramref name="matrix"/>.</param> | ||
| /// <returns>The <see cref="CieXyz"/></returns> | ||
| internal static CieXyz Transform( |
There was a problem hiding this comment.
I'm confused now. We have a public and internal version overloads of this method.
There was a problem hiding this comment.
the public ones are the ones without the inverse matrix. I made the methods with the invers matrix internal, to not change the public api. so at the public level we create the inverse matrix by our selfs to not open space for errors.
We can also go the way to only offer the methods with the inverse matrix as a parameter.
|
@stefannikolei A good idea on paper. I'm just confused by the API now. |
I will have a look. |
Should I make all new overloads public? Or only keep the overloads around which accept matrix and inverse? |
2 participants
Prerequisites
Description
Move RgbToCieXyz and CieXyzToRgb matrix construction from Rgb into RgbWorkingSpace, computing both matrices once at construction. Pass the precomputed inverse adaptation matrix through all chromatic adaptation calls to eliminate repeated Matrix4x4.Invert invocations during color conversions.
ColorspaceCieXyzToRgbConvertRgbWorkingSpaceAdapt@JimBobSquarePants What do you think about this?