| Message ID | 20260407-kbingham-awb-split-v1-5-a39af3f4dc20@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
On Tue, Apr 07, 2026 at 11:01:08PM +0100, Kieran Bingham wrote: > Remove the open coded matrix multiplication as glsl can perform this > directly. Adapt the uniform ccmUniformDataIn_ to ensure we correctly > upload the matrix in row major ordering. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/shaders/bayer_1x_packed.frag | 45 ++---------------------------- > src/libcamera/shaders/bayer_unpacked.frag | 44 ++--------------------------- > src/libcamera/software_isp/debayer_egl.cpp | 12 ++------ > 3 files changed, 7 insertions(+), 94 deletions(-) > > diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag > index 9a1992e219dd066945b3f46ec509f47a31590385..57f0be9b264bdbdf523291dc9b35ab5fd6c9ef6a 100644 > --- a/src/libcamera/shaders/bayer_1x_packed.frag > +++ b/src/libcamera/shaders/bayer_1x_packed.frag > @@ -231,49 +231,8 @@ void main(void) > /* Apply AWB gains, and saturate each channel at sensor range */ > rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); > > - /* > - * CCM is a 3x3 in the format > - * > - * +--------------+----------------+---------------+ > - * | RedRedGain | RedGreenGain | RedBlueGain | > - * +--------------+----------------+---------------+ > - * | GreenRedGain | GreenGreenGain | GreenBlueGain | > - * +--------------+----------------+---------------+ > - * | BlueRedGain | BlueGreenGain | BlueBlueGain | > - * +--------------+----------------+---------------+ > - * > - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin > - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin > - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin > - * > - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); > - * > - * CPU > - * float ccm [] = { > - * RedRedGain, RedGreenGain, RedBlueGain, > - * GreenRedGain, GreenGreenGain, GreenBlueGain, > - * BlueRedGain, BlueGreenGain, BlueBlueGain, > - * }; > - * > - * GPU > - * ccm = { > - * RedRedGain, GreenRedGain, BlueRedGain, > - * RedGreenGain, GreenGreenGain, BlueGreenGain, > - * RedBlueGain, GreenBlueGain, BlueBlueGain, > - * } > - * > - * However the indexing for the mat data-type is column major hence > - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain > - * > - */ > - float rin, gin, bin; > - rin = rgb.r; > - gin = rgb.g; > - bin = rgb.b; > - > - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); > - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); > - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); > + /* Row major Colour Correction Matrix multiplication */ Row-major is related to how data is stored in memory. When writing ccm * rgb that's not relevant any more. You can drop it from the comment. Same below. > + rgb = ccm * rgb; I wonder why that was open-coded. Any idea ? I suppose there's no negative performance impact from this patch, as the compiler should generate the same or better instructions ? > > /* > * Contrast > diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag > index 87e4e4915fe19679943fdcc3d213a0224b89065e..52de0a7901cfac33dbc7b306f1840ec0048cd8ea 100644 > --- a/src/libcamera/shaders/bayer_unpacked.frag > +++ b/src/libcamera/shaders/bayer_unpacked.frag > @@ -134,49 +134,9 @@ void main(void) { > /* Apply AWB gains, and saturate each channel at sensor range */ > rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); > > - /* > - * CCM is a 3x3 in the format > - * > - * +--------------+----------------+---------------+ > - * | RedRedGain | RedGreenGain | RedBlueGain | > - * +--------------+----------------+---------------+ > - * | GreenRedGain | GreenGreenGain | GreenBlueGain | > - * +--------------+----------------+---------------+ > - * | BlueRedGain | BlueGreenGain | BlueBlueGain | > - * +--------------+----------------+---------------+ > - * > - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin > - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin > - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin > - * > - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); > - * > - * CPU > - * float ccm [] = { > - * RedRedGain, RedGreenGain, RedBlueGain, > - * GreenRedGain, GreenGreenGain, GreenBlueGain, > - * BlueRedGain, BlueGreenGain, BlueBlueGain, > - * }; > - * > - * GPU > - * ccm = { > - * RedRedGain, GreenRedGain, BlueRedGain, > - * RedGreenGain, GreenGreenGain, BlueGreenGain, > - * RedBlueGain, GreenBlueGain, BlueBlueGain, > - * } > - * > - * However the indexing for the mat data-type is column major hence > - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain > - * > - */ > - float rin, gin, bin; > - rin = rgb.r; > - gin = rgb.g; > - bin = rgb.b; > > - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); > - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); > - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); > + /* Row major Colour Correction Matrix multiplication */ > + rgb = ccm * rgb; > > /* > * Contrast > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 738036e649224f370bdf9a0cc59b399f8b1066de..1022f595cc71751cd0c9133b3e5755f591224dc4 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -464,15 +464,9 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) > << " textureUniformProjMatrix_ " << textureUniformProjMatrix_; > > GLfloat ccm[9] = { > - params.combinedMatrix[0][0], > - params.combinedMatrix[0][1], > - params.combinedMatrix[0][2], > - params.combinedMatrix[1][0], > - params.combinedMatrix[1][1], > - params.combinedMatrix[1][2], > - params.combinedMatrix[2][0], > - params.combinedMatrix[2][1], > - params.combinedMatrix[2][2], > + params.combinedMatrix[0][0], params.combinedMatrix[1][0], params.combinedMatrix[2][0], > + params.combinedMatrix[0][1], params.combinedMatrix[1][1], params.combinedMatrix[2][1], > + params.combinedMatrix[0][2], params.combinedMatrix[1][2], params.combinedMatrix[2][2], You're now storing the data in column-major order, ... > }; > glUniformMatrix3fv(ccmUniformDataIn_, 1, GL_FALSE, ccm); ... and the third argument matches that. As the libcamera Matrix class stores elements in row-major order, I think it would be clearer to store the data in row-major order in the ccm array as well, and set the third argument to GL_TRUE accordingly. We could then possibly avoid copying the data to a local array (here or as a separate patch). > LOG(Debayer, Debug) << " ccmUniformDataIn_ " << ccmUniformDataIn_ << " data " << params.combinedMatrix; >
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > On Tue, Apr 07, 2026 at 11:01:08PM +0100, Kieran Bingham wrote: >> Remove the open coded matrix multiplication as glsl can perform this >> directly. Adapt the uniform ccmUniformDataIn_ to ensure we correctly > >> upload the matrix in row major ordering. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> src/libcamera/shaders/bayer_1x_packed.frag | 45 ++---------------------------- >> src/libcamera/shaders/bayer_unpacked.frag | 44 ++--------------------------- >> src/libcamera/software_isp/debayer_egl.cpp | 12 ++------ >> 3 files changed, 7 insertions(+), 94 deletions(-) >> >> diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag >> index 9a1992e219dd066945b3f46ec509f47a31590385..57f0be9b264bdbdf523291dc9b35ab5fd6c9ef6a 100644 >> --- a/src/libcamera/shaders/bayer_1x_packed.frag >> +++ b/src/libcamera/shaders/bayer_1x_packed.frag >> @@ -231,49 +231,8 @@ void main(void) >> /* Apply AWB gains, and saturate each channel at sensor range */ >> rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); >> >> - /* >> - * CCM is a 3x3 in the format >> - * >> - * +--------------+----------------+---------------+ >> - * | RedRedGain | RedGreenGain | RedBlueGain | >> - * +--------------+----------------+---------------+ >> - * | GreenRedGain | GreenGreenGain | GreenBlueGain | >> - * +--------------+----------------+---------------+ >> - * | BlueRedGain | BlueGreenGain | BlueBlueGain | >> - * +--------------+----------------+---------------+ >> - * >> - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin >> - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin >> - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin >> - * >> - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); >> - * >> - * CPU >> - * float ccm [] = { >> - * RedRedGain, RedGreenGain, RedBlueGain, >> - * GreenRedGain, GreenGreenGain, GreenBlueGain, >> - * BlueRedGain, BlueGreenGain, BlueBlueGain, >> - * }; >> - * >> - * GPU >> - * ccm = { >> - * RedRedGain, GreenRedGain, BlueRedGain, >> - * RedGreenGain, GreenGreenGain, BlueGreenGain, >> - * RedBlueGain, GreenBlueGain, BlueBlueGain, >> - * } >> - * >> - * However the indexing for the mat data-type is column major hence >> - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain >> - * >> - */ >> - float rin, gin, bin; >> - rin = rgb.r; >> - gin = rgb.g; >> - bin = rgb.b; >> - >> - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); >> - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); >> - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); >> + /* Row major Colour Correction Matrix multiplication */ > > Row-major is related to how data is stored in memory. When writing > > ccm * rgb > > that's not relevant any more. You can drop it from the comment. Same > below. > >> + rgb = ccm * rgb; > > I wonder why that was open-coded. Any idea ? I think the reason was just not realising initially, when transforming the code from CPU ISP, that it can be written as a matrix multiplication directly in GPU ISP. > I suppose there's no negative performance impact from this patch, as > the compiler should generate the same or better instructions ? > >> >> /* >> * Contrast >> diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag >> index 87e4e4915fe19679943fdcc3d213a0224b89065e..52de0a7901cfac33dbc7b306f1840ec0048cd8ea 100644 >> --- a/src/libcamera/shaders/bayer_unpacked.frag >> +++ b/src/libcamera/shaders/bayer_unpacked.frag >> @@ -134,49 +134,9 @@ void main(void) { >> /* Apply AWB gains, and saturate each channel at sensor range */ >> rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); >> >> - /* >> - * CCM is a 3x3 in the format >> - * >> - * +--------------+----------------+---------------+ >> - * | RedRedGain | RedGreenGain | RedBlueGain | >> - * +--------------+----------------+---------------+ >> - * | GreenRedGain | GreenGreenGain | GreenBlueGain | >> - * +--------------+----------------+---------------+ >> - * | BlueRedGain | BlueGreenGain | BlueBlueGain | >> - * +--------------+----------------+---------------+ >> - * >> - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin >> - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin >> - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin >> - * >> - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); >> - * >> - * CPU >> - * float ccm [] = { >> - * RedRedGain, RedGreenGain, RedBlueGain, >> - * GreenRedGain, GreenGreenGain, GreenBlueGain, >> - * BlueRedGain, BlueGreenGain, BlueBlueGain, >> - * }; >> - * >> - * GPU >> - * ccm = { >> - * RedRedGain, GreenRedGain, BlueRedGain, >> - * RedGreenGain, GreenGreenGain, BlueGreenGain, >> - * RedBlueGain, GreenBlueGain, BlueBlueGain, >> - * } >> - * >> - * However the indexing for the mat data-type is column major hence >> - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain >> - * >> - */ >> - float rin, gin, bin; >> - rin = rgb.r; >> - gin = rgb.g; >> - bin = rgb.b; >> >> - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); >> - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); >> - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); >> + /* Row major Colour Correction Matrix multiplication */ >> + rgb = ccm * rgb; >> >> /* >> * Contrast >> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp >> index 738036e649224f370bdf9a0cc59b399f8b1066de..1022f595cc71751cd0c9133b3e5755f591224dc4 100644 >> --- a/src/libcamera/software_isp/debayer_egl.cpp >> +++ b/src/libcamera/software_isp/debayer_egl.cpp >> @@ -464,15 +464,9 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) >> << " textureUniformProjMatrix_ " << textureUniformProjMatrix_; >> >> GLfloat ccm[9] = { >> - params.combinedMatrix[0][0], >> - params.combinedMatrix[0][1], >> - params.combinedMatrix[0][2], >> - params.combinedMatrix[1][0], >> - params.combinedMatrix[1][1], >> - params.combinedMatrix[1][2], >> - params.combinedMatrix[2][0], >> - params.combinedMatrix[2][1], >> - params.combinedMatrix[2][2], >> + params.combinedMatrix[0][0], params.combinedMatrix[1][0], params.combinedMatrix[2][0], >> + params.combinedMatrix[0][1], params.combinedMatrix[1][1], params.combinedMatrix[2][1], >> + params.combinedMatrix[0][2], params.combinedMatrix[1][2], params.combinedMatrix[2][2], > > You're now storing the data in column-major order, ... > >> }; >> glUniformMatrix3fv(ccmUniformDataIn_, 1, GL_FALSE, ccm); > > ... and the third argument matches that. As the libcamera Matrix class > stores elements in row-major order, I think it would be clearer to store > the data in row-major order in the ccm array as well, and set the third > argument to GL_TRUE accordingly. > > We could then possibly avoid copying the data to a local array (here or > as a separate patch). > >> LOG(Debayer, Debug) << " ccmUniformDataIn_ " << ccmUniformDataIn_ << " data " << >> params.combinedMatrix; >>
Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > Remove the open coded matrix multiplication as glsl can perform this > directly. Adapt the uniform ccmUniformDataIn_ to ensure we correctly > upload the matrix in row major ordering. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/shaders/bayer_1x_packed.frag | 45 ++---------------------------- > src/libcamera/shaders/bayer_unpacked.frag | 44 ++--------------------------- > src/libcamera/software_isp/debayer_egl.cpp | 12 ++------ > 3 files changed, 7 insertions(+), 94 deletions(-) > > diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag > index 9a1992e219dd066945b3f46ec509f47a31590385..57f0be9b264bdbdf523291dc9b35ab5fd6c9ef6a 100644 > --- a/src/libcamera/shaders/bayer_1x_packed.frag > +++ b/src/libcamera/shaders/bayer_1x_packed.frag > @@ -231,49 +231,8 @@ void main(void) > /* Apply AWB gains, and saturate each channel at sensor range */ > rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); > > - /* > - * CCM is a 3x3 in the format > - * > - * +--------------+----------------+---------------+ > - * | RedRedGain | RedGreenGain | RedBlueGain | > - * +--------------+----------------+---------------+ > - * | GreenRedGain | GreenGreenGain | GreenBlueGain | > - * +--------------+----------------+---------------+ > - * | BlueRedGain | BlueGreenGain | BlueBlueGain | > - * +--------------+----------------+---------------+ > - * > - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin > - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin > - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin > - * > - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); > - * > - * CPU > - * float ccm [] = { > - * RedRedGain, RedGreenGain, RedBlueGain, > - * GreenRedGain, GreenGreenGain, GreenBlueGain, > - * BlueRedGain, BlueGreenGain, BlueBlueGain, > - * }; > - * > - * GPU > - * ccm = { > - * RedRedGain, GreenRedGain, BlueRedGain, > - * RedGreenGain, GreenGreenGain, BlueGreenGain, > - * RedBlueGain, GreenBlueGain, BlueBlueGain, > - * } > - * > - * However the indexing for the mat data-type is column major hence > - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain > - * > - */ > - float rin, gin, bin; > - rin = rgb.r; > - gin = rgb.g; > - bin = rgb.b; > - > - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); > - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); > - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); > + /* Row major Colour Correction Matrix multiplication */ > + rgb = ccm * rgb; > > /* > * Contrast > diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag > index 87e4e4915fe19679943fdcc3d213a0224b89065e..52de0a7901cfac33dbc7b306f1840ec0048cd8ea 100644 > --- a/src/libcamera/shaders/bayer_unpacked.frag > +++ b/src/libcamera/shaders/bayer_unpacked.frag > @@ -134,49 +134,9 @@ void main(void) { > /* Apply AWB gains, and saturate each channel at sensor range */ > rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); > > - /* > - * CCM is a 3x3 in the format > - * > - * +--------------+----------------+---------------+ > - * | RedRedGain | RedGreenGain | RedBlueGain | > - * +--------------+----------------+---------------+ > - * | GreenRedGain | GreenGreenGain | GreenBlueGain | > - * +--------------+----------------+---------------+ > - * | BlueRedGain | BlueGreenGain | BlueBlueGain | > - * +--------------+----------------+---------------+ > - * > - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin > - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin > - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin > - * > - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); > - * > - * CPU > - * float ccm [] = { > - * RedRedGain, RedGreenGain, RedBlueGain, > - * GreenRedGain, GreenGreenGain, GreenBlueGain, > - * BlueRedGain, BlueGreenGain, BlueBlueGain, > - * }; > - * > - * GPU > - * ccm = { > - * RedRedGain, GreenRedGain, BlueRedGain, > - * RedGreenGain, GreenGreenGain, BlueGreenGain, > - * RedBlueGain, GreenBlueGain, BlueBlueGain, > - * } > - * > - * However the indexing for the mat data-type is column major hence > - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain > - * > - */ > - float rin, gin, bin; > - rin = rgb.r; > - gin = rgb.g; > - bin = rgb.b; > > - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); > - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); > - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); > + /* Row major Colour Correction Matrix multiplication */ > + rgb = ccm * rgb; > > /* > * Contrast > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > index 738036e649224f370bdf9a0cc59b399f8b1066de..1022f595cc71751cd0c9133b3e5755f591224dc4 100644 > --- a/src/libcamera/software_isp/debayer_egl.cpp > +++ b/src/libcamera/software_isp/debayer_egl.cpp > @@ -464,15 +464,9 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) > << " textureUniformProjMatrix_ " << textureUniformProjMatrix_; > > GLfloat ccm[9] = { > - params.combinedMatrix[0][0], > - params.combinedMatrix[0][1], > - params.combinedMatrix[0][2], > - params.combinedMatrix[1][0], > - params.combinedMatrix[1][1], > - params.combinedMatrix[1][2], > - params.combinedMatrix[2][0], > - params.combinedMatrix[2][1], > - params.combinedMatrix[2][2], > + params.combinedMatrix[0][0], params.combinedMatrix[1][0], params.combinedMatrix[2][0], > + params.combinedMatrix[0][1], params.combinedMatrix[1][1], params.combinedMatrix[2][1], > + params.combinedMatrix[0][2], params.combinedMatrix[1][2], params.combinedMatrix[2][2], This gets reformatted to one element per line by the autoformatter. It can be prevented by wrapping the block by /* clang-format off */ ... /* clang-format on */ Or perhaps even better by changing the element order as Laurent suggests and then doing something like GLfloat ccm[9]; std::copy(params.combinedMatrix.data().begin(), params.combinedMatrix.data().end(), ccm); > }; > glUniformMatrix3fv(ccmUniformDataIn_, 1, GL_FALSE, ccm); > LOG(Debayer, Debug) << " ccmUniformDataIn_ " << ccmUniformDataIn_ << " data " << params.combinedMatrix;
On Wed, Apr 08, 2026 at 02:10:06PM +0200, Milan Zamazal wrote: > Kieran Bingham <kieran.bingham@ideasonboard.com> writes: > > > Remove the open coded matrix multiplication as glsl can perform this > > directly. Adapt the uniform ccmUniformDataIn_ to ensure we correctly > > upload the matrix in row major ordering. > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > src/libcamera/shaders/bayer_1x_packed.frag | 45 ++---------------------------- > > src/libcamera/shaders/bayer_unpacked.frag | 44 ++--------------------------- > > src/libcamera/software_isp/debayer_egl.cpp | 12 ++------ > > 3 files changed, 7 insertions(+), 94 deletions(-) > > > > diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag > > index 9a1992e219dd066945b3f46ec509f47a31590385..57f0be9b264bdbdf523291dc9b35ab5fd6c9ef6a 100644 > > --- a/src/libcamera/shaders/bayer_1x_packed.frag > > +++ b/src/libcamera/shaders/bayer_1x_packed.frag > > @@ -231,49 +231,8 @@ void main(void) > > /* Apply AWB gains, and saturate each channel at sensor range */ > > rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); > > > > - /* > > - * CCM is a 3x3 in the format > > - * > > - * +--------------+----------------+---------------+ > > - * | RedRedGain | RedGreenGain | RedBlueGain | > > - * +--------------+----------------+---------------+ > > - * | GreenRedGain | GreenGreenGain | GreenBlueGain | > > - * +--------------+----------------+---------------+ > > - * | BlueRedGain | BlueGreenGain | BlueBlueGain | > > - * +--------------+----------------+---------------+ > > - * > > - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin > > - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin > > - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin > > - * > > - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); > > - * > > - * CPU > > - * float ccm [] = { > > - * RedRedGain, RedGreenGain, RedBlueGain, > > - * GreenRedGain, GreenGreenGain, GreenBlueGain, > > - * BlueRedGain, BlueGreenGain, BlueBlueGain, > > - * }; > > - * > > - * GPU > > - * ccm = { > > - * RedRedGain, GreenRedGain, BlueRedGain, > > - * RedGreenGain, GreenGreenGain, BlueGreenGain, > > - * RedBlueGain, GreenBlueGain, BlueBlueGain, > > - * } > > - * > > - * However the indexing for the mat data-type is column major hence > > - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain > > - * > > - */ > > - float rin, gin, bin; > > - rin = rgb.r; > > - gin = rgb.g; > > - bin = rgb.b; > > - > > - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); > > - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); > > - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); > > + /* Row major Colour Correction Matrix multiplication */ > > + rgb = ccm * rgb; > > > > /* > > * Contrast > > diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag > > index 87e4e4915fe19679943fdcc3d213a0224b89065e..52de0a7901cfac33dbc7b306f1840ec0048cd8ea 100644 > > --- a/src/libcamera/shaders/bayer_unpacked.frag > > +++ b/src/libcamera/shaders/bayer_unpacked.frag > > @@ -134,49 +134,9 @@ void main(void) { > > /* Apply AWB gains, and saturate each channel at sensor range */ > > rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); > > > > - /* > > - * CCM is a 3x3 in the format > > - * > > - * +--------------+----------------+---------------+ > > - * | RedRedGain | RedGreenGain | RedBlueGain | > > - * +--------------+----------------+---------------+ > > - * | GreenRedGain | GreenGreenGain | GreenBlueGain | > > - * +--------------+----------------+---------------+ > > - * | BlueRedGain | BlueGreenGain | BlueBlueGain | > > - * +--------------+----------------+---------------+ > > - * > > - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin > > - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin > > - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin > > - * > > - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); > > - * > > - * CPU > > - * float ccm [] = { > > - * RedRedGain, RedGreenGain, RedBlueGain, > > - * GreenRedGain, GreenGreenGain, GreenBlueGain, > > - * BlueRedGain, BlueGreenGain, BlueBlueGain, > > - * }; > > - * > > - * GPU > > - * ccm = { > > - * RedRedGain, GreenRedGain, BlueRedGain, > > - * RedGreenGain, GreenGreenGain, BlueGreenGain, > > - * RedBlueGain, GreenBlueGain, BlueBlueGain, > > - * } > > - * > > - * However the indexing for the mat data-type is column major hence > > - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain > > - * > > - */ > > - float rin, gin, bin; > > - rin = rgb.r; > > - gin = rgb.g; > > - bin = rgb.b; > > > > - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); > > - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); > > - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); > > + /* Row major Colour Correction Matrix multiplication */ > > + rgb = ccm * rgb; > > > > /* > > * Contrast > > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp > > index 738036e649224f370bdf9a0cc59b399f8b1066de..1022f595cc71751cd0c9133b3e5755f591224dc4 100644 > > --- a/src/libcamera/software_isp/debayer_egl.cpp > > +++ b/src/libcamera/software_isp/debayer_egl.cpp > > @@ -464,15 +464,9 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) > > << " textureUniformProjMatrix_ " << textureUniformProjMatrix_; > > > > GLfloat ccm[9] = { > > - params.combinedMatrix[0][0], > > - params.combinedMatrix[0][1], > > - params.combinedMatrix[0][2], > > - params.combinedMatrix[1][0], > > - params.combinedMatrix[1][1], > > - params.combinedMatrix[1][2], > > - params.combinedMatrix[2][0], > > - params.combinedMatrix[2][1], > > - params.combinedMatrix[2][2], > > + params.combinedMatrix[0][0], params.combinedMatrix[1][0], params.combinedMatrix[2][0], > > + params.combinedMatrix[0][1], params.combinedMatrix[1][1], params.combinedMatrix[2][1], > > + params.combinedMatrix[0][2], params.combinedMatrix[1][2], params.combinedMatrix[2][2], > > This gets reformatted to one element per line by the autoformatter. It > can be prevented by wrapping the block by > > /* clang-format off */ > ... > /* clang-format on */ > > Or perhaps even better by changing the element order as Laurent suggests > and then doing something like > > GLfloat ccm[9]; > std::copy(params.combinedMatrix.data().begin(), params.combinedMatrix.data().end(), ccm); Or glUniformMatrix3fv(ccmUniformDataIn_, 1, GL_TRUE, params.combinedMatrix.data().data()); if we can assume that GLfloat is always a float. > > }; > > glUniformMatrix3fv(ccmUniformDataIn_, 1, GL_FALSE, ccm); > > LOG(Debayer, Debug) << " ccmUniformDataIn_ " << ccmUniformDataIn_ << " data " << params.combinedMatrix;
diff --git a/src/libcamera/shaders/bayer_1x_packed.frag b/src/libcamera/shaders/bayer_1x_packed.frag index 9a1992e219dd066945b3f46ec509f47a31590385..57f0be9b264bdbdf523291dc9b35ab5fd6c9ef6a 100644 --- a/src/libcamera/shaders/bayer_1x_packed.frag +++ b/src/libcamera/shaders/bayer_1x_packed.frag @@ -231,49 +231,8 @@ void main(void) /* Apply AWB gains, and saturate each channel at sensor range */ rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); - /* - * CCM is a 3x3 in the format - * - * +--------------+----------------+---------------+ - * | RedRedGain | RedGreenGain | RedBlueGain | - * +--------------+----------------+---------------+ - * | GreenRedGain | GreenGreenGain | GreenBlueGain | - * +--------------+----------------+---------------+ - * | BlueRedGain | BlueGreenGain | BlueBlueGain | - * +--------------+----------------+---------------+ - * - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin - * - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); - * - * CPU - * float ccm [] = { - * RedRedGain, RedGreenGain, RedBlueGain, - * GreenRedGain, GreenGreenGain, GreenBlueGain, - * BlueRedGain, BlueGreenGain, BlueBlueGain, - * }; - * - * GPU - * ccm = { - * RedRedGain, GreenRedGain, BlueRedGain, - * RedGreenGain, GreenGreenGain, BlueGreenGain, - * RedBlueGain, GreenBlueGain, BlueBlueGain, - * } - * - * However the indexing for the mat data-type is column major hence - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain - * - */ - float rin, gin, bin; - rin = rgb.r; - gin = rgb.g; - bin = rgb.b; - - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); + /* Row major Colour Correction Matrix multiplication */ + rgb = ccm * rgb; /* * Contrast diff --git a/src/libcamera/shaders/bayer_unpacked.frag b/src/libcamera/shaders/bayer_unpacked.frag index 87e4e4915fe19679943fdcc3d213a0224b89065e..52de0a7901cfac33dbc7b306f1840ec0048cd8ea 100644 --- a/src/libcamera/shaders/bayer_unpacked.frag +++ b/src/libcamera/shaders/bayer_unpacked.frag @@ -134,49 +134,9 @@ void main(void) { /* Apply AWB gains, and saturate each channel at sensor range */ rgb = clamp(rgb * awb, vec3(0.0), vec3(1.0) - blacklevel); - /* - * CCM is a 3x3 in the format - * - * +--------------+----------------+---------------+ - * | RedRedGain | RedGreenGain | RedBlueGain | - * +--------------+----------------+---------------+ - * | GreenRedGain | GreenGreenGain | GreenBlueGain | - * +--------------+----------------+---------------+ - * | BlueRedGain | BlueGreenGain | BlueBlueGain | - * +--------------+----------------+---------------+ - * - * Rout = RedRedGain * Rin + RedGreenGain * Gin + RedBlueGain * Bin - * Gout = GreenRedGain * Rin + GreenGreenGain * Gin + GreenBlueGain * Bin - * Bout = BlueRedGain * Rin + BlueGreenGain * Gin + BlueBlueGain * Bin - * - * We upload to the GPU without transposition glUniformMatrix3f(.., .., GL_FALSE, ccm); - * - * CPU - * float ccm [] = { - * RedRedGain, RedGreenGain, RedBlueGain, - * GreenRedGain, GreenGreenGain, GreenBlueGain, - * BlueRedGain, BlueGreenGain, BlueBlueGain, - * }; - * - * GPU - * ccm = { - * RedRedGain, GreenRedGain, BlueRedGain, - * RedGreenGain, GreenGreenGain, BlueGreenGain, - * RedBlueGain, GreenBlueGain, BlueBlueGain, - * } - * - * However the indexing for the mat data-type is column major hence - * ccm[0][0] = RedRedGain, ccm[0][1] = RedGreenGain, ccm[0][2] = RedBlueGain - * - */ - float rin, gin, bin; - rin = rgb.r; - gin = rgb.g; - bin = rgb.b; - rgb.r = (rin * ccm[0][0]) + (gin * ccm[0][1]) + (bin * ccm[0][2]); - rgb.g = (rin * ccm[1][0]) + (gin * ccm[1][1]) + (bin * ccm[1][2]); - rgb.b = (rin * ccm[2][0]) + (gin * ccm[2][1]) + (bin * ccm[2][2]); + /* Row major Colour Correction Matrix multiplication */ + rgb = ccm * rgb; /* * Contrast diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp index 738036e649224f370bdf9a0cc59b399f8b1066de..1022f595cc71751cd0c9133b3e5755f591224dc4 100644 --- a/src/libcamera/software_isp/debayer_egl.cpp +++ b/src/libcamera/software_isp/debayer_egl.cpp @@ -464,15 +464,9 @@ void DebayerEGL::setShaderVariableValues(const DebayerParams ¶ms) << " textureUniformProjMatrix_ " << textureUniformProjMatrix_; GLfloat ccm[9] = { - params.combinedMatrix[0][0], - params.combinedMatrix[0][1], - params.combinedMatrix[0][2], - params.combinedMatrix[1][0], - params.combinedMatrix[1][1], - params.combinedMatrix[1][2], - params.combinedMatrix[2][0], - params.combinedMatrix[2][1], - params.combinedMatrix[2][2], + params.combinedMatrix[0][0], params.combinedMatrix[1][0], params.combinedMatrix[2][0], + params.combinedMatrix[0][1], params.combinedMatrix[1][1], params.combinedMatrix[2][1], + params.combinedMatrix[0][2], params.combinedMatrix[1][2], params.combinedMatrix[2][2], }; glUniformMatrix3fv(ccmUniformDataIn_, 1, GL_FALSE, ccm); LOG(Debayer, Debug) << " ccmUniformDataIn_ " << ccmUniformDataIn_ << " data " << params.combinedMatrix;
Remove the open coded matrix multiplication as glsl can perform this directly. Adapt the uniform ccmUniformDataIn_ to ensure we correctly upload the matrix in row major ordering. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/libcamera/shaders/bayer_1x_packed.frag | 45 ++---------------------------- src/libcamera/shaders/bayer_unpacked.frag | 44 ++--------------------------- src/libcamera/software_isp/debayer_egl.cpp | 12 ++------ 3 files changed, 7 insertions(+), 94 deletions(-)