[libcamera-devel] [PATCH v2 1/2] qcam: assets: shader: bayer_8.frag: Add precision

[Kunal Agarwal]

Hi Laurent,

> That's true for OpenGL ES only, I'd update this to
>
> The OpenGL ES shading language has no default precision declared
> implicitly for floats in fragment shaders. The lack of an explicit
> default precision results in shader compilation errors

Yes, mentioning OpenGl ES would be required. Thanks

> What do you mean by "Also suitable for textures" ?

This can be ignored I think. Not very sure of it. Read it somewhere

> Which hardware would that be ? If you don't know, I'd write this as
>
> Specify a default precision of mediump for floats. This matches the
> other fragment shaders, and is guaranteed by the OpenGL ES shader
> language specification to be supported by all devices, while the higher
> precision highp is optional.

Now that I read about it again, that limitation is only for some older phones.

> If you're fine with the above modifications, there's no need to resubmit
> the patch, I can update the commit message when applying.

Yes, That would be great. Thanks

Regards,

Kunal Agarwal




On Fri, Jul 22, 2022 at 1:53 AM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> On Thu, Jul 21, 2022 at 11:21:45PM +0300, Laurent Pinchart wrote:
> > Hi Kunal,
> >
> > Thank you for the patch.
> >
> > On Wed, Jun 29, 2022 at 12:42:38AM +0530, Kunal Agarwal via libcamera-devel wrote:
> > > Currently there is no defined precision for data types
> > > which results in compilation errors for shader.
> >
> > That's true for OpenGL ES only, I'd update this to
> >
> > The OpenGL ES shading language has no default precision declared
> > implicitly for floats in fragment shaders. The lack of an explicit
> > default precision results in shader compilation errors
> >
> > > Adding precision mediump for sufficient and required
> > > range and accuracy. Also suitable for textures.
> >
> > What do you mean by "Also suitable for textures" ?
> >
> > > Precision highp also works but is not supported by some
> > > older hardware devices and consumes more memory.
> >
> > Which hardware would that be ? If you don't know, I'd write this as
> >
> > Specify a default precision of mediump for floats. This matches the
> > other fragment shaders, and is guaranteed by the OpenGL ES shader
> > language specification to be supported by all devices, while the higher
> > precision highp is optional.
> >
> > If you're fine with the above modifications, there's no need to resubmit
> > the patch, I can update the commit message when applying.
>
> And I forgot to mention,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > > Signed-off-by: Kunal Agarwal <kunalagarwal1072002 at gmail.com>
> > > ---
> > >  src/qcam/assets/shader/bayer_8.frag | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/src/qcam/assets/shader/bayer_8.frag b/src/qcam/assets/shader/bayer_8.frag
> > > index 4ece44ab..7e35ca88 100644
> > > --- a/src/qcam/assets/shader/bayer_8.frag
> > > +++ b/src/qcam/assets/shader/bayer_8.frag
> > > @@ -15,6 +15,9 @@ Copyright (C) 2021, Linaro
> > >  */
> > >
> > >  //Pixel Shader
> > > +#ifdef GL_ES
> > > +precision mediump float;
> > > +#endif
> > >
> > >  /** Monochrome RGBA or GL_LUMINANCE Bayer encoded texture.*/
> > >  uniform sampler2D       tex_y;
>
> --
> Regards,
>
> Laurent Pinchart