[libcamera-devel,7/7] qcam: viewfinder_gl: Add support for RGB formats
diff mbox series

Message ID 20201103155025.5948-8-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • qcam: Miscellaneous shaders cleanups and RGB support
Related show

Commit Message

Laurent Pinchart Nov. 3, 2020, 3:50 p.m. UTC 
Add support for 24-bit and 32-bit RGB formats. The fragment samples the
texture and reorders the components, using a pattern set through the
RGB_PATTERN macro.

An alternative to manual reordering in the shader would be to set the
texture swizzling mask. This is however not available in OpenGL ES
before version 3.0, which we don't mandate at the moment.

Only the BGR888 and RGB888 formats have been tested, with the vimc
pipeline handler.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/assets/shader/RGB.frag    | 22 +++++++++
 src/qcam/assets/shader/shaders.qrc |  1 +
 src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--
 3 files changed, 91 insertions(+), 3 deletions(-)
 create mode 100644 src/qcam/assets/shader/RGB.frag

Comments

Kieran Bingham Nov. 3, 2020, 11 p.m. UTC | #1 
Hi Laurent,

On 03/11/2020 15:50, Laurent Pinchart wrote:
> Add support for 24-bit and 32-bit RGB formats. The fragment samples the
> texture and reorders the components, using a pattern set through the
> RGB_PATTERN macro.
> 
> An alternative to manual reordering in the shader would be to set the
> texture swizzling mask. This is however not available in OpenGL ES
> before version 3.0, which we don't mandate at the moment.
> 
> Only the BGR888 and RGB888 formats have been tested, with the vimc
> pipeline handler.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/qcam/assets/shader/RGB.frag    | 22 +++++++++
>  src/qcam/assets/shader/shaders.qrc |  1 +
>  src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--
>  3 files changed, 91 insertions(+), 3 deletions(-)
>  create mode 100644 src/qcam/assets/shader/RGB.frag
> 
> diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag
> new file mode 100644
> index 000000000000..4c374ac98095
> --- /dev/null
> +++ b/src/qcam/assets/shader/RGB.frag
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Laurent Pinchart
> + *
> + * RGB.frag - Fragment shader code for RGB formats
> + */
> +
> +#ifdef GL_ES
> +precision mediump float;
> +#endif
> +
> +varying vec2 textureOut;
> +uniform sampler2D tex_y;
> +
> +void main(void)
> +{
> +	vec3 rgb;
> +
> +	rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
> +
> +	gl_FragColor = vec4(rgb, 1.0);
> +}
> diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc
> index 863109146281..8a8f9de1a5fa 100644
> --- a/src/qcam/assets/shader/shaders.qrc
> +++ b/src/qcam/assets/shader/shaders.qrc
> @@ -1,6 +1,7 @@
>  <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
>  <!DOCTYPE RCC><RCC version="1.0">
>  <qresource>
> +	<file>RGB.frag</file>
>  	<file>YUV_2_planes.frag</file>
>  	<file>YUV_3_planes.frag</file>
>  	<file>YUV_packed.frag</file>
> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> index cbc1365500f5..5d9b442e7985 100644
> --- a/src/qcam/viewfinder_gl.cpp
> +++ b/src/qcam/viewfinder_gl.cpp
> @@ -14,21 +14,28 @@
>  #include <libcamera/formats.h>
>  
>  static const QList<libcamera::PixelFormat> supportedFormats{
> -	/* Packed (single plane) */
> +	/* YUV - packed (single plane) */
>  	libcamera::formats::UYVY,
>  	libcamera::formats::VYUY,
>  	libcamera::formats::YUYV,
>  	libcamera::formats::YVYU,
> -	/* Semi planar (two planes) */
> +	/* YUV - semi planar (two planes) */
>  	libcamera::formats::NV12,
>  	libcamera::formats::NV21,
>  	libcamera::formats::NV16,
>  	libcamera::formats::NV61,
>  	libcamera::formats::NV24,
>  	libcamera::formats::NV42,
> -	/* Fully planar (three planes) */
> +	/* YUV - fully planar (three planes) */
>  	libcamera::formats::YUV420,
>  	libcamera::formats::YVU420,
> +	/* RGB */
> +	libcamera::formats::ABGR8888,
> +	libcamera::formats::ARGB8888,
> +	libcamera::formats::BGRA8888,
> +	libcamera::formats::RGBA8888,
> +	libcamera::formats::BGR888,
> +	libcamera::formats::RGB888,
>  };
>  
>  ViewFinderGL::ViewFinderGL(QWidget *parent)
> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)
>  		fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
>  		fragmentShaderFile_ = ":YUV_packed.frag";
>  		break;
> +	case libcamera::formats::ABGR8888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");

pattern rgb, the same as BGR888, so I presume the A is stripped by not
being used as a channel or some other magic elsewhere?

> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::ARGB8888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::BGRA8888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN gba");

Yet here it's specified.... at the end

> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::RGBA8888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN abg");

but here at the beginning?

What is the definition of the RGB_PATTERNs?
They don't seem to have a pattern that makes sense to me yet.

> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::BGR888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::RGB888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
>  	default:
>  		ret = false;
>  		qWarning() << "[ViewFinderGL]:"
> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()
>  					       1.0f / (size_.width() / 2 - 1));
>  		break;
>  
> +	case libcamera::formats::ABGR8888:
> +	case libcamera::formats::ARGB8888:
> +	case libcamera::formats::BGRA8888:
> +	case libcamera::formats::RGBA8888:
> +		glActiveTexture(GL_TEXTURE0);
> +		configureTexture(*textures_[0]);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RGBA,
> +			     size_.width(),
> +			     size_.height(),
> +			     0,
> +			     GL_RGBA,
> +			     GL_UNSIGNED_BYTE,
> +			     data_);
> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> +		break;
> +
> +	case libcamera::formats::BGR888:
> +	case libcamera::formats::RGB888:
> +		glActiveTexture(GL_TEXTURE0);
> +		configureTexture(*textures_[0]);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RGB,
> +			     size_.width(),
> +			     size_.height(),
> +			     0,
> +			     GL_RGB,
> +			     GL_UNSIGNED_BYTE,
> +			     data_);
> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> +		break;
> +
>  	default:
>  		break;
>  	};
>
Laurent Pinchart Nov. 4, 2020, 2:09 a.m. UTC | #2 
Hi Kieran,

On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:
> On 03/11/2020 15:50, Laurent Pinchart wrote:
> > Add support for 24-bit and 32-bit RGB formats. The fragment samples the
> > texture and reorders the components, using a pattern set through the
> > RGB_PATTERN macro.
> > 
> > An alternative to manual reordering in the shader would be to set the
> > texture swizzling mask. This is however not available in OpenGL ES
> > before version 3.0, which we don't mandate at the moment.
> > 
> > Only the BGR888 and RGB888 formats have been tested, with the vimc
> > pipeline handler.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/qcam/assets/shader/RGB.frag    | 22 +++++++++
> >  src/qcam/assets/shader/shaders.qrc |  1 +
> >  src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--
> >  3 files changed, 91 insertions(+), 3 deletions(-)
> >  create mode 100644 src/qcam/assets/shader/RGB.frag
> > 
> > diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag
> > new file mode 100644
> > index 000000000000..4c374ac98095
> > --- /dev/null
> > +++ b/src/qcam/assets/shader/RGB.frag
> > @@ -0,0 +1,22 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Laurent Pinchart
> > + *
> > + * RGB.frag - Fragment shader code for RGB formats
> > + */
> > +
> > +#ifdef GL_ES
> > +precision mediump float;
> > +#endif
> > +
> > +varying vec2 textureOut;
> > +uniform sampler2D tex_y;
> > +
> > +void main(void)
> > +{
> > +	vec3 rgb;
> > +
> > +	rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
> > +
> > +	gl_FragColor = vec4(rgb, 1.0);
> > +}
> > diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc
> > index 863109146281..8a8f9de1a5fa 100644
> > --- a/src/qcam/assets/shader/shaders.qrc
> > +++ b/src/qcam/assets/shader/shaders.qrc
> > @@ -1,6 +1,7 @@
> >  <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
> >  <!DOCTYPE RCC><RCC version="1.0">
> >  <qresource>
> > +	<file>RGB.frag</file>
> >  	<file>YUV_2_planes.frag</file>
> >  	<file>YUV_3_planes.frag</file>
> >  	<file>YUV_packed.frag</file>
> > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> > index cbc1365500f5..5d9b442e7985 100644
> > --- a/src/qcam/viewfinder_gl.cpp
> > +++ b/src/qcam/viewfinder_gl.cpp
> > @@ -14,21 +14,28 @@
> >  #include <libcamera/formats.h>
> >  
> >  static const QList<libcamera::PixelFormat> supportedFormats{
> > -	/* Packed (single plane) */
> > +	/* YUV - packed (single plane) */
> >  	libcamera::formats::UYVY,
> >  	libcamera::formats::VYUY,
> >  	libcamera::formats::YUYV,
> >  	libcamera::formats::YVYU,
> > -	/* Semi planar (two planes) */
> > +	/* YUV - semi planar (two planes) */
> >  	libcamera::formats::NV12,
> >  	libcamera::formats::NV21,
> >  	libcamera::formats::NV16,
> >  	libcamera::formats::NV61,
> >  	libcamera::formats::NV24,
> >  	libcamera::formats::NV42,
> > -	/* Fully planar (three planes) */
> > +	/* YUV - fully planar (three planes) */
> >  	libcamera::formats::YUV420,
> >  	libcamera::formats::YVU420,
> > +	/* RGB */
> > +	libcamera::formats::ABGR8888,
> > +	libcamera::formats::ARGB8888,
> > +	libcamera::formats::BGRA8888,
> > +	libcamera::formats::RGBA8888,
> > +	libcamera::formats::BGR888,
> > +	libcamera::formats::RGB888,
> >  };
> >  
> >  ViewFinderGL::ViewFinderGL(QWidget *parent)
> > @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)
> >  		fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
> >  		fragmentShaderFile_ = ":YUV_packed.frag";
> >  		break;
> > +	case libcamera::formats::ABGR8888:
> > +		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
> 
> pattern rgb, the same as BGR888, so I presume the A is stripped by not
> being used as a channel or some other magic elsewhere?
> 
> > +		fragmentShaderFile_ = ":RGB.frag";
> > +		break;
> > +	case libcamera::formats::ARGB8888:
> > +		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
> > +		fragmentShaderFile_ = ":RGB.frag";
> > +		break;
> > +	case libcamera::formats::BGRA8888:
> > +		fragmentShaderDefines_.append("#define RGB_PATTERN gba");
> 
> Yet here it's specified.... at the end
> 
> > +		fragmentShaderFile_ = ":RGB.frag";
> > +		break;
> > +	case libcamera::formats::RGBA8888:
> > +		fragmentShaderDefines_.append("#define RGB_PATTERN abg");
> 
> but here at the beginning?
> 
> What is the definition of the RGB_PATTERNs?
> They don't seem to have a pattern that makes sense to me yet.

The shader language accesses vector components by name. The texture2D()
function, which samples a texture, returns a vec4, a 4-components
floating point vector. Multiple component names are supported:

- {x, y, z, w}
- {r, g, b, a} 
- {s, t, p, q}

.x, .r and .s are all equivalent to [0] in C syntax, and similarly for
other names. The reason different names are defined and alias each other
is that vectors can store different types of data, and standardizing on
{r, g, b, a} would lead to weird-looking code when the vector stores a
3D coordinnate for instance ({s, t, p, q} are used as a convention for
texture coordinates).

The indexing syntax also supports multiple components to be selected:

	vec4 colour(1.0, 2.0, 3.0, 4.0);
	vec2 value;

	value = colour.rb;

	// value now contains (1.0, 3.0)

The RGB fragment shader code simply contains

	vec3 rgb;

	rgb = texture2D(tex_y, textureOut).RGB_PATTERN;

The pattern thus defines the components mapping.

One additional piece of information that is required is how OpenGL
expects texture data to be layed out in memory. The texture is created
with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A
components are layed down in that order, in one byte each. The RGBA8888
format, has the opposite convention, it stores RGBA in that order in a
32-bit word that is then stored in memory in little endian format, so
bytes contains A, B, G and R in that order.

So, taking the RGBA8888 example, we have ABGR in memory, which is read
in a vec4. We want to extract RGB (in that order) in the shader, so we
need components [3], [2] and [1] of the vector, which are expressed as
.abg.

I hope this makes it clearer.

> > +		fragmentShaderFile_ = ":RGB.frag";
> > +		break;
> > +	case libcamera::formats::BGR888:
> > +		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
> > +		fragmentShaderFile_ = ":RGB.frag";
> > +		break;
> > +	case libcamera::formats::RGB888:
> > +		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
> > +		fragmentShaderFile_ = ":RGB.frag";
> > +		break;
> >  	default:
> >  		ret = false;
> >  		qWarning() << "[ViewFinderGL]:"
> > @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()
> >  					       1.0f / (size_.width() / 2 - 1));
> >  		break;
> >  
> > +	case libcamera::formats::ABGR8888:
> > +	case libcamera::formats::ARGB8888:
> > +	case libcamera::formats::BGRA8888:
> > +	case libcamera::formats::RGBA8888:
> > +		glActiveTexture(GL_TEXTURE0);
> > +		configureTexture(*textures_[0]);
> > +		glTexImage2D(GL_TEXTURE_2D,
> > +			     0,
> > +			     GL_RGBA,
> > +			     size_.width(),
> > +			     size_.height(),
> > +			     0,
> > +			     GL_RGBA,
> > +			     GL_UNSIGNED_BYTE,
> > +			     data_);
> > +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> > +		break;
> > +
> > +	case libcamera::formats::BGR888:
> > +	case libcamera::formats::RGB888:
> > +		glActiveTexture(GL_TEXTURE0);
> > +		configureTexture(*textures_[0]);
> > +		glTexImage2D(GL_TEXTURE_2D,
> > +			     0,
> > +			     GL_RGB,
> > +			     size_.width(),
> > +			     size_.height(),
> > +			     0,
> > +			     GL_RGB,
> > +			     GL_UNSIGNED_BYTE,
> > +			     data_);
> > +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> > +		break;
> > +
> >  	default:
> >  		break;
> >  	};
Andrey Konovalov Nov. 4, 2020, 9:32 a.m. UTC | #3 
Hi Laurent,

On 04.11.2020 05:09, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:
>> On 03/11/2020 15:50, Laurent Pinchart wrote:
>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the
>>> texture and reorders the components, using a pattern set through the
>>> RGB_PATTERN macro.
>>>
>>> An alternative to manual reordering in the shader would be to set the
>>> texture swizzling mask. This is however not available in OpenGL ES
>>> before version 3.0, which we don't mandate at the moment.
>>>
>>> Only the BGR888 and RGB888 formats have been tested, with the vimc
>>> pipeline handler.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>> ---
>>>   src/qcam/assets/shader/RGB.frag    | 22 +++++++++
>>>   src/qcam/assets/shader/shaders.qrc |  1 +
>>>   src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--
>>>   3 files changed, 91 insertions(+), 3 deletions(-)
>>>   create mode 100644 src/qcam/assets/shader/RGB.frag
>>>
>>> diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag
>>> new file mode 100644
>>> index 000000000000..4c374ac98095
>>> --- /dev/null
>>> +++ b/src/qcam/assets/shader/RGB.frag
>>> @@ -0,0 +1,22 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2020, Laurent Pinchart
>>> + *
>>> + * RGB.frag - Fragment shader code for RGB formats
>>> + */
>>> +
>>> +#ifdef GL_ES
>>> +precision mediump float;
>>> +#endif
>>> +
>>> +varying vec2 textureOut;
>>> +uniform sampler2D tex_y;
>>> +
>>> +void main(void)
>>> +{
>>> +	vec3 rgb;
>>> +
>>> +	rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
>>> +
>>> +	gl_FragColor = vec4(rgb, 1.0);
>>> +}
>>> diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc
>>> index 863109146281..8a8f9de1a5fa 100644
>>> --- a/src/qcam/assets/shader/shaders.qrc
>>> +++ b/src/qcam/assets/shader/shaders.qrc
>>> @@ -1,6 +1,7 @@
>>>   <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
>>>   <!DOCTYPE RCC><RCC version="1.0">
>>>   <qresource>
>>> +	<file>RGB.frag</file>
>>>   	<file>YUV_2_planes.frag</file>
>>>   	<file>YUV_3_planes.frag</file>
>>>   	<file>YUV_packed.frag</file>
>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
>>> index cbc1365500f5..5d9b442e7985 100644
>>> --- a/src/qcam/viewfinder_gl.cpp
>>> +++ b/src/qcam/viewfinder_gl.cpp
>>> @@ -14,21 +14,28 @@
>>>   #include <libcamera/formats.h>
>>>   
>>>   static const QList<libcamera::PixelFormat> supportedFormats{
>>> -	/* Packed (single plane) */
>>> +	/* YUV - packed (single plane) */
>>>   	libcamera::formats::UYVY,
>>>   	libcamera::formats::VYUY,
>>>   	libcamera::formats::YUYV,
>>>   	libcamera::formats::YVYU,
>>> -	/* Semi planar (two planes) */
>>> +	/* YUV - semi planar (two planes) */
>>>   	libcamera::formats::NV12,
>>>   	libcamera::formats::NV21,
>>>   	libcamera::formats::NV16,
>>>   	libcamera::formats::NV61,
>>>   	libcamera::formats::NV24,
>>>   	libcamera::formats::NV42,
>>> -	/* Fully planar (three planes) */
>>> +	/* YUV - fully planar (three planes) */
>>>   	libcamera::formats::YUV420,
>>>   	libcamera::formats::YVU420,
>>> +	/* RGB */
>>> +	libcamera::formats::ABGR8888,
>>> +	libcamera::formats::ARGB8888,
>>> +	libcamera::formats::BGRA8888,
>>> +	libcamera::formats::RGBA8888,
>>> +	libcamera::formats::BGR888,
>>> +	libcamera::formats::RGB888,
>>>   };
>>>   
>>>   ViewFinderGL::ViewFinderGL(QWidget *parent)
>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)
>>>   		fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
>>>   		fragmentShaderFile_ = ":YUV_packed.frag";
>>>   		break;
>>> +	case libcamera::formats::ABGR8888:
>>> +		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
>>
>> pattern rgb, the same as BGR888, so I presume the A is stripped by not
>> being used as a channel or some other magic elsewhere?
>>
>>> +		fragmentShaderFile_ = ":RGB.frag";
>>> +		break;
>>> +	case libcamera::formats::ARGB8888:
>>> +		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
>>> +		fragmentShaderFile_ = ":RGB.frag";
>>> +		break;
>>> +	case libcamera::formats::BGRA8888:
>>> +		fragmentShaderDefines_.append("#define RGB_PATTERN gba");
>>
>> Yet here it's specified.... at the end
>>
>>> +		fragmentShaderFile_ = ":RGB.frag";
>>> +		break;
>>> +	case libcamera::formats::RGBA8888:
>>> +		fragmentShaderDefines_.append("#define RGB_PATTERN abg");
>>
>> but here at the beginning?
>>
>> What is the definition of the RGB_PATTERNs?
>> They don't seem to have a pattern that makes sense to me yet.
> 
> The shader language accesses vector components by name. The texture2D()
> function, which samples a texture, returns a vec4, a 4-components
> floating point vector. Multiple component names are supported:
> 
> - {x, y, z, w}
> - {r, g, b, a}
> - {s, t, p, q}
> 
> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for
> other names. The reason different names are defined and alias each other
> is that vectors can store different types of data, and standardizing on
> {r, g, b, a} would lead to weird-looking code when the vector stores a
> 3D coordinnate for instance ({s, t, p, q} are used as a convention for
> texture coordinates).
> 
> The indexing syntax also supports multiple components to be selected:
> 
> 	vec4 colour(1.0, 2.0, 3.0, 4.0);
> 	vec2 value;
> 
> 	value = colour.rb;
> 
> 	// value now contains (1.0, 3.0)
> 
> The RGB fragment shader code simply contains
> 
> 	vec3 rgb;
> 
> 	rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
> 
> The pattern thus defines the components mapping.
> 
> One additional piece of information that is required is how OpenGL
> expects texture data to be layed out in memory. The texture is created
> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A
> components are layed down in that order, in one byte each. The RGBA8888
> format, has the opposite convention, it stores RGBA in that order in a
> 32-bit word that is then stored in memory in little endian format

- Ah... I've missed that little endian part of the puzzle, and have got confused too.

Thanks a lot for the detailed explanation!

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

>, so
> bytes contains A, B, G and R in that order.
> 
> So, taking the RGBA8888 example, we have ABGR in memory, which is read
> in a vec4. We want to extract RGB (in that order) in the shader, so we
> need components [3], [2] and [1] of the vector, which are expressed as
> .abg.
> 
> I hope this makes it clearer.
> 
>>> +		fragmentShaderFile_ = ":RGB.frag";
>>> +		break;
>>> +	case libcamera::formats::BGR888:
>>> +		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
>>> +		fragmentShaderFile_ = ":RGB.frag";
>>> +		break;
>>> +	case libcamera::formats::RGB888:
>>> +		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
>>> +		fragmentShaderFile_ = ":RGB.frag";
>>> +		break;
>>>   	default:
>>>   		ret = false;
>>>   		qWarning() << "[ViewFinderGL]:"
>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()
>>>   					       1.0f / (size_.width() / 2 - 1));
>>>   		break;
>>>   
>>> +	case libcamera::formats::ABGR8888:
>>> +	case libcamera::formats::ARGB8888:
>>> +	case libcamera::formats::BGRA8888:
>>> +	case libcamera::formats::RGBA8888:
>>> +		glActiveTexture(GL_TEXTURE0);
>>> +		configureTexture(*textures_[0]);
>>> +		glTexImage2D(GL_TEXTURE_2D,
>>> +			     0,
>>> +			     GL_RGBA,
>>> +			     size_.width(),
>>> +			     size_.height(),
>>> +			     0,
>>> +			     GL_RGBA,
>>> +			     GL_UNSIGNED_BYTE,
>>> +			     data_);
>>> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
>>> +		break;
>>> +
>>> +	case libcamera::formats::BGR888:
>>> +	case libcamera::formats::RGB888:
>>> +		glActiveTexture(GL_TEXTURE0);
>>> +		configureTexture(*textures_[0]);
>>> +		glTexImage2D(GL_TEXTURE_2D,
>>> +			     0,
>>> +			     GL_RGB,
>>> +			     size_.width(),
>>> +			     size_.height(),
>>> +			     0,
>>> +			     GL_RGB,
>>> +			     GL_UNSIGNED_BYTE,
>>> +			     data_);
>>> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
>>> +		break;
>>> +
>>>   	default:
>>>   		break;
>>>   	};
>
Kieran Bingham Nov. 4, 2020, 9:49 a.m. UTC | #4 
Hi Laurent,

