[vlc-devel] [PATCH] opengl: insert shader headers at compile time

[Alexandre Janniaux]

Hi,

We can reintroduce the field later when we add a correct
support for this. But that's not what the commit message is
conveying.

Regards,
--
Alexandre Janniaux
Videolabs

On Thu, Jun 18, 2020 at 10:52:51AM +0200, Romain Vimont wrote:
> On Thu, Jun 18, 2020 at 10:23:21AM +0200, Alexandre Janniaux wrote:
> > Hi,
> >
> > It doesn't feel like a good idea, and goes against Haasn
> > patches a few week ago.
>
> I think this change is independant of his patches.
>
> (We already discussed about this for commit
> 2fdbd23f2806cdad15ca03e0b83de46e543f6b6c.)
>
> > Are there reasons to do so?
>
> Currently, the renderer initializes two fields (glsl_version and
> glsl_precision_header) from constants, and inject their value into the
> shader code (via asprintf).
>
> Using these struct fields is useless: it's simpler to just put the
> constants in the string.
>
> Btw, the sub_renderer already does that. This change had also been
> independently proposed by Marvin:
> https://mailman.videolan.org/pipermail/vlc-devel/2020-March/132784.html
>
> >
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Wed, Jun 17, 2020 at 09:33:11PM +0200, Romain Vimont wrote:
> > > The version and precision header strings are different for OpenGL and
> > > OpenGL ES.
> > >
> > > Insert them directly into the shader string literal, instead of
> > > initializing renderer fields and inserting them at runtime.
> > > ---
> > >  modules/video_output/opengl/renderer.c | 31 ++++++++++++++------------
> > >  modules/video_output/opengl/renderer.h |  6 -----
> > >  2 files changed, 17 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/modules/video_output/opengl/renderer.c b/modules/video_output/opengl/renderer.c
> > > index 335ca27e4841..6f7d42ee7ab9 100644
> > > --- a/modules/video_output/opengl/renderer.c
> > > +++ b/modules/video_output/opengl/renderer.c
> > > @@ -169,12 +169,23 @@ InitStereoMatrix(GLfloat matrix_out[static 3*3],
> > >  #undef ROW
> > >  }
> > >
> > > +/* https://en.wikipedia.org/wiki/OpenGL_Shading_Language#Versions */
> > > +#ifdef USE_OPENGL_ES2
> > > +# define SHADER_VERSION "#version 100\n"
> > > +  /* In OpenGL ES, the fragment language has no default precision qualifier for
> > > +   * floating point types. */
> > > +# define FRAGMENT_SHADER_PRECISION "precision highp float;\n"
> > > +#else
> > > +# define SHADER_VERSION "#version 120\n"
> > > +# define FRAGMENT_SHADER_PRECISION
> > > +#endif
> > > +
> > >  static char *
> > >  BuildVertexShader(const struct vlc_gl_renderer *renderer)
> > >  {
> > >      /* Basic vertex shader */
> > >      static const char *template =
> > > -        "#version %u\n"
> > > +        SHADER_VERSION
> > >          "attribute vec2 PicCoordsIn;\n"
> > >          "varying vec2 PicCoords;\n"
> > >          "attribute vec3 VertexPosition;\n"
> > > @@ -188,8 +199,8 @@ BuildVertexShader(const struct vlc_gl_renderer *renderer)
> > >          "               * vec4(VertexPosition, 1.0);\n"
> > >          "}";
> > >
> > > -    char *code;
> > > -    if (asprintf(&code, template, renderer->glsl_version) < 0)
> > > +    char *code = strdup(template);
> > > +    if (!code)
> > >          return NULL;
> > >
> > >      if (renderer->dump_shaders)
> > > @@ -204,9 +215,9 @@ BuildFragmentShader(struct vlc_gl_renderer *renderer)
> > >      struct vlc_gl_sampler *sampler = renderer->sampler;
> > >
> > >      static const char *template =
> > > -        "#version %u\n"
> > > +        SHADER_VERSION
> > >          "%s" /* extensions */
> > > -        "%s" /* precision header */
> > > +        FRAGMENT_SHADER_PRECISION
> > >          "%s" /* vlc_texture definition */
> > >          "varying vec2 PicCoords;\n"
> > >          "void main() {\n"
> > > @@ -217,8 +228,7 @@ BuildFragmentShader(struct vlc_gl_renderer *renderer)
> > >                             ? sampler->shader.extensions : "";
> > >
> > >      char *code;
> > > -    int ret = asprintf(&code, template, renderer->glsl_version, extensions,
> > > -                       renderer->glsl_precision_header, sampler->shader.body);
> > > +    int ret = asprintf(&code, template, extensions, sampler->shader.body);
> > >      if (ret < 0)
> > >          return NULL;
> > >
> > > @@ -328,13 +338,6 @@ vlc_gl_renderer_New(vlc_gl_t *gl, const struct vlc_gl_api *api,
> > >      renderer->api = api;
> > >      renderer->vt = vt;
> > >      renderer->dump_shaders = var_InheritInteger(gl, "verbose") >= 4;
> > > -#if defined(USE_OPENGL_ES2)
> > > -    renderer->glsl_version = 100;
> > > -    renderer->glsl_precision_header = "precision highp float;\n";
> > > -#else
> > > -    renderer->glsl_version = 120;
> > > -    renderer->glsl_precision_header = "";
> > > -#endif
> > >
> > >      int ret = opengl_link_program(renderer);
> > >      if (ret != VLC_SUCCESS)
> > > diff --git a/modules/video_output/opengl/renderer.h b/modules/video_output/opengl/renderer.h
> > > index 6f4029504fb2..7130cfb26263 100644
> > > --- a/modules/video_output/opengl/renderer.h
> > > +++ b/modules/video_output/opengl/renderer.h
> > > @@ -50,12 +50,6 @@ struct vlc_gl_renderer
> > >      /* True to dump shaders */
> > >      bool dump_shaders;
> > >
> > > -    /* GLSL version, set by the caller. 100 for GLSL ES, 120 for desktop GLSL */
> > > -    unsigned glsl_version;
> > > -    /* Precision header, set by the caller. In OpenGLES, the fragment language
> > > -     * has no default precision qualifier for floating point types. */
> > > -    const char *glsl_precision_header;
> > > -
> > >      GLuint program_id;
> > >
> > >      struct {
> > > --
> > > 2.27.0
> > >
> > > _______________________________________________
> > > vlc-devel mailing list
> > > To unsubscribe or modify your subscription options:
> > > https://mailman.videolan.org/listinfo/vlc-devel
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel