| Message ID | 20260618122245.946138-3-bryan.odonoghue@linaro.org |
|---|---|
| State | RFC |
| Headers | show |
| Series |
|
| Related | show |
Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > In our mutli-pass shader design we will use textures to pass the output of s/mutli/multi/ > one compute or fragment shader to the next. Each phase will have its own > distinct name and output. > > A better more accurate name for the thing which we render into is > eglImageRGBAOut_ which has a very obvious meaning - its our RGBA output. s/its/it's/ > We aren't outputting Bayer data here, we are outputting RGBA data so denote > it as such by way of its name. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> In exchange for a promise to enable a spell checker in the editor used to write the commit messages, I won't complain about the naming here: Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/software_isp/software_isp_pipeline_gpu.cpp | 6 +++--- > src/libcamera/software_isp/software_isp_pipeline_gpu.h | 2 +- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp b/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp > index 53bb5e61f..c68a04fff 100644 > --- a/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp > +++ b/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp > @@ -538,7 +538,7 @@ int SoftwareIspPipelineGpu::debayerGPU(FrameBuffer *input, FrameBuffer *output, > } > > /* Generate the output render framebuffer as render to texture */ > - egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, output->planes()[0].fd.get()); > + egl_.createOutputDMABufTexture2D(*eglImageRGBAOut_, output->planes()[0].fd.get()); > > setShaderVariableValues(params); > glViewport(0, 0, width_, height_); > @@ -626,14 +626,14 @@ int SoftwareIspPipelineGpu::start() > eglImageBayerIn_ = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_, height_, inputConfig_.stride, GL_TEXTURE0, 0); > > /* Texture we will render to */ > - eglImageBayerOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1); > + eglImageRGBAOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1); > > return 0; > } > > void SoftwareIspPipelineGpu::stop() > { > - eglImageBayerOut_.reset(); > + eglImageRGBAOut_.reset(); > eglImageBayerIn_.reset(); > > if (programId_) > diff --git a/src/libcamera/software_isp/software_isp_pipeline_gpu.h b/src/libcamera/software_isp/software_isp_pipeline_gpu.h > index eb365d198..6f161e063 100644 > --- a/src/libcamera/software_isp/software_isp_pipeline_gpu.h > +++ b/src/libcamera/software_isp/software_isp_pipeline_gpu.h > @@ -74,7 +74,7 @@ private: > > /* Pointer to object representing input texture */ > std::unique_ptr<eGLImage> eglImageBayerIn_; > - std::unique_ptr<eGLImage> eglImageBayerOut_; > + std::unique_ptr<eGLImage> eglImageRGBAOut_; > > /* Shader parameters */ > float firstRed_x_;
Quoting Milan Zamazal (2026-06-22 19:16:14) > Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes: > > > In our mutli-pass shader design we will use textures to pass the output of > > s/mutli/multi/ > > > one compute or fragment shader to the next. Each phase will have its own > > distinct name and output. > > > > A better more accurate name for the thing which we render into is > > eglImageRGBAOut_ which has a very obvious meaning - its our RGBA output. > > s/its/it's/ > > > We aren't outputting Bayer data here, we are outputting RGBA data so denote > > it as such by way of its name. > > > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > In exchange for a promise to enable a spell checker in the editor used > to write the commit messages, I won't complain about the naming here: > > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > My first thoughts were what happens when we have different outputs like YUV ... but we don't support that yet, and this is moving us to a more modular design which means it should be easier to adapt for different outputs later, and if that happens then we can consider output buffer names, so with the above handled: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > src/libcamera/software_isp/software_isp_pipeline_gpu.cpp | 6 +++--- > > src/libcamera/software_isp/software_isp_pipeline_gpu.h | 2 +- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp b/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp > > index 53bb5e61f..c68a04fff 100644 > > --- a/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp > > +++ b/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp > > @@ -538,7 +538,7 @@ int SoftwareIspPipelineGpu::debayerGPU(FrameBuffer *input, FrameBuffer *output, > > } > > > > /* Generate the output render framebuffer as render to texture */ > > - egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, output->planes()[0].fd.get()); > > + egl_.createOutputDMABufTexture2D(*eglImageRGBAOut_, output->planes()[0].fd.get()); > > > > setShaderVariableValues(params); > > glViewport(0, 0, width_, height_); > > @@ -626,14 +626,14 @@ int SoftwareIspPipelineGpu::start() > > eglImageBayerIn_ = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_, height_, inputConfig_.stride, GL_TEXTURE0, 0); > > > > /* Texture we will render to */ > > - eglImageBayerOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1); > > + eglImageRGBAOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1); > > > > return 0; > > } > > > > void SoftwareIspPipelineGpu::stop() > > { > > - eglImageBayerOut_.reset(); > > + eglImageRGBAOut_.reset(); > > eglImageBayerIn_.reset(); > > > > if (programId_) > > diff --git a/src/libcamera/software_isp/software_isp_pipeline_gpu.h b/src/libcamera/software_isp/software_isp_pipeline_gpu.h > > index eb365d198..6f161e063 100644 > > --- a/src/libcamera/software_isp/software_isp_pipeline_gpu.h > > +++ b/src/libcamera/software_isp/software_isp_pipeline_gpu.h > > @@ -74,7 +74,7 @@ private: > > > > /* Pointer to object representing input texture */ > > std::unique_ptr<eGLImage> eglImageBayerIn_; > > - std::unique_ptr<eGLImage> eglImageBayerOut_; > > + std::unique_ptr<eGLImage> eglImageRGBAOut_; > > > > /* Shader parameters */ > > float firstRed_x_; >
diff --git a/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp b/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp index 53bb5e61f..c68a04fff 100644 --- a/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp +++ b/src/libcamera/software_isp/software_isp_pipeline_gpu.cpp @@ -538,7 +538,7 @@ int SoftwareIspPipelineGpu::debayerGPU(FrameBuffer *input, FrameBuffer *output, } /* Generate the output render framebuffer as render to texture */ - egl_.createOutputDMABufTexture2D(*eglImageBayerOut_, output->planes()[0].fd.get()); + egl_.createOutputDMABufTexture2D(*eglImageRGBAOut_, output->planes()[0].fd.get()); setShaderVariableValues(params); glViewport(0, 0, width_, height_); @@ -626,14 +626,14 @@ int SoftwareIspPipelineGpu::start() eglImageBayerIn_ = std::make_unique<eGLImage>(glFormat_, inputConfig_.stride / bytesPerPixel_, height_, inputConfig_.stride, GL_TEXTURE0, 0); /* Texture we will render to */ - eglImageBayerOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1); + eglImageRGBAOut_ = std::make_unique<eGLImage>(GL_RGBA, outputSize_.width, outputSize_.height, outputConfig_.stride, GL_TEXTURE1, 1); return 0; } void SoftwareIspPipelineGpu::stop() { - eglImageBayerOut_.reset(); + eglImageRGBAOut_.reset(); eglImageBayerIn_.reset(); if (programId_) diff --git a/src/libcamera/software_isp/software_isp_pipeline_gpu.h b/src/libcamera/software_isp/software_isp_pipeline_gpu.h index eb365d198..6f161e063 100644 --- a/src/libcamera/software_isp/software_isp_pipeline_gpu.h +++ b/src/libcamera/software_isp/software_isp_pipeline_gpu.h @@ -74,7 +74,7 @@ private: /* Pointer to object representing input texture */ std::unique_ptr<eGLImage> eglImageBayerIn_; - std::unique_ptr<eGLImage> eglImageBayerOut_; + std::unique_ptr<eGLImage> eglImageRGBAOut_; /* Shader parameters */ float firstRed_x_;
In our mutli-pass shader design we will use textures to pass the output of one compute or fragment shader to the next. Each phase will have its own distinct name and output. A better more accurate name for the thing which we render into is eglImageRGBAOut_ which has a very obvious meaning - its our RGBA output. We aren't outputting Bayer data here, we are outputting RGBA data so denote it as such by way of its name. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- src/libcamera/software_isp/software_isp_pipeline_gpu.cpp | 6 +++--- src/libcamera/software_isp/software_isp_pipeline_gpu.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)