[v2,4/5] libcamera: shaders: Replace C array with std::array
diff mbox series

Message ID 20260630125214.3327516-5-laurent.pinchart@ideasonboard.com
State New
Headers show
Series
  • libcamera: Improve shader header generation
Related show

Commit Message

Laurent Pinchart June 30, 2026, 12:52 p.m. UTC 
Using C arrays and separate length variables is error-prone. Replace
them with std::array in the generated shader header, and update the
software ISP code accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Changes since v1:

- Drop "unsigned char" from comment block in generated header
---
 src/libcamera/software_isp/debayer_egl.cpp | 23 +++++++---------------
 utils/codegen/gen-shader-header.py         |  9 +++++----
 2 files changed, 12 insertions(+), 20 deletions(-)

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp
index af9b9d426920..1f5fc6a4466d 100644
--- a/src/libcamera/software_isp/debayer_egl.cpp
+++ b/src/libcamera/software_isp/debayer_egl.cpp
@@ -17,6 +17,7 @@ 
 #include <tuple>
 #include <vector>
 
+#include <libcamera/base/span.h>
 #include <libcamera/base/utils.h>
 
 #include <libcamera/formats.h>
@@ -145,10 +146,8 @@  int DebayerEGL::getShaderVariableLocations(void)
 int DebayerEGL::initBayerShaders(PixelFormat inputFormat, 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;
+	Span<const unsigned char> fragmentShaderData;
+	Span<const unsigned char> vertexShaderData;
 	GLenum err;
 
 	/* Target gles 100 glsl requires "#version x" as first directive in shader */
@@ -216,9 +215,7 @@  int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm
 	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;
 		break;
 	case libcamera::formats::SBGGR10_CSI2P:
 	case libcamera::formats::SGBRG10_CSI2P:
@@ -227,16 +224,12 @@  int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm
 		egl_.pushEnv(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;
 			glFormat_ = GL_RG;
 			bytesPerPixel_ = 2;
 		} else {
 			fragmentShaderData = bayer_1x_packed_frag;
-			fragmentShaderDataLen = bayer_1x_packed_frag_len;
 			vertexShaderData = identity_vert;
-			vertexShaderDataLen = identity_vert_len;
 			shaderStridePixels_ = width_;
 		}
 		break;
@@ -247,28 +240,26 @@  int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm
 		egl_.pushEnv(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;
 			glFormat_ = GL_RG;
 			bytesPerPixel_ = 2;
 		} else {
 			fragmentShaderData = bayer_1x_packed_frag;
-			fragmentShaderDataLen = bayer_1x_packed_frag_len;
 			vertexShaderData = identity_vert;
-			vertexShaderDataLen = identity_vert_len;
 			shaderStridePixels_ = width_;
 		}
 		break;
 	};
 
-	if (egl_.compileVertexShader(vertexShaderId_, vertexShaderData, vertexShaderDataLen, shaderEnv)) {
+	if (egl_.compileVertexShader(vertexShaderId_, vertexShaderData.data(),
+				     vertexShaderData.size(), shaderEnv)) {
 		LOG(Debayer, Error) << "Compile vertex shader fail";
 		return -ENODEV;
 	}
 	utils::scope_exit vShaderGuard([&] { glDeleteShader(vertexShaderId_); });
 
-	if (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData, fragmentShaderDataLen, shaderEnv)) {
+	if (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData.data(),
+				       fragmentShaderData.size(), shaderEnv)) {
 		LOG(Debayer, Error) << "Compile fragment shader fail";
 		return -ENODEV;
 	}
diff --git a/utils/codegen/gen-shader-header.py b/utils/codegen/gen-shader-header.py
index fe4f06065f0c..994e19c8e8e0 100755
--- a/utils/codegen/gen-shader-header.py
+++ b/utils/codegen/gen-shader-header.py
@@ -19,15 +19,14 @@  def process_file(path, out):
     hex_data = [f'0x{c:02x}' for c in data]
     var_name = path.name.replace('.', '_')
 
-    out.write(f'unsigned char const {var_name}[] = {{\n')
+    out.write(f'static constexpr std::array<unsigned char, {len(data)}> {var_name}{{\n')
 
     for i in range(math.ceil(len(data) / 16)):
         out.write('\t')
         out.write(', '.join(hex_data[16 * i:16 * (i + 1)]))
         out.write(',\n')
 
-    out.write('};\n\n')
-    out.write(f'const unsigned int {var_name}_len = {len(data)};\n')
+    out.write('};\n')
 
 
 def main(argv):
@@ -46,6 +45,8 @@  def main(argv):
 
 #pragma once
 
+#include <array>
+
 /*
  * List the names of the shaders at the top of header for readability's sake.
  *
@@ -53,7 +54,7 @@  def main(argv):
 
     for path in args.inputs:
         name = path.name.replace('.', '_')
-        args.output.write(f' * unsigned char {name};\n')
+        args.output.write(f' * - {name}\n')
 
     args.output.write('''\
  */