On 04/11/2020 09:32, Andrey Konovalov wrote:
> Hi Laurent,
> 
> On 04.11.2020 05:09, Laurent Pinchart wrote:
>> Hi Kieran,
>>
>> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:
>>> On 03/11/2020 15:50, Laurent Pinchart wrote:
>>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the
>>>> texture and reorders the components, using a pattern set through the
>>>> RGB_PATTERN macro.
>>>>
>>>> An alternative to manual reordering in the shader would be to set the
>>>> texture swizzling mask. This is however not available in OpenGL ES
>>>> before version 3.0, which we don't mandate at the moment.
>>>>
>>>> Only the BGR888 and RGB888 formats have been tested, with the vimc
>>>> pipeline handler.
>>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> ---
>>>>   src/qcam/assets/shader/RGB.frag    | 22 +++++++++
>>>>   src/qcam/assets/shader/shaders.qrc |  1 +
>>>>   src/qcam/viewfinder_gl.cpp         | 71
>>>> ++++++++++++++++++++++++++++--
>>>>   3 files changed, 91 insertions(+), 3 deletions(-)
>>>>   create mode 100644 src/qcam/assets/shader/RGB.frag
>>>>
>>>> diff --git a/src/qcam/assets/shader/RGB.frag
>>>> b/src/qcam/assets/shader/RGB.frag
>>>> new file mode 100644
>>>> index 000000000000..4c374ac98095
>>>> --- /dev/null
>>>> +++ b/src/qcam/assets/shader/RGB.frag
>>>> @@ -0,0 +1,22 @@
>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>>> +/*
>>>> + * Copyright (C) 2020, Laurent Pinchart
>>>> + *
>>>> + * RGB.frag - Fragment shader code for RGB formats
>>>> + */
>>>> +
>>>> +#ifdef GL_ES
>>>> +precision mediump float;
>>>> +#endif
>>>> +
>>>> +varying vec2 textureOut;
>>>> +uniform sampler2D tex_y;
>>>> +
>>>> +void main(void)
>>>> +{
>>>> +    vec3 rgb;
>>>> +
>>>> +    rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
>>>> +
>>>> +    gl_FragColor = vec4(rgb, 1.0);
>>>> +}
>>>> diff --git a/src/qcam/assets/shader/shaders.qrc
>>>> b/src/qcam/assets/shader/shaders.qrc
>>>> index 863109146281..8a8f9de1a5fa 100644
>>>> --- a/src/qcam/assets/shader/shaders.qrc
>>>> +++ b/src/qcam/assets/shader/shaders.qrc
>>>> @@ -1,6 +1,7 @@
>>>>   <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
>>>>   <!DOCTYPE RCC><RCC version="1.0">
>>>>   <qresource>
>>>> +    <file>RGB.frag</file>
>>>>       <file>YUV_2_planes.frag</file>
>>>>       <file>YUV_3_planes.frag</file>
>>>>       <file>YUV_packed.frag</file>
>>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
>>>> index cbc1365500f5..5d9b442e7985 100644
>>>> --- a/src/qcam/viewfinder_gl.cpp
>>>> +++ b/src/qcam/viewfinder_gl.cpp
>>>> @@ -14,21 +14,28 @@
>>>>   #include <libcamera/formats.h>
>>>>     static const QList<libcamera::PixelFormat> supportedFormats{
>>>> -    /* Packed (single plane) */
>>>> +    /* YUV - packed (single plane) */
>>>>       libcamera::formats::UYVY,
>>>>       libcamera::formats::VYUY,
>>>>       libcamera::formats::YUYV,
>>>>       libcamera::formats::YVYU,
>>>> -    /* Semi planar (two planes) */
>>>> +    /* YUV - semi planar (two planes) */
>>>>       libcamera::formats::NV12,
>>>>       libcamera::formats::NV21,
>>>>       libcamera::formats::NV16,
>>>>       libcamera::formats::NV61,
>>>>       libcamera::formats::NV24,
>>>>       libcamera::formats::NV42,
>>>> -    /* Fully planar (three planes) */
>>>> +    /* YUV - fully planar (three planes) */
>>>>       libcamera::formats::YUV420,
>>>>       libcamera::formats::YVU420,
>>>> +    /* RGB */
>>>> +    libcamera::formats::ABGR8888,
>>>> +    libcamera::formats::ARGB8888,
>>>> +    libcamera::formats::BGRA8888,
>>>> +    libcamera::formats::RGBA8888,
>>>> +    libcamera::formats::BGR888,
>>>> +    libcamera::formats::RGB888,
>>>>   };
>>>>     ViewFinderGL::ViewFinderGL(QWidget *parent)
>>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const
>>>> libcamera::PixelFormat &format)
>>>>           fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
>>>>           fragmentShaderFile_ = ":YUV_packed.frag";
>>>>           break;
>>>> +    case libcamera::formats::ABGR8888:
>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
>>>
>>> pattern rgb, the same as BGR888, so I presume the A is stripped by not
>>> being used as a channel or some other magic elsewhere?
>>>
>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>> +        break;
>>>> +    case libcamera::formats::ARGB8888:
>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>> +        break;
>>>> +    case libcamera::formats::BGRA8888:
>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN gba");
>>>
>>> Yet here it's specified.... at the end
>>>
>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>> +        break;
>>>> +    case libcamera::formats::RGBA8888:
>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN abg");
>>>
>>> but here at the beginning?
>>>
>>> What is the definition of the RGB_PATTERNs?
>>> They don't seem to have a pattern that makes sense to me yet.
>>
>> The shader language accesses vector components by name. The texture2D()
>> function, which samples a texture, returns a vec4, a 4-components
>> floating point vector. Multiple component names are supported:
>>
>> - {x, y, z, w}
>> - {r, g, b, a}
>> - {s, t, p, q}
>>

I wish they also had a set of component names which were simply 'linear'
(e, f, g, h} for instance

(also because those are unused above and 'almost' linear in a qwerty
keyboard ;D)

Or using an index value 1,2,3,4 but I can see how using numeric indexes
here could cause difficulty

>> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for
>> other names. The reason different names are defined and alias each other
>> is that vectors can store different types of data, and standardizing on
>> {r, g, b, a} would lead to weird-looking code when the vector stores a
>> 3D coordinnate for instance ({s, t, p, q} are used as a convention for
>> texture coordinates).
>>
>> The indexing syntax also supports multiple components to be selected:
>>
>>     vec4 colour(1.0, 2.0, 3.0, 4.0);
>>     vec2 value;
>>
>>     value = colour.rb;
>>
>>     // value now contains (1.0, 3.0)
>>
>> The RGB fragment shader code simply contains
>>
>>     vec3 rgb;
>>
>>     rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
>>
>> The pattern thus defines the components mapping.
>>
>> One additional piece of information that is required is how OpenGL
>> expects texture data to be layed out in memory. The texture is created
>> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A
>> components are layed down in that order, in one byte each. The RGBA8888
>> format, has the opposite convention, it stores RGBA in that order in a
>> 32-bit word that is then stored in memory in little endian format
> 
> - Ah... I've missed that little endian part of the puzzle, and have got
> confused too.
> 
> Thanks a lot for the detailed explanation!

Agreed. Should we add all that to the commit message?
It's a really useful explanation - it seems valuable to keep it.


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

And this too now ;-)

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
>> , so
>> bytes contains A, B, G and R in that order.
>>
>> So, taking the RGBA8888 example, we have ABGR in memory, which is read
>> in a vec4. We want to extract RGB (in that order) in the shader, so we
>> need components [3], [2] and [1] of the vector, which are expressed as
>> .abg.
>>
>> I hope this makes it clearer.
>>
>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>> +        break;
>>>> +    case libcamera::formats::BGR888:
>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>> +        break;
>>>> +    case libcamera::formats::RGB888:
>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>> +        break;
>>>>       default:
>>>>           ret = false;
>>>>           qWarning() << "[ViewFinderGL]:"
>>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()
>>>>                              1.0f / (size_.width() / 2 - 1));
>>>>           break;
>>>>   +    case libcamera::formats::ABGR8888:
>>>> +    case libcamera::formats::ARGB8888:
>>>> +    case libcamera::formats::BGRA8888:
>>>> +    case libcamera::formats::RGBA8888:
>>>> +        glActiveTexture(GL_TEXTURE0);
>>>> +        configureTexture(*textures_[0]);
>>>> +        glTexImage2D(GL_TEXTURE_2D,
>>>> +                 0,
>>>> +                 GL_RGBA,
>>>> +                 size_.width(),
>>>> +                 size_.height(),
>>>> +                 0,
>>>> +                 GL_RGBA,
>>>> +                 GL_UNSIGNED_BYTE,
>>>> +                 data_);
>>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);
>>>> +        break;
>>>> +
>>>> +    case libcamera::formats::BGR888:
>>>> +    case libcamera::formats::RGB888:
>>>> +        glActiveTexture(GL_TEXTURE0);
>>>> +        configureTexture(*textures_[0]);
>>>> +        glTexImage2D(GL_TEXTURE_2D,
>>>> +                 0,
>>>> +                 GL_RGB,
>>>> +                 size_.width(),
>>>> +                 size_.height(),
>>>> +                 0,
>>>> +                 GL_RGB,
>>>> +                 GL_UNSIGNED_BYTE,
>>>> +                 data_);
>>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);
>>>> +        break;
>>>> +
>>>>       default:
>>>>           break;
>>>>       };
>>
Laurent Pinchart Nov. 4, 2020, 10:19 a.m. UTC | #5 
Hi Kieran,

On Wed, Nov 04, 2020 at 09:49:43AM +0000, Kieran Bingham wrote:
> On 04/11/2020 09:32, Andrey Konovalov wrote:
> > On 04.11.2020 05:09, Laurent Pinchart wrote:
> >> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:
> >>> On 03/11/2020 15:50, Laurent Pinchart wrote:
> >>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the
> >>>> texture and reorders the components, using a pattern set through the
> >>>> RGB_PATTERN macro.
> >>>>
> >>>> An alternative to manual reordering in the shader would be to set the
> >>>> texture swizzling mask. This is however not available in OpenGL ES
> >>>> before version 3.0, which we don't mandate at the moment.
> >>>>
> >>>> Only the BGR888 and RGB888 formats have been tested, with the vimc
> >>>> pipeline handler.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> ---
> >>>>   src/qcam/assets/shader/RGB.frag    | 22 +++++++++
> >>>>   src/qcam/assets/shader/shaders.qrc |  1 +
> >>>>   src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--
> >>>>   3 files changed, 91 insertions(+), 3 deletions(-)
> >>>>   create mode 100644 src/qcam/assets/shader/RGB.frag
> >>>>
> >>>> diff --git a/src/qcam/assets/shader/RGB.frag
> >>>> b/src/qcam/assets/shader/RGB.frag
> >>>> new file mode 100644
> >>>> index 000000000000..4c374ac98095
> >>>> --- /dev/null
> >>>> +++ b/src/qcam/assets/shader/RGB.frag
> >>>> @@ -0,0 +1,22 @@
> >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >>>> +/*
> >>>> + * Copyright (C) 2020, Laurent Pinchart
> >>>> + *
> >>>> + * RGB.frag - Fragment shader code for RGB formats
> >>>> + */
> >>>> +
> >>>> +#ifdef GL_ES
> >>>> +precision mediump float;
> >>>> +#endif
> >>>> +
> >>>> +varying vec2 textureOut;
> >>>> +uniform sampler2D tex_y;
> >>>> +
> >>>> +void main(void)
> >>>> +{
> >>>> +    vec3 rgb;
> >>>> +
> >>>> +    rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
> >>>> +
> >>>> +    gl_FragColor = vec4(rgb, 1.0);
> >>>> +}
> >>>> diff --git a/src/qcam/assets/shader/shaders.qrc
> >>>> b/src/qcam/assets/shader/shaders.qrc
> >>>> index 863109146281..8a8f9de1a5fa 100644
> >>>> --- a/src/qcam/assets/shader/shaders.qrc
> >>>> +++ b/src/qcam/assets/shader/shaders.qrc
> >>>> @@ -1,6 +1,7 @@
> >>>>   <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
> >>>>   <!DOCTYPE RCC><RCC version="1.0">
> >>>>   <qresource>
> >>>> +    <file>RGB.frag</file>
> >>>>       <file>YUV_2_planes.frag</file>
> >>>>       <file>YUV_3_planes.frag</file>
> >>>>       <file>YUV_packed.frag</file>
> >>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> >>>> index cbc1365500f5..5d9b442e7985 100644
> >>>> --- a/src/qcam/viewfinder_gl.cpp
> >>>> +++ b/src/qcam/viewfinder_gl.cpp
> >>>> @@ -14,21 +14,28 @@
> >>>>   #include <libcamera/formats.h>
> >>>>     static const QList<libcamera::PixelFormat> supportedFormats{
> >>>> -    /* Packed (single plane) */
> >>>> +    /* YUV - packed (single plane) */
> >>>>       libcamera::formats::UYVY,
> >>>>       libcamera::formats::VYUY,
> >>>>       libcamera::formats::YUYV,
> >>>>       libcamera::formats::YVYU,
> >>>> -    /* Semi planar (two planes) */
> >>>> +    /* YUV - semi planar (two planes) */
> >>>>       libcamera::formats::NV12,
> >>>>       libcamera::formats::NV21,
> >>>>       libcamera::formats::NV16,
> >>>>       libcamera::formats::NV61,
> >>>>       libcamera::formats::NV24,
> >>>>       libcamera::formats::NV42,
> >>>> -    /* Fully planar (three planes) */
> >>>> +    /* YUV - fully planar (three planes) */
> >>>>       libcamera::formats::YUV420,
> >>>>       libcamera::formats::YVU420,
> >>>> +    /* RGB */
> >>>> +    libcamera::formats::ABGR8888,
> >>>> +    libcamera::formats::ARGB8888,
> >>>> +    libcamera::formats::BGRA8888,
> >>>> +    libcamera::formats::RGBA8888,
> >>>> +    libcamera::formats::BGR888,
> >>>> +    libcamera::formats::RGB888,
> >>>>   };
> >>>>     ViewFinderGL::ViewFinderGL(QWidget *parent)
> >>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const
> >>>> libcamera::PixelFormat &format)
> >>>>           fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
> >>>>           fragmentShaderFile_ = ":YUV_packed.frag";
> >>>>           break;
> >>>> +    case libcamera::formats::ABGR8888:
> >>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
> >>>
> >>> pattern rgb, the same as BGR888, so I presume the A is stripped by not
> >>> being used as a channel or some other magic elsewhere?
> >>>
> >>>> +        fragmentShaderFile_ = ":RGB.frag";
> >>>> +        break;
> >>>> +    case libcamera::formats::ARGB8888:
> >>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
> >>>> +        fragmentShaderFile_ = ":RGB.frag";
> >>>> +        break;
> >>>> +    case libcamera::formats::BGRA8888:
> >>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN gba");
> >>>
> >>> Yet here it's specified.... at the end
> >>>
> >>>> +        fragmentShaderFile_ = ":RGB.frag";
> >>>> +        break;
> >>>> +    case libcamera::formats::RGBA8888:
> >>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN abg");
> >>>
> >>> but here at the beginning?
> >>>
> >>> What is the definition of the RGB_PATTERNs?
> >>> They don't seem to have a pattern that makes sense to me yet.
> >>
> >> The shader language accesses vector components by name. The texture2D()
> >> function, which samples a texture, returns a vec4, a 4-components
> >> floating point vector. Multiple component names are supported:
> >>
> >> - {x, y, z, w}
> >> - {r, g, b, a}
> >> - {s, t, p, q}
> >>
> 
> I wish they also had a set of component names which were simply 'linear'
> (e, f, g, h} for instance
> 
> (also because those are unused above and 'almost' linear in a qwerty
> keyboard ;D)
> 
> Or using an index value 1,2,3,4 but I can see how using numeric indexes
> here could cause difficulty
> 
> >> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for
> >> other names. The reason different names are defined and alias each other
> >> is that vectors can store different types of data, and standardizing on
> >> {r, g, b, a} would lead to weird-looking code when the vector stores a
> >> 3D coordinnate for instance ({s, t, p, q} are used as a convention for
> >> texture coordinates).
> >>
> >> The indexing syntax also supports multiple components to be selected:
> >>
> >>     vec4 colour(1.0, 2.0, 3.0, 4.0);
> >>     vec2 value;
> >>
> >>     value = colour.rb;
> >>
> >>     // value now contains (1.0, 3.0)
> >>
> >> The RGB fragment shader code simply contains
> >>
> >>     vec3 rgb;
> >>
> >>     rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
> >>
> >> The pattern thus defines the components mapping.
> >>
> >> One additional piece of information that is required is how OpenGL
> >> expects texture data to be layed out in memory. The texture is created
> >> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A
> >> components are layed down in that order, in one byte each. The RGBA8888
> >> format, has the opposite convention, it stores RGBA in that order in a
> >> 32-bit word that is then stored in memory in little endian format
> > 
> > - Ah... I've missed that little endian part of the puzzle, and have got
> > confused too.
> > 
> > Thanks a lot for the detailed explanation!
> 
> Agreed. Should we add all that to the commit message?
> It's a really useful explanation - it seems valuable to keep it.

I knew you would say that :-)

I'm not sure we really need to document the OpenGL API and the shader
language in the commit message though. I've expanded the first paragraph
to the following:

"Add support for 24-bit and 32-bit RGB formats. The fragment samples the
texture and reorders the components, using a pattern set through the
RGB_PATTERN macro. The pattern stores the shader vec4 element indices 
(named {r, g, b, a} by convention, for elements 0 to 3) to be extracted 
from the texture samples, when interpreted by OpenGL as RGBA.

Note that, as textures are created with GL_UNSIGNED_BYTE, the RGBA
order corresponds to bytes in memory, while the libcamera formats are
named based on the components order in a 32-bit word stored in memory in
little endian format."

I think this provides enough context, please please let me know if you
disagree.

> > Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> 
> And this too now ;-)
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> >> , so
> >> bytes contains A, B, G and R in that order.
> >>
> >> So, taking the RGBA8888 example, we have ABGR in memory, which is read
> >> in a vec4. We want to extract RGB (in that order) in the shader, so we
> >> need components [3], [2] and [1] of the vector, which are expressed as
> >> .abg.
> >>
> >> I hope this makes it clearer.
> >>
> >>>> +        fragmentShaderFile_ = ":RGB.frag";
> >>>> +        break;
> >>>> +    case libcamera::formats::BGR888:
> >>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
> >>>> +        fragmentShaderFile_ = ":RGB.frag";
> >>>> +        break;
> >>>> +    case libcamera::formats::RGB888:
> >>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
> >>>> +        fragmentShaderFile_ = ":RGB.frag";
> >>>> +        break;
> >>>>       default:
> >>>>           ret = false;
> >>>>           qWarning() << "[ViewFinderGL]:"
> >>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()
> >>>>                              1.0f / (size_.width() / 2 - 1));
> >>>>           break;
> >>>> +    case libcamera::formats::ABGR8888:
> >>>> +    case libcamera::formats::ARGB8888:
> >>>> +    case libcamera::formats::BGRA8888:
> >>>> +    case libcamera::formats::RGBA8888:
> >>>> +        glActiveTexture(GL_TEXTURE0);
> >>>> +        configureTexture(*textures_[0]);
> >>>> +        glTexImage2D(GL_TEXTURE_2D,
> >>>> +                 0,
> >>>> +                 GL_RGBA,
> >>>> +                 size_.width(),
> >>>> +                 size_.height(),
> >>>> +                 0,
> >>>> +                 GL_RGBA,
> >>>> +                 GL_UNSIGNED_BYTE,
> >>>> +                 data_);
> >>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);
> >>>> +        break;
> >>>> +
> >>>> +    case libcamera::formats::BGR888:
> >>>> +    case libcamera::formats::RGB888:
> >>>> +        glActiveTexture(GL_TEXTURE0);
> >>>> +        configureTexture(*textures_[0]);
> >>>> +        glTexImage2D(GL_TEXTURE_2D,
> >>>> +                 0,
> >>>> +                 GL_RGB,
> >>>> +                 size_.width(),
> >>>> +                 size_.height(),
> >>>> +                 0,
> >>>> +                 GL_RGB,
> >>>> +                 GL_UNSIGNED_BYTE,
> >>>> +                 data_);
> >>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);
> >>>> +        break;
> >>>> +
> >>>>       default:
> >>>>           break;
> >>>>       };
Kieran Bingham Nov. 4, 2020, 10:23 a.m. UTC | #6 
Hi Laurent,

On 04/11/2020 10:19, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Wed, Nov 04, 2020 at 09:49:43AM +0000, Kieran Bingham wrote:
>> On 04/11/2020 09:32, Andrey Konovalov wrote:
>>> On 04.11.2020 05:09, Laurent Pinchart wrote:
>>>> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:
>>>>> On 03/11/2020 15:50, Laurent Pinchart wrote:
>>>>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the
>>>>>> texture and reorders the components, using a pattern set through the
>>>>>> RGB_PATTERN macro.
>>>>>>
>>>>>> An alternative to manual reordering in the shader would be to set the
>>>>>> texture swizzling mask. This is however not available in OpenGL ES
>>>>>> before version 3.0, which we don't mandate at the moment.
>>>>>>
>>>>>> Only the BGR888 and RGB888 formats have been tested, with the vimc
>>>>>> pipeline handler.
>>>>>>
>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>>> ---
>>>>>>   src/qcam/assets/shader/RGB.frag    | 22 +++++++++
>>>>>>   src/qcam/assets/shader/shaders.qrc |  1 +
>>>>>>   src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--
>>>>>>   3 files changed, 91 insertions(+), 3 deletions(-)
>>>>>>   create mode 100644 src/qcam/assets/shader/RGB.frag
>>>>>>
>>>>>> diff --git a/src/qcam/assets/shader/RGB.frag
>>>>>> b/src/qcam/assets/shader/RGB.frag
>>>>>> new file mode 100644
>>>>>> index 000000000000..4c374ac98095
>>>>>> --- /dev/null
>>>>>> +++ b/src/qcam/assets/shader/RGB.frag
>>>>>> @@ -0,0 +1,22 @@
>>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>>>>> +/*
>>>>>> + * Copyright (C) 2020, Laurent Pinchart
>>>>>> + *
>>>>>> + * RGB.frag - Fragment shader code for RGB formats
>>>>>> + */
>>>>>> +
>>>>>> +#ifdef GL_ES
>>>>>> +precision mediump float;
>>>>>> +#endif
>>>>>> +
>>>>>> +varying vec2 textureOut;
>>>>>> +uniform sampler2D tex_y;
>>>>>> +
>>>>>> +void main(void)
>>>>>> +{
>>>>>> +    vec3 rgb;
>>>>>> +
>>>>>> +    rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
>>>>>> +
>>>>>> +    gl_FragColor = vec4(rgb, 1.0);
>>>>>> +}
>>>>>> diff --git a/src/qcam/assets/shader/shaders.qrc
>>>>>> b/src/qcam/assets/shader/shaders.qrc
>>>>>> index 863109146281..8a8f9de1a5fa 100644
>>>>>> --- a/src/qcam/assets/shader/shaders.qrc
>>>>>> +++ b/src/qcam/assets/shader/shaders.qrc
>>>>>> @@ -1,6 +1,7 @@
>>>>>>   <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
>>>>>>   <!DOCTYPE RCC><RCC version="1.0">
>>>>>>   <qresource>
>>>>>> +    <file>RGB.frag</file>
>>>>>>       <file>YUV_2_planes.frag</file>
>>>>>>       <file>YUV_3_planes.frag</file>
>>>>>>       <file>YUV_packed.frag</file>
>>>>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
>>>>>> index cbc1365500f5..5d9b442e7985 100644
>>>>>> --- a/src/qcam/viewfinder_gl.cpp
>>>>>> +++ b/src/qcam/viewfinder_gl.cpp
>>>>>> @@ -14,21 +14,28 @@
>>>>>>   #include <libcamera/formats.h>
>>>>>>     static const QList<libcamera::PixelFormat> supportedFormats{
>>>>>> -    /* Packed (single plane) */
>>>>>> +    /* YUV - packed (single plane) */
>>>>>>       libcamera::formats::UYVY,
>>>>>>       libcamera::formats::VYUY,
>>>>>>       libcamera::formats::YUYV,
>>>>>>       libcamera::formats::YVYU,
>>>>>> -    /* Semi planar (two planes) */
>>>>>> +    /* YUV - semi planar (two planes) */
>>>>>>       libcamera::formats::NV12,
>>>>>>       libcamera::formats::NV21,
>>>>>>       libcamera::formats::NV16,
>>>>>>       libcamera::formats::NV61,
>>>>>>       libcamera::formats::NV24,
>>>>>>       libcamera::formats::NV42,
>>>>>> -    /* Fully planar (three planes) */
>>>>>> +    /* YUV - fully planar (three planes) */
>>>>>>       libcamera::formats::YUV420,
>>>>>>       libcamera::formats::YVU420,
>>>>>> +    /* RGB */
>>>>>> +    libcamera::formats::ABGR8888,
>>>>>> +    libcamera::formats::ARGB8888,
>>>>>> +    libcamera::formats::BGRA8888,
>>>>>> +    libcamera::formats::RGBA8888,
>>>>>> +    libcamera::formats::BGR888,
>>>>>> +    libcamera::formats::RGB888,
>>>>>>   };
>>>>>>     ViewFinderGL::ViewFinderGL(QWidget *parent)
>>>>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const
>>>>>> libcamera::PixelFormat &format)
>>>>>>           fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
>>>>>>           fragmentShaderFile_ = ":YUV_packed.frag";
>>>>>>           break;
>>>>>> +    case libcamera::formats::ABGR8888:
>>>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
>>>>>
>>>>> pattern rgb, the same as BGR888, so I presume the A is stripped by not
>>>>> being used as a channel or some other magic elsewhere?
>>>>>
>>>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>>>> +        break;
>>>>>> +    case libcamera::formats::ARGB8888:
>>>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
>>>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>>>> +        break;
>>>>>> +    case libcamera::formats::BGRA8888:
>>>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN gba");
>>>>>
>>>>> Yet here it's specified.... at the end
>>>>>
>>>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>>>> +        break;
>>>>>> +    case libcamera::formats::RGBA8888:
>>>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN abg");
>>>>>
>>>>> but here at the beginning?
>>>>>
>>>>> What is the definition of the RGB_PATTERNs?
>>>>> They don't seem to have a pattern that makes sense to me yet.
>>>>
>>>> The shader language accesses vector components by name. The texture2D()
>>>> function, which samples a texture, returns a vec4, a 4-components
>>>> floating point vector. Multiple component names are supported:
>>>>
>>>> - {x, y, z, w}
>>>> - {r, g, b, a}
>>>> - {s, t, p, q}
>>>>
>>
>> I wish they also had a set of component names which were simply 'linear'
>> (e, f, g, h} for instance
>>
>> (also because those are unused above and 'almost' linear in a qwerty
>> keyboard ;D)
>>
>> Or using an index value 1,2,3,4 but I can see how using numeric indexes
>> here could cause difficulty
>>
>>>> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for
>>>> other names. The reason different names are defined and alias each other
>>>> is that vectors can store different types of data, and standardizing on
>>>> {r, g, b, a} would lead to weird-looking code when the vector stores a
>>>> 3D coordinnate for instance ({s, t, p, q} are used as a convention for
>>>> texture coordinates).
>>>>
>>>> The indexing syntax also supports multiple components to be selected:
>>>>
>>>>     vec4 colour(1.0, 2.0, 3.0, 4.0);
>>>>     vec2 value;
>>>>
>>>>     value = colour.rb;
>>>>
>>>>     // value now contains (1.0, 3.0)
>>>>
>>>> The RGB fragment shader code simply contains
>>>>
>>>>     vec3 rgb;
>>>>
>>>>     rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
>>>>
>>>> The pattern thus defines the components mapping.
>>>>
>>>> One additional piece of information that is required is how OpenGL
>>>> expects texture data to be layed out in memory. The texture is created
>>>> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A
>>>> components are layed down in that order, in one byte each. The RGBA8888
>>>> format, has the opposite convention, it stores RGBA in that order in a
>>>> 32-bit word that is then stored in memory in little endian format
>>>
>>> - Ah... I've missed that little endian part of the puzzle, and have got
>>> confused too.
>>>
>>> Thanks a lot for the detailed explanation!
>>
>> Agreed. Should we add all that to the commit message?
>> It's a really useful explanation - it seems valuable to keep it.
> 
> I knew you would say that :-)
> 
> I'm not sure we really need to document the OpenGL API and the shader
> language in the commit message though. I've expanded the first paragraph
> to the following:

