[vlc-devel] [PATCH 11/17] opengl: fetch locations from sampler

[Romain Vimont]

On Wed, May 13, 2020 at 05:56:35PM +0200, Alexandre Janniaux wrote:
> Hi,
> 
> The patch seems valid, alhtough I again have bikeshedding
> inline. They can probably be moved to later patches or
> discarded though.
> 
> On Thu, Apr 02, 2020 at 02:24:24PM +0200, Romain Vimont wrote:
> > Fetch locations related to sampler from sampler->pf_fetch_locations.
> > ---
> >  .../video_output/opengl/fragment_shaders.c    | 39 ++++++++++++++++++-
> >  modules/video_output/opengl/renderer.c        | 16 +-------
> >  2 files changed, 38 insertions(+), 17 deletions(-)
> >
> > diff --git a/modules/video_output/opengl/fragment_shaders.c b/modules/video_output/opengl/fragment_shaders.c
> > index 2a196a609c..b0b718b3c7 100644
> > --- a/modules/video_output/opengl/fragment_shaders.c
> > +++ b/modules/video_output/opengl/fragment_shaders.c
> > @@ -218,13 +218,30 @@ sampler_base_fetch_locations(struct vlc_gl_sampler *sampler, GLuint program)
> >              return VLC_EGENERIC;
> >      }
> >
> > +    sampler->uloc.TransformMatrix =
> > +        vt->GetUniformLocation(program, "TransformMatrix");
> > +    if (sampler->uloc.TransformMatrix == -1)
> > +        return VLC_EGENERIC;
> > +
> > +    sampler->uloc.OrientationMatrix =
> > +        vt->GetUniformLocation(program, "OrientationMatrix");
> > +    if (sampler->uloc.OrientationMatrix == -1)
> > +        return VLC_EGENERIC;
> > +
> >      for (unsigned int i = 0; i < interop->tex_count; ++i)
> >      {
> > -        char name[sizeof("TextureX")];
> > +        char name[sizeof("TexCoordsMapX")];
> > +
> 
> It could benefit from an assert(interop->tex_count < 10) for
> both texture and texcoordsmap.

OK, I added an assert.

_In practice, interop->tex_count may never be greater than
PICTURE_PLANE_MAX (5), otherwise it would crash in other places._
> 
> >          snprintf(name, sizeof(name), "Texture%1u", i);
> >          sampler->uloc.Texture[i] = vt->GetUniformLocation(program, name);
> >          if (sampler->uloc.Texture[i] == -1)
> >              return VLC_EGENERIC;
> > +
> > +        snprintf(name, sizeof(name), "TexCoordsMap%1u", i);
> > +        sampler->uloc.TexCoordsMap[i] = vt->GetUniformLocation(program, name);
> > +        if (sampler->uloc.TexCoordsMap[i] == -1)
> > +            return VLC_EGENERIC;
> > +
> >          if (interop->tex_target == GL_TEXTURE_RECTANGLE)
> >          {
> >              snprintf(name, sizeof(name), "TexSize%1u", i);
> > @@ -314,7 +331,25 @@ sampler_xyz12_fetch_locations(struct vlc_gl_sampler *sampler, GLuint program)
> >      const opengl_vtable_t *vt = sampler->vt;
> >
> >      sampler->uloc.Texture[0] = vt->GetUniformLocation(program, "Texture0");
> > -    return sampler->uloc.Texture[0] != -1 ? VLC_SUCCESS : VLC_EGENERIC;
> > +    if (sampler->uloc.Texture[0] == -1)
> > +        return VLC_EGENERIC;
> > +
> > +    sampler->uloc.TransformMatrix =
> > +        vt->GetUniformLocation(program, "TransformMatrix");
> > +    if (sampler->uloc.TransformMatrix == -1)
> > +        return VLC_EGENERIC;
> > +
> > +    sampler->uloc.OrientationMatrix =
> > +        vt->GetUniformLocation(program, "OrientationMatrix");
> > +    if (sampler->uloc.OrientationMatrix == -1)
> > +        return VLC_EGENERIC;
> > +
> > +    sampler->uloc.TexCoordsMap[0] =
> > +        vt->GetUniformLocation(program, "TexCoordsMap0");
> > +    if (sampler->uloc.TexCoordsMap[0] == -1)
> > +        return VLC_EGENERIC;
> > +
> > +    return VLC_SUCCESS;
> >  }
> >
> >  static void
> > diff --git a/modules/video_output/opengl/renderer.c b/modules/video_output/opengl/renderer.c
> > index 81655b92ea..5fb4f85104 100644
> > --- a/modules/video_output/opengl/renderer.c
> > +++ b/modules/video_output/opengl/renderer.c
> > @@ -357,24 +357,10 @@ opengl_link_program(struct vlc_gl_renderer *renderer)
> >
> >      GET_ALOC(PicCoordsIn, "PicCoordsIn");
> >      GET_ALOC(VertexPosition, "VertexPosition");
> > -
> > -#define GET_SAMPLER_ULOC(x, str) GET_LOC(Uniform, sampler->uloc.x, str)
> > -    GET_SAMPLER_ULOC(TransformMatrix, "TransformMatrix");
> > -    GET_SAMPLER_ULOC(OrientationMatrix, "OrientationMatrix");
> > -    GET_SAMPLER_ULOC(TexCoordsMap[0], "TexCoordsMap0");
> > -    /* MultiTexCoord 1 and 2 can be optimized out if not used */
> > -    if (interop->tex_count > 1)
> > -        GET_SAMPLER_ULOC(TexCoordsMap[1], "TexCoordsMap1");
> > -    else
> > -        sampler->uloc.TexCoordsMap[1] = -1;
> > -    if (interop->tex_count > 2)
> > -        GET_SAMPLER_ULOC(TexCoordsMap[2], "TexCoordsMap2");
> > -    else
> > -        sampler->uloc.TexCoordsMap[2] = -1;
> >  #undef GET_LOC
> >  #undef GET_ULOC
> >  #undef GET_ALOC
> > -#undef GET_SAMPLER_ULOC
> > +
> >      int ret = sampler->pf_fetch_locations(sampler, program_id);
> >      assert(ret == VLC_SUCCESS);
> >      if (ret != VLC_SUCCESS)
> > --
> > 2.26.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