| Message ID | 20260618122245.946138-12-bryan.odonoghue@linaro.org |
|---|---|
| State | RFC |
| Headers | show |
| Series |
|
| Related | show |
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > 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. s/funtion/function/ > 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(-) > > 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 ¶ms) > { > - /* 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; The method should return, rather than continue, on any of the errors above, shouldn't it? > + > 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(); > +} > + Moving an already moved code? Any chance to avoid this by patch reordering and introducing selectShaders earlier? > } > 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 ¶ms, 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 ¶ms, eGLImage &eglImageIn); > + int selectShaders(struct ShaderConfig &shaderCfg, PixelFormat &inputFormat, PixelFormat &outputFormat); > const char *name() const override { return "GpuIspShaderPassDemosiac"; } > + > private: > /* Shader parameters */ > GLint textureUniformStep_;
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 ¶ms) { - /* 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 ¶ms, 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 ¶ms, eGLImage &eglImageIn); + int selectShaders(struct ShaderConfig &shaderCfg, PixelFormat &inputFormat, PixelFormat &outputFormat); const char *name() const override { return "GpuIspShaderPassDemosiac"; } + private: /* Shader parameters */ GLint textureUniformStep_;
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(-)