[libcamera-devel] [PATCH v1.1 6/7] qcam: viewfinder_gl: Store textures in an array

[Niklas Söderlund]

Hi Laurent,

Thanks for your work.

On 2020-11-04 04:17:25 +0200, Laurent Pinchart wrote:
> In preparation for RGB formats support, store the three Y, U and V
> textures in an array. This makes the code more generic, and will avoid
> referring to an RGB texture as textureY_.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Andrey Konovalov <andrey.konovalov at linaro.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
> Changes since v1:
> 
> - Reorganize texture fields in the ViewFinderGL class
> ---
>  src/qcam/viewfinder_gl.cpp | 37 +++++++++++++++++--------------------
>  src/qcam/viewfinder_gl.h   |  9 +++++----
>  2 files changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> index e6625cac9795..cbc1365500f5 100644
> --- a/src/qcam/viewfinder_gl.cpp
> +++ b/src/qcam/viewfinder_gl.cpp
> @@ -33,10 +33,7 @@ static const QList<libcamera::PixelFormat> supportedFormats{
>  
>  ViewFinderGL::ViewFinderGL(QWidget *parent)
>  	: QOpenGLWidget(parent), buffer_(nullptr), data_(nullptr),
> -	  vertexBuffer_(QOpenGLBuffer::VertexBuffer),
> -	  textureU_(QOpenGLTexture::Target2D),
> -	  textureV_(QOpenGLTexture::Target2D),
> -	  textureY_(QOpenGLTexture::Target2D)
> +	  vertexBuffer_(QOpenGLBuffer::VertexBuffer)
>  {
>  }
>  
> @@ -263,14 +260,14 @@ bool ViewFinderGL::createFragmentShader()
>  	textureUniformV_ = shaderProgram_.uniformLocation("tex_v");
>  	textureUniformStepX_ = shaderProgram_.uniformLocation("tex_stepx");
>  
> -	if (!textureY_.isCreated())
> -		textureY_.create();
> +	/* Create the textures. */
> +	for (std::unique_ptr<QOpenGLTexture> &texture : textures_) {
> +		if (texture)
> +			continue;
>  
> -	if (!textureU_.isCreated())
> -		textureU_.create();
> -
> -	if (!textureV_.isCreated())
> -		textureV_.create();
> +		texture = std::make_unique<QOpenGLTexture>(QOpenGLTexture::Target2D);
> +		texture->create();
> +	}
>  
>  	return true;
>  }
> @@ -337,7 +334,7 @@ void ViewFinderGL::doRender()
>  	case libcamera::formats::NV42:
>  		/* Activate texture Y */
>  		glActiveTexture(GL_TEXTURE0);
> -		configureTexture(textureY_);
> +		configureTexture(*textures_[0]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RED,
> @@ -351,7 +348,7 @@ void ViewFinderGL::doRender()
>  
>  		/* Activate texture UV/VU */
>  		glActiveTexture(GL_TEXTURE1);
> -		configureTexture(textureU_);
> +		configureTexture(*textures_[1]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RG,
> @@ -367,7 +364,7 @@ void ViewFinderGL::doRender()
>  	case libcamera::formats::YUV420:
>  		/* Activate texture Y */
>  		glActiveTexture(GL_TEXTURE0);
> -		configureTexture(textureY_);
> +		configureTexture(*textures_[0]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RED,
> @@ -381,7 +378,7 @@ void ViewFinderGL::doRender()
>  
>  		/* Activate texture U */
>  		glActiveTexture(GL_TEXTURE1);
> -		configureTexture(textureU_);
> +		configureTexture(*textures_[1]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RED,
> @@ -395,7 +392,7 @@ void ViewFinderGL::doRender()
>  
>  		/* Activate texture V */
>  		glActiveTexture(GL_TEXTURE2);
> -		configureTexture(textureV_);
> +		configureTexture(*textures_[2]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RED,
> @@ -411,7 +408,7 @@ void ViewFinderGL::doRender()
>  	case libcamera::formats::YVU420:
>  		/* Activate texture Y */
>  		glActiveTexture(GL_TEXTURE0);
> -		configureTexture(textureY_);
> +		configureTexture(*textures_[0]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RED,
> @@ -425,7 +422,7 @@ void ViewFinderGL::doRender()
>  
>  		/* Activate texture V */
>  		glActiveTexture(GL_TEXTURE2);
> -		configureTexture(textureV_);
> +		configureTexture(*textures_[2]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RED,
> @@ -439,7 +436,7 @@ void ViewFinderGL::doRender()
>  
>  		/* Activate texture U */
>  		glActiveTexture(GL_TEXTURE1);
> -		configureTexture(textureU_);
> +		configureTexture(*textures_[1]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RED,
> @@ -462,7 +459,7 @@ void ViewFinderGL::doRender()
>  		 * The texture width is thus half of the image with.
>  		 */
>  		glActiveTexture(GL_TEXTURE0);
> -		configureTexture(textureY_);
> +		configureTexture(*textures_[0]);
>  		glTexImage2D(GL_TEXTURE_2D,
>  			     0,
>  			     GL_RGBA,
> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h
> index b3e36514d3d4..150fa4ae94da 100644
> --- a/src/qcam/viewfinder_gl.h
> +++ b/src/qcam/viewfinder_gl.h
> @@ -8,6 +8,7 @@
>  #ifndef __VIEWFINDER_GL_H__
>  #define __VIEWFINDER_GL_H__
>  
> +#include <array>
>  #include <memory>
>  
>  #include <QImage>
> @@ -77,14 +78,14 @@ private:
>  	/* Vertex buffer */
>  	QOpenGLBuffer vertexBuffer_;
>  
> -	/* YUV texture planars and parameters */
> +	/* Textures */
> +	std::array<std::unique_ptr<QOpenGLTexture>, 3> textures_;
> +
> +	/* YUV texture parameters */
>  	GLuint textureUniformU_;
>  	GLuint textureUniformV_;
>  	GLuint textureUniformY_;
>  	GLuint textureUniformStepX_;
> -	QOpenGLTexture textureU_;
> -	QOpenGLTexture textureV_;
> -	QOpenGLTexture textureY_;
>  	unsigned int horzSubSample_;
>  	unsigned int vertSubSample_;
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund