[libcamera-devel] [PATCH 2/7] qcam: viewfinder_gl: Keep fragment shader when format doesn't change

[Andrey Konovalov]

Hi Laurent,

Thanks for your patch!

On 03.11.2020 18:50, Laurent Pinchart wrote:
> When ViewFinderGL::setFormat() is called, the fragment shader is deleted
> and recreated for the new format. This results in unnecessary shader
> recompilation if only the size is changed and the pixel format remains
> the same. Keep the existing shader in that case.
> 
> The null test for fragmentShader_ can be removed, as if the shader
> program is linked, the fragment shader is guaranteed to exist.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Andrey Konovalov <andrey.konovalov at linaro.org>

Thanks,
Andrey

> ---
>   src/qcam/viewfinder_gl.cpp | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> index 03a576ba89d2..c07292523504 100644
> --- a/src/qcam/viewfinder_gl.cpp
> +++ b/src/qcam/viewfinder_gl.cpp
> @@ -53,19 +53,23 @@ const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const
>   int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,
>   			    const QSize &size)
>   {
> -	/* If the fragment is created remove it and create a new one. */
> -	if (fragmentShader_) {
> +	if (format != format_) {
> +		/*
> +		 * If the fragment already exists, remove it and create a new
> +		 * one for the new format.
> +		 */
>   		if (shaderProgram_.isLinked()) {
>   			shaderProgram_.release();
>   			shaderProgram_.removeShader(fragmentShader_.get());
>   			fragmentShader_.reset();
>   		}
> +
> +		if (!selectFormat(format))
> +			return -1;
> +
> +		format_ = format;
>   	}
>   
> -	if (!selectFormat(format))
> -		return -1;
> -
> -	format_ = format;
>   	size_ = size;
>   
>   	updateGeometry();
>