That's fine, but you could reference it if there is a suitable
link/reference ;-)

I doubt many libcamera reader/developers are also OpenGL developers
(well, I'm not - but maybe some others are ;D)


> "Add support for 24-bit and 32-bit RGB formats. The fragment samples the
> texture and reorders the components, using a pattern set through the
> RGB_PATTERN macro. The pattern stores the shader vec4 element indices 
> (named {r, g, b, a} by convention, for elements 0 to 3) to be extracted 
> from the texture samples, when interpreted by OpenGL as RGBA.
> 
> Note that, as textures are created with GL_UNSIGNED_BYTE, the RGBA
> order corresponds to bytes in memory, while the libcamera formats are
> named based on the components order in a 32-bit word stored in memory in
> little endian format."
> 
> I think this provides enough context, please please let me know if you
> disagree.

Highlighting how the indexing works is enough for me to understand the
context of what is not obvious in the code, so that works for me.

Thanks

Kieran


> 
>>> Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>
>>
>> And this too now ;-)
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>>>> , so
>>>> bytes contains A, B, G and R in that order.
>>>>
>>>> So, taking the RGBA8888 example, we have ABGR in memory, which is read
>>>> in a vec4. We want to extract RGB (in that order) in the shader, so we
>>>> need components [3], [2] and [1] of the vector, which are expressed as
>>>> .abg.
>>>>
>>>> I hope this makes it clearer.
>>>>
>>>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>>>> +        break;
>>>>>> +    case libcamera::formats::BGR888:
>>>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
>>>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>>>> +        break;
>>>>>> +    case libcamera::formats::RGB888:
>>>>>> +        fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
>>>>>> +        fragmentShaderFile_ = ":RGB.frag";
>>>>>> +        break;
>>>>>>       default:
>>>>>>           ret = false;
>>>>>>           qWarning() << "[ViewFinderGL]:"
>>>>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()
>>>>>>                              1.0f / (size_.width() / 2 - 1));
>>>>>>           break;
>>>>>> +    case libcamera::formats::ABGR8888:
>>>>>> +    case libcamera::formats::ARGB8888:
>>>>>> +    case libcamera::formats::BGRA8888:
>>>>>> +    case libcamera::formats::RGBA8888:
>>>>>> +        glActiveTexture(GL_TEXTURE0);
>>>>>> +        configureTexture(*textures_[0]);
>>>>>> +        glTexImage2D(GL_TEXTURE_2D,
>>>>>> +                 0,
>>>>>> +                 GL_RGBA,
>>>>>> +                 size_.width(),
>>>>>> +                 size_.height(),
>>>>>> +                 0,
>>>>>> +                 GL_RGBA,
>>>>>> +                 GL_UNSIGNED_BYTE,
>>>>>> +                 data_);
>>>>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);
>>>>>> +        break;
>>>>>> +
>>>>>> +    case libcamera::formats::BGR888:
>>>>>> +    case libcamera::formats::RGB888:
>>>>>> +        glActiveTexture(GL_TEXTURE0);
>>>>>> +        configureTexture(*textures_[0]);
>>>>>> +        glTexImage2D(GL_TEXTURE_2D,
>>>>>> +                 0,
>>>>>> +                 GL_RGB,
>>>>>> +                 size_.width(),
>>>>>> +                 size_.height(),
>>>>>> +                 0,
>>>>>> +                 GL_RGB,
>>>>>> +                 GL_UNSIGNED_BYTE,
>>>>>> +                 data_);
>>>>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);
>>>>>> +        break;
>>>>>> +
>>>>>>       default:
>>>>>>           break;
>>>>>>       };
>
Niklas Söderlund Nov. 6, 2020, 11:17 a.m. UTC | #7 
Hi Laurent,

Thanks for your patch.

