[11/30] libcamera: software_isp: gpu_pipeline_shader_pass: Move common shader selection logic into base class in new method initShaders()
diff mbox series

Message ID 20260618122245.946138-12-bryan.odonoghue@linaro.org
State New
Headers show
Series
  • RFC/RFT: gpuisp: Multipass with speed optimisations on top
Related show

Commit Message

Bryan O'Donoghue June 18, 2026, 12:22 p.m. UTC 
Shader initialisation has some common and some pass-specific logic that
needs to happen.

- Common: Environment setup such as defining top-level defines as flags to
  include features when compiling shaders. This will have to be consistent
  from one shader feeding the next.

- Shader specific: Selecting which vertex, fragment and potentially compute
  shaders to run - populating the shader compile environment with any
  defines specific to that shader.

- Common: Compiling and linking a shader program. Error checking and
  associating a compiled vertex/fragment or compute shader with a given
  programId_.

Move the common code to the base class and have the base class call out to
a pure virtual funtion that the derived class must implement.

In this way GpuIspShaderPass::initShaders() calls the derived class
GpuIspShaderPassDerivedClass::selectShader() while encapsulating that call
in the afore mentioned common pieces of logic.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../software_isp/gpu_pipeline_shader_pass.cpp | 118 ++++++++++++++-
 .../software_isp/gpu_pipeline_shader_pass.h   |  11 +-
 .../gpu_pipeline_shader_pass_demosiac.cpp     | 139 +++---------------
 .../gpu_pipeline_shader_pass_demosiac.h       |   3 +-
 4 files changed, 151 insertions(+), 120 deletions(-)

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/gpu_pipeline_shader_pass.cpp b/src/libcamera/software_isp/gpu_pipeline_shader_pass.cpp
index 669a1c1b6..d0d13eef9 100644
--- a/src/libcamera/software_isp/gpu_pipeline_shader_pass.cpp
+++ b/src/libcamera/software_isp/gpu_pipeline_shader_pass.cpp
@@ -36,16 +36,29 @@  LOG_DEFINE_CATEGORY(GpuShaderPass)
 
 int GpuIspShaderPass::process(eGLImage &eglImageIn, eGLImage &eglImageOut, uint32_t width, uint32_t height, const DebayerParams &params)
 {
-	/* Switch to the output framebuffer */
+	GLenum err;
+
 	egl_.useProgram(programId_);
+	err = glGetError();
+	if (err != GL_NO_ERROR)
+		LOG(GpuShaderPass, Error) << "Switch program @ error " << err;
+
+	/* Switch to the output framebuffer */
 	egl_.attachTextureToFBO(eglImageOut);
+	err = glGetError();
+	if (err != GL_NO_ERROR)
+		LOG(GpuShaderPass, Error) << "attachTextureToFBO @ error " << err;
 
 	setShaderVariableValues(params, eglImageIn);
+	err = glGetError();
+	if (err != GL_NO_ERROR)
+		LOG(GpuShaderPass, Error) << "setShaderVariables @ error " << err;
+
 	glViewport(0, 0, width, height);
 	glClear(GL_COLOR_BUFFER_BIT);
-	glDrawArrays(GL_TRIANGLE_FAN, 0, DEBAYER_OPENGL_COORDS);
 
-	GLenum err = glGetError();
+	glDrawArrays(GL_TRIANGLE_FAN, 0, DEBAYER_OPENGL_COORDS);
+	err = glGetError();
 	if (err != GL_NO_ERROR) {
 		LOG(GpuShaderPass, Error) << "Drawing scene fail " << err;
 		return -ENODEV;
@@ -71,4 +84,103 @@  void GpuIspShaderPass::configure(const struct PassConfig &passInputCfg, const st
 	passOutputCfg_ = passOutputCfg;
 }
 
+int GpuIspShaderPass::initShaders(PixelFormat inputFormat, PixelFormat outputFormat)
+{
+	struct ShaderConfig shaderCfg;
+	GLenum err;
+	int ret;
+
+	/* Target gles 100 glsl requires "#version x" as first directive in shader */
+	egl_.pushEnv(shaderCfg.shaderEnv, "#version 100");
+
+	/* Specify GL_OES_EGL_image_external */
+	egl_.pushEnv(shaderCfg.shaderEnv, "#extension GL_OES_EGL_image_external: enable");
+
+	/* The BLC and Demosiac shaders want to know this so do it in the base class */
+	switch (inputFormat) {
+	case libcamera::formats::SBGGR8:
+	case libcamera::formats::SBGGR10_CSI2P:
+	case libcamera::formats::SBGGR12_CSI2P:
+		firstRed_x_ = 1.0;
+		firstRed_y_ = 1.0;
+		break;
+	case libcamera::formats::SGBRG8:
+	case libcamera::formats::SGBRG10_CSI2P:
+	case libcamera::formats::SGBRG12_CSI2P:
+		firstRed_x_ = 0.0;
+		firstRed_y_ = 1.0;
+		break;
+	case libcamera::formats::SGRBG8:
+	case libcamera::formats::SGRBG10_CSI2P:
+	case libcamera::formats::SGRBG12_CSI2P:
+		firstRed_x_ = 1.0;
+		firstRed_y_ = 0.0;
+		break;
+	case libcamera::formats::SRGGB8:
+	case libcamera::formats::SRGGB10_CSI2P:
+	case libcamera::formats::SRGGB12_CSI2P:
+		firstRed_x_ = 0.0;
+		firstRed_y_ = 0.0;
+		break;
+	default:
+		LOG(GpuShaderPass, Error) << "Unsupported input format";
+		return -EINVAL;
+	};
+
+	/*
+	 * Tell shaders how to re-order output taking account of how the pixels
+	 * are actually stored by EGL.
+	 */
+	switch (outputFormat) {
+	case formats::ARGB8888:
+	case formats::XRGB8888:
+		break;
+	case formats::ABGR8888:
+	case formats::XBGR8888:
+		egl_.pushEnv(shaderCfg.shaderEnv, "#define SWAP_BLUE");
+		break;
+	default:
+		LOG(GpuShaderPass, Error) << "Unsupported output format";
+		return -EINVAL;
+	}
+
+	ret = selectShaders(shaderCfg, inputFormat, outputFormat);
+	if (ret) {
+		LOG(GpuShaderPass, Error) << "selectShaders fail";
+		return ret;
+	}
+
+	LOG(GpuShaderPass, Info) << "ShaderPass = " << typeid(this).name();
+
+	if (egl_.compileVertexShader(vertexShaderId_, shaderCfg.vertexShaderData, shaderCfg.vertexShaderDataLen, shaderCfg.shaderEnv)) {
+		LOG(GpuShaderPass, Error) << "Compile vertex shader fail";
+		return -ENODEV;
+	}
+	utils::scope_exit vShaderGuard([&] { glDeleteShader(vertexShaderId_); });
+
+	if (egl_.compileFragmentShader(fragmentShaderId_, shaderCfg.fragmentShaderData, shaderCfg.fragmentShaderDataLen, shaderCfg.shaderEnv)) {
+		LOG(GpuShaderPass, Error) << "Compile fragment shader fail";
+		return -ENODEV;
+	}
+	utils::scope_exit fShaderGuard([&] { glDeleteShader(fragmentShaderId_); });
+
+	if (egl_.linkProgram(programId_, vertexShaderId_, fragmentShaderId_)) {
+		LOG(GpuShaderPass, Error) << "Linking program fail";
+		return -ENODEV;
+	}
+
+	egl_.dumpShaderSource(vertexShaderId_);
+	egl_.dumpShaderSource(fragmentShaderId_);
+
+	/* Ensure we set the programId_ */
+	egl_.useProgram(programId_);
+	err = glGetError();
+	if (err != GL_NO_ERROR) {
+		LOG(GpuShaderPass, Error) << "Use program error " << err;
+		return -ENODEV;
+	}
+
+	return getShaderVariableLocations();
+}
+
 }
diff --git a/src/libcamera/software_isp/gpu_pipeline_shader_pass.h b/src/libcamera/software_isp/gpu_pipeline_shader_pass.h
index d55b27a85..a329845ee 100644
--- a/src/libcamera/software_isp/gpu_pipeline_shader_pass.h
+++ b/src/libcamera/software_isp/gpu_pipeline_shader_pass.h
@@ -41,6 +41,14 @@  struct PassConfig {
 	Rectangle window;
 };
 
+struct ShaderConfig {
+	std::vector<std::string> shaderEnv;
+	const unsigned char *vertexShaderData;
+	unsigned int vertexShaderDataLen;
+	const unsigned char *fragmentShaderData;
+	unsigned int fragmentShaderDataLen;
+};
+
 class GpuIspShaderPass : public Object
 {
 public:
@@ -51,7 +59,8 @@  public:
 	virtual int start();
 	virtual void stop();
 	virtual void configure(const struct PassConfig &passInputCfg, const struct PassConfig &passOutputCfg);
-	virtual int initShaders(PixelFormat inputFormat, PixelFormat outputFormat) = 0;
+	virtual int initShaders(PixelFormat inputFormat, PixelFormat outputFormat);
+	virtual int selectShaders(struct ShaderConfig &shaderCfg, PixelFormat &inputFormat, PixelFormat &outputFormat) = 0;
 	virtual int getShaderVariableLocations(void) = 0;
 	virtual void setShaderVariableValues(const DebayerParams &params, eGLImage &eglImageIn) = 0;
 	virtual const char *name() const { return "GpuIspShaderPass"; }
diff --git a/src/libcamera/software_isp/gpu_pipeline_shader_pass_demosiac.cpp b/src/libcamera/software_isp/gpu_pipeline_shader_pass_demosiac.cpp
index 02d9da2f0..b0c431c13 100644
--- a/src/libcamera/software_isp/gpu_pipeline_shader_pass_demosiac.cpp
+++ b/src/libcamera/software_isp/gpu_pipeline_shader_pass_demosiac.cpp
@@ -44,101 +44,41 @@  void GpuIspShaderPassDemosiac::stop()
 {
 }
 
-int GpuIspShaderPassDemosiac::initShaders(PixelFormat inputFormat, PixelFormat outputFormat)
+int GpuIspShaderPassDemosiac::selectShaders(struct ShaderConfig &shaderCfg, [[maybe_unused]]PixelFormat &inputFormat, [[maybe_unused]]PixelFormat &outputFormat)
 {
-	std::vector<std::string> shaderEnv;
-	unsigned int fragmentShaderDataLen = 0;
-	const unsigned char *fragmentShaderData = 0;
-	unsigned int vertexShaderDataLen = 0;
-	const unsigned char *vertexShaderData = 0;
-	GLenum err;
-
-	/* Target gles 100 glsl requires "#version x" as first directive in shader */
-	egl_.pushEnv(shaderEnv, "#version 100");
-
-	/* Specify GL_OES_EGL_image_external */
-	egl_.pushEnv(shaderEnv, "#extension GL_OES_EGL_image_external: enable");
-
-	/*
-	 * Tell shaders how to re-order output taking account of how the pixels
-	 * are actually stored by EGL.
-	 */
-	switch (outputFormat) {
-	case formats::ARGB8888:
-	case formats::XRGB8888:
-		break;
-	case formats::ABGR8888:
-	case formats::XBGR8888:
-		egl_.pushEnv(shaderEnv, "#define SWAP_BLUE");
-		break;
-	default:
-		LOG(GpuShaderDemosiac, Error) << "Unsupported output format";
-		return -EINVAL;
-	}
-
 	/* Pixel location parameters */
 	glFormat_ = GL_LUMINANCE;
 	bytesPerPixel_ = 1;
 	shaderStridePixels_ = passInputCfg_.stride;
 
-	switch (inputFormat) {
-	case libcamera::formats::SBGGR8:
-	case libcamera::formats::SBGGR10_CSI2P:
-	case libcamera::formats::SBGGR12_CSI2P:
-		firstRed_x_ = 1.0;
-		firstRed_y_ = 1.0;
-		break;
-	case libcamera::formats::SGBRG8:
-	case libcamera::formats::SGBRG10_CSI2P:
-	case libcamera::formats::SGBRG12_CSI2P:
-		firstRed_x_ = 0.0;
-		firstRed_y_ = 1.0;
-		break;
-	case libcamera::formats::SGRBG8:
-	case libcamera::formats::SGRBG10_CSI2P:
-	case libcamera::formats::SGRBG12_CSI2P:
-		firstRed_x_ = 1.0;
-		firstRed_y_ = 0.0;
-		break;
-	case libcamera::formats::SRGGB8:
-	case libcamera::formats::SRGGB10_CSI2P:
-	case libcamera::formats::SRGGB12_CSI2P:
-		firstRed_x_ = 0.0;
-		firstRed_y_ = 0.0;
-		break;
-	default:
-		LOG(GpuShaderDemosiac, Error) << "Unsupported input format";
-		return -EINVAL;
-	};
-
 	/* Shader selection */
 	switch (inputFormat) {
 	case libcamera::formats::SBGGR8:
 	case libcamera::formats::SGBRG8:
 	case libcamera::formats::SGRBG8:
 	case libcamera::formats::SRGGB8:
-		fragmentShaderData = bayer_unpacked_frag;
-		fragmentShaderDataLen = bayer_unpacked_frag_len;
-		vertexShaderData = bayer_unpacked_vert;
-		vertexShaderDataLen = bayer_unpacked_vert_len;
+		shaderCfg.fragmentShaderData = bayer_unpacked_frag;
+		shaderCfg.fragmentShaderDataLen = bayer_unpacked_frag_len;
+		shaderCfg.vertexShaderData = bayer_unpacked_vert;
+		shaderCfg.vertexShaderDataLen = bayer_unpacked_vert_len;
 		break;
 	case libcamera::formats::SBGGR10_CSI2P:
 	case libcamera::formats::SGBRG10_CSI2P:
 	case libcamera::formats::SGRBG10_CSI2P:
 	case libcamera::formats::SRGGB10_CSI2P:
-		egl_.pushEnv(shaderEnv, "#define RAW10P");
+		egl_.pushEnv(shaderCfg.shaderEnv, "#define RAW10P");
 		if (BayerFormat::fromPixelFormat(inputFormat).packing == BayerFormat::Packing::None) {
-			fragmentShaderData = bayer_unpacked_frag;
-			fragmentShaderDataLen = bayer_unpacked_frag_len;
-			vertexShaderData = bayer_unpacked_vert;
-			vertexShaderDataLen = bayer_unpacked_vert_len;
+			shaderCfg.fragmentShaderData = bayer_unpacked_frag;
+			shaderCfg.fragmentShaderDataLen = bayer_unpacked_frag_len;
+			shaderCfg.vertexShaderData = bayer_unpacked_vert;
+			shaderCfg.vertexShaderDataLen = bayer_unpacked_vert_len;
 			glFormat_ = GL_RG;
 			bytesPerPixel_ = 2;
 		} else {
-			fragmentShaderData = bayer_1x_packed_frag;
-			fragmentShaderDataLen = bayer_1x_packed_frag_len;
-			vertexShaderData = identity_vert;
-			vertexShaderDataLen = identity_vert_len;
+			shaderCfg.fragmentShaderData = bayer_1x_packed_frag;
+			shaderCfg.fragmentShaderDataLen = bayer_1x_packed_frag_len;
+			shaderCfg.vertexShaderData = identity_vert;
+			shaderCfg.vertexShaderDataLen = identity_vert_len;
 			shaderStridePixels_ = passInputCfg_.size.width;
 		}
 		break;
@@ -146,56 +86,25 @@  int GpuIspShaderPassDemosiac::initShaders(PixelFormat inputFormat, PixelFormat o
 	case libcamera::formats::SGBRG12_CSI2P:
 	case libcamera::formats::SGRBG12_CSI2P:
 	case libcamera::formats::SRGGB12_CSI2P:
-		egl_.pushEnv(shaderEnv, "#define RAW12P");
+		egl_.pushEnv(shaderCfg.shaderEnv, "#define RAW12P");
 		if (BayerFormat::fromPixelFormat(inputFormat).packing == BayerFormat::Packing::None) {
-			fragmentShaderData = bayer_unpacked_frag;
-			fragmentShaderDataLen = bayer_unpacked_frag_len;
-			vertexShaderData = bayer_unpacked_vert;
-			vertexShaderDataLen = bayer_unpacked_vert_len;
+			shaderCfg.fragmentShaderData = bayer_unpacked_frag;
+			shaderCfg.fragmentShaderDataLen = bayer_unpacked_frag_len;
+			shaderCfg.vertexShaderData = bayer_unpacked_vert;
+			shaderCfg.vertexShaderDataLen = bayer_unpacked_vert_len;
 			glFormat_ = GL_RG;
 			bytesPerPixel_ = 2;
 		} else {
-			fragmentShaderData = bayer_1x_packed_frag;
-			fragmentShaderDataLen = bayer_1x_packed_frag_len;
-			vertexShaderData = identity_vert;
-			vertexShaderDataLen = identity_vert_len;
+			shaderCfg.fragmentShaderData = bayer_1x_packed_frag;
+			shaderCfg.fragmentShaderDataLen = bayer_1x_packed_frag_len;
+			shaderCfg.vertexShaderData = identity_vert;
+			shaderCfg.vertexShaderDataLen = identity_vert_len;
 			shaderStridePixels_ = passInputCfg_.size.width;
 		}
 		break;
 	};
 
-	/* TODO: move from here to the end of the method into a helper function in the base class
-	 *       this logic will be common to all pipeline instances
-	 */
-	if (egl_.compileVertexShader(vertexShaderId_, vertexShaderData, vertexShaderDataLen, shaderEnv)) {
-		LOG(GpuShaderDemosiac, Error) << "Compile vertex shader fail";
-		return -ENODEV;
-	}
-	utils::scope_exit vShaderGuard([&] { glDeleteShader(vertexShaderId_); });
-
-	if (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData, fragmentShaderDataLen, shaderEnv)) {
-		LOG(GpuShaderDemosiac, Error) << "Compile fragment shader fail";
-		return -ENODEV;
-	}
-	utils::scope_exit fShaderGuard([&] { glDeleteShader(fragmentShaderId_); });
-
-	if (egl_.linkProgram(programId_, vertexShaderId_, fragmentShaderId_)) {
-		LOG(GpuShaderDemosiac, Error) << "Linking program fail";
-		return -ENODEV;
-	}
-
-	egl_.dumpShaderSource(vertexShaderId_);
-	egl_.dumpShaderSource(fragmentShaderId_);
-
-	/* Ensure we set the programId_ */
-	egl_.useProgram(programId_);
-	err = glGetError();
-	if (err != GL_NO_ERROR) {
-		LOG(GpuShaderDemosiac, Error) << "Use program error " << err;
-		return -ENODEV;
-	}
-
-	return getShaderVariableLocations();
+	return 0;
 }
 
 int GpuIspShaderPassDemosiac::getShaderVariableLocations(void)
diff --git a/src/libcamera/software_isp/gpu_pipeline_shader_pass_demosiac.h b/src/libcamera/software_isp/gpu_pipeline_shader_pass_demosiac.h
index 60f175ad3..c83024bc4 100644
--- a/src/libcamera/software_isp/gpu_pipeline_shader_pass_demosiac.h
+++ b/src/libcamera/software_isp/gpu_pipeline_shader_pass_demosiac.h
@@ -37,10 +37,11 @@  public:
 	void stop();
 
 	/* Things that every ISP pipeline pass will need to do */
-	int initShaders(PixelFormat inputFormat, PixelFormat outputFormat);
 	int getShaderVariableLocations(void);
 	void setShaderVariableValues(const DebayerParams &params, eGLImage &eglImageIn);
+	int selectShaders(struct ShaderConfig &shaderCfg, PixelFormat &inputFormat, PixelFormat &outputFormat);
 	const char *name() const override { return "GpuIspShaderPassDemosiac"; }
+
 private:
 	/* Shader parameters */
 	GLint textureUniformStep_;