[v2,2/3] libcamera: software_isp: debayer_egl: Check for errors when getting shader variable locations
diff mbox series

Message ID 20261027122844.56616-3-remy.alipoursafari@collabora.com
State New
Headers show
Series
  • Support rockchip-cif and do slight checks on softisp
Related show

Commit Message

Remy Alipoursafari Oct. 27, 2026, 12:28 p.m. UTC 
According to the OpenGL spec, glGet{Attrib,Uniform}Location return -1 if
the name passed does not correspond to an active attribute/uniform
variable in the shader program.

This acts as an early check in case there is something wrong with the
shader program.

Signed-off-by: Remy Alipoursafari <remy.alipoursafari@collabora.com>
---
 src/libcamera/software_isp/debayer_egl.cpp | 26 +++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

Comments

Barnabás Pőcze Oct. 27, 2026, 4:56 p.m. UTC | #1 
2026. 10. 27. 13:28 keltezéssel, Remy Alipoursafari írta:
> According to the OpenGL spec, glGet{Attrib,Uniform}Location return -1 if
> the name passed does not correspond to an active attribute/uniform
> variable in the shader program.
> 
> This acts as an early check in case there is something wrong with the
> shader program.

My only concern is that this has the potential of making debugging the shaders
very inconvenient since optimizations might remove all unused uniforms,
which will trigger a failure here.

Otherwise I think it is reasonably useful.


> 
> Signed-off-by: Remy Alipoursafari <remy.alipoursafari@collabora.com>
> ---
>   src/libcamera/software_isp/debayer_egl.cpp | 26 +++++++++++++++++++++-
>   1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
> index 33933b11..5236e1f9 100644
> --- a/src/libcamera/software_isp/debayer_egl.cpp
> +++ b/src/libcamera/software_isp/debayer_egl.cpp
> @@ -130,6 +130,25 @@ int DebayerEGL::getShaderVariableLocations(void)
>   			    << " tex_bayer_first_red " << textureUniformBayerFirstRed_
>   			    << " proj_matrix " << textureUniformProjMatrix_
>   			    << " tex_lsc " << textureUniformLsc_;
> +
> +	if (
> +		attributeVertex_ == -1 ||
> +		attributeTexture_ == -1 ||
> +		textureUniformBayerDataIn_ == -1 ||
> +		awbUniformDataIn_ == -1 ||
> +		ccmUniformDataIn_ == -1 ||
> +		blackLevelUniformDataIn_ == -1 ||
> +		gammaUniformDataIn_ == -1 ||
> +		contrastExpUniformDataIn_ == -1 ||
> +		textureUniformStep_ == -1 ||
> +		textureUniformSize_ == -1 ||
> +		textureUniformStrideFactor_ == -1 ||
> +		textureUniformBayerFirstRed_ == -1 ||
> +		textureUniformProjMatrix_ == -1 ||
> +		(lscEnabled_ && textureUniformLsc_ == -1)) {
> +		return -1;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -274,7 +293,12 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm
>   		return -ENODEV;
>   	}
>   
> -	return getShaderVariableLocations();
> +	if (getShaderVariableLocations() != 0) {
> +		LOG(Debayer, Error) << "Find shader variables fail";
> +		return -ENODEV;
> +	}
> +
> +	return 0;
>   }
>   
>   int DebayerEGL::configure(const StreamConfiguration &inputCfg,

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
index 33933b11..5236e1f9 100644
--- a/src/libcamera/software_isp/debayer_egl.cpp
+++ b/src/libcamera/software_isp/debayer_egl.cpp
@@ -130,6 +130,25 @@  int DebayerEGL::getShaderVariableLocations(void)
 			    << " tex_bayer_first_red " << textureUniformBayerFirstRed_
 			    << " proj_matrix " << textureUniformProjMatrix_
 			    << " tex_lsc " << textureUniformLsc_;
+
+	if (
+		attributeVertex_ == -1 ||
+		attributeTexture_ == -1 ||
+		textureUniformBayerDataIn_ == -1 ||
+		awbUniformDataIn_ == -1 ||
+		ccmUniformDataIn_ == -1 ||
+		blackLevelUniformDataIn_ == -1 ||
+		gammaUniformDataIn_ == -1 ||
+		contrastExpUniformDataIn_ == -1 ||
+		textureUniformStep_ == -1 ||
+		textureUniformSize_ == -1 ||
+		textureUniformStrideFactor_ == -1 ||
+		textureUniformBayerFirstRed_ == -1 ||
+		textureUniformProjMatrix_ == -1 ||
+		(lscEnabled_ && textureUniformLsc_ == -1)) {
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -274,7 +293,12 @@  int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm
 		return -ENODEV;
 	}
 
-	return getShaderVariableLocations();
+	if (getShaderVariableLocations() != 0) {
+		LOG(Debayer, Error) << "Find shader variables fail";
+		return -ENODEV;
+	}
+
+	return 0;
 }
 
 int DebayerEGL::configure(const StreamConfiguration &inputCfg,