On 2020-11-03 17:50:25 +0200, Laurent Pinchart wrote:
> Add support for 24-bit and 32-bit RGB formats. The fragment samples the
> texture and reorders the components, using a pattern set through the
> RGB_PATTERN macro.
> 
> An alternative to manual reordering in the shader would be to set the
> texture swizzling mask. This is however not available in OpenGL ES
> before version 3.0, which we don't mandate at the moment.
> 
> Only the BGR888 and RGB888 formats have been tested, with the vimc
> pipeline handler.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/qcam/assets/shader/RGB.frag    | 22 +++++++++
>  src/qcam/assets/shader/shaders.qrc |  1 +
>  src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--
>  3 files changed, 91 insertions(+), 3 deletions(-)
>  create mode 100644 src/qcam/assets/shader/RGB.frag
> 
> diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag
> new file mode 100644
> index 000000000000..4c374ac98095
> --- /dev/null
> +++ b/src/qcam/assets/shader/RGB.frag
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Laurent Pinchart
> + *
> + * RGB.frag - Fragment shader code for RGB formats
> + */
> +
> +#ifdef GL_ES
> +precision mediump float;
> +#endif
> +
> +varying vec2 textureOut;
> +uniform sampler2D tex_y;
> +
> +void main(void)
> +{
> +	vec3 rgb;
> +
> +	rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
> +
> +	gl_FragColor = vec4(rgb, 1.0);
> +}
> diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc
> index 863109146281..8a8f9de1a5fa 100644
> --- a/src/qcam/assets/shader/shaders.qrc
> +++ b/src/qcam/assets/shader/shaders.qrc
> @@ -1,6 +1,7 @@
>  <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
>  <!DOCTYPE RCC><RCC version="1.0">
>  <qresource>
> +	<file>RGB.frag</file>
>  	<file>YUV_2_planes.frag</file>
>  	<file>YUV_3_planes.frag</file>
>  	<file>YUV_packed.frag</file>
> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> index cbc1365500f5..5d9b442e7985 100644
> --- a/src/qcam/viewfinder_gl.cpp
> +++ b/src/qcam/viewfinder_gl.cpp
> @@ -14,21 +14,28 @@
>  #include <libcamera/formats.h>
>  
>  static const QList<libcamera::PixelFormat> supportedFormats{
> -	/* Packed (single plane) */
> +	/* YUV - packed (single plane) */
>  	libcamera::formats::UYVY,
>  	libcamera::formats::VYUY,
>  	libcamera::formats::YUYV,
>  	libcamera::formats::YVYU,
> -	/* Semi planar (two planes) */
> +	/* YUV - semi planar (two planes) */
>  	libcamera::formats::NV12,
>  	libcamera::formats::NV21,
>  	libcamera::formats::NV16,
>  	libcamera::formats::NV61,
>  	libcamera::formats::NV24,
>  	libcamera::formats::NV42,
> -	/* Fully planar (three planes) */
> +	/* YUV - fully planar (three planes) */
>  	libcamera::formats::YUV420,
>  	libcamera::formats::YVU420,
> +	/* RGB */
> +	libcamera::formats::ABGR8888,
> +	libcamera::formats::ARGB8888,
> +	libcamera::formats::BGRA8888,
> +	libcamera::formats::RGBA8888,
> +	libcamera::formats::BGR888,
> +	libcamera::formats::RGB888,
>  };
>  
>  ViewFinderGL::ViewFinderGL(QWidget *parent)
> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)
>  		fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
>  		fragmentShaderFile_ = ":YUV_packed.frag";
>  		break;
> +	case libcamera::formats::ABGR8888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::ARGB8888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::BGRA8888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN gba");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::RGBA8888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN abg");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::BGR888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
> +	case libcamera::formats::RGB888:
> +		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
> +		fragmentShaderFile_ = ":RGB.frag";
> +		break;
>  	default:
>  		ret = false;
>  		qWarning() << "[ViewFinderGL]:"
> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()
>  					       1.0f / (size_.width() / 2 - 1));
>  		break;
>  
> +	case libcamera::formats::ABGR8888:
> +	case libcamera::formats::ARGB8888:
> +	case libcamera::formats::BGRA8888:
> +	case libcamera::formats::RGBA8888:
> +		glActiveTexture(GL_TEXTURE0);
> +		configureTexture(*textures_[0]);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RGBA,
> +			     size_.width(),
> +			     size_.height(),
> +			     0,
> +			     GL_RGBA,
> +			     GL_UNSIGNED_BYTE,
> +			     data_);
> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> +		break;
> +
> +	case libcamera::formats::BGR888:
> +	case libcamera::formats::RGB888:
> +		glActiveTexture(GL_TEXTURE0);
> +		configureTexture(*textures_[0]);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RGB,
> +			     size_.width(),
> +			     size_.height(),
> +			     0,
> +			     GL_RGB,
> +			     GL_UNSIGNED_BYTE,
> +			     data_);
> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> +		break;
> +
>  	default:
>  		break;
>  	};
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch
diff mbox series 

diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag
new file mode 100644
index 000000000000..4c374ac98095
--- /dev/null
+++ b/src/qcam/assets/shader/RGB.frag
@@ -0,0 +1,22 @@ 
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2020, Laurent Pinchart
+ *
+ * RGB.frag - Fragment shader code for RGB formats
+ */
+
+#ifdef GL_ES
+precision mediump float;
+#endif
+
+varying vec2 textureOut;
+uniform sampler2D tex_y;
+
+void main(void)
+{
+	vec3 rgb;
+
+	rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
+
+	gl_FragColor = vec4(rgb, 1.0);
+}
diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc
index 863109146281..8a8f9de1a5fa 100644
--- a/src/qcam/assets/shader/shaders.qrc
+++ b/src/qcam/assets/shader/shaders.qrc
@@ -1,6 +1,7 @@ 
 <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
 <!DOCTYPE RCC><RCC version="1.0">
 <qresource>
+	<file>RGB.frag</file>
 	<file>YUV_2_planes.frag</file>
 	<file>YUV_3_planes.frag</file>
 	<file>YUV_packed.frag</file>
diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
index cbc1365500f5..5d9b442e7985 100644
--- a/src/qcam/viewfinder_gl.cpp
+++ b/src/qcam/viewfinder_gl.cpp
@@ -14,21 +14,28 @@ 
 #include <libcamera/formats.h>
 
 static const QList<libcamera::PixelFormat> supportedFormats{
-	/* Packed (single plane) */
+	/* YUV - packed (single plane) */
 	libcamera::formats::UYVY,
 	libcamera::formats::VYUY,
 	libcamera::formats::YUYV,
 	libcamera::formats::YVYU,
-	/* Semi planar (two planes) */
+	/* YUV - semi planar (two planes) */
 	libcamera::formats::NV12,
 	libcamera::formats::NV21,
 	libcamera::formats::NV16,
 	libcamera::formats::NV61,
 	libcamera::formats::NV24,
 	libcamera::formats::NV42,
-	/* Fully planar (three planes) */
+	/* YUV - fully planar (three planes) */
 	libcamera::formats::YUV420,
 	libcamera::formats::YVU420,
+	/* RGB */
+	libcamera::formats::ABGR8888,
+	libcamera::formats::ARGB8888,
+	libcamera::formats::BGRA8888,
+	libcamera::formats::RGBA8888,
+	libcamera::formats::BGR888,
+	libcamera::formats::RGB888,
 };
 
 ViewFinderGL::ViewFinderGL(QWidget *parent)
@@ -172,6 +179,30 @@  bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)
 		fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
 		fragmentShaderFile_ = ":YUV_packed.frag";
 		break;
+	case libcamera::formats::ABGR8888:
+		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
+		fragmentShaderFile_ = ":RGB.frag";
+		break;
+	case libcamera::formats::ARGB8888:
+		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
+		fragmentShaderFile_ = ":RGB.frag";
+		break;
+	case libcamera::formats::BGRA8888:
+		fragmentShaderDefines_.append("#define RGB_PATTERN gba");
+		fragmentShaderFile_ = ":RGB.frag";
+		break;
+	case libcamera::formats::RGBA8888:
+		fragmentShaderDefines_.append("#define RGB_PATTERN abg");
+		fragmentShaderFile_ = ":RGB.frag";
+		break;
+	case libcamera::formats::BGR888:
+		fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
+		fragmentShaderFile_ = ":RGB.frag";
+		break;
+	case libcamera::formats::RGB888:
+		fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
+		fragmentShaderFile_ = ":RGB.frag";
+		break;
 	default:
 		ret = false;
 		qWarning() << "[ViewFinderGL]:"
@@ -481,6 +512,40 @@  void ViewFinderGL::doRender()
 					       1.0f / (size_.width() / 2 - 1));
 		break;
 
+	case libcamera::formats::ABGR8888:
+	case libcamera::formats::ARGB8888:
+	case libcamera::formats::BGRA8888:
+	case libcamera::formats::RGBA8888:
+		glActiveTexture(GL_TEXTURE0);
+		configureTexture(*textures_[0]);
+		glTexImage2D(GL_TEXTURE_2D,
+			     0,
+			     GL_RGBA,
+			     size_.width(),
+			     size_.height(),
+			     0,
+			     GL_RGBA,
+			     GL_UNSIGNED_BYTE,
+			     data_);
+		shaderProgram_.setUniformValue(textureUniformY_, 0);
+		break;
+
+	case libcamera::formats::BGR888:
+	case libcamera::formats::RGB888:
+		glActiveTexture(GL_TEXTURE0);
+		configureTexture(*textures_[0]);
+		glTexImage2D(GL_TEXTURE_2D,
+			     0,
+			     GL_RGB,
+			     size_.width(),
+			     size_.height(),
+			     0,
+			     GL_RGB,
+			     GL_UNSIGNED_BYTE,
+			     data_);
+		shaderProgram_.setUniformValue(textureUniformY_, 0);
+		break;
+
 	default:
 		break;
 	};