[virglrenderer-devel] [PATCH 5/6] renderer: add texture view internals

Gert Wollny gert.wollny at collabora.com
Wed Jun 20 16:20:22 UTC 2018


Patches 1-4, 6 
Reviewed-by: Gert Wollny <gert.wollny at collabora.com>

For this patch I'm not quite sure, so I added some comments below. The
series doesn't apply cleanly to the current master though, so I didn't
test it yet. 

Best, 
Gert 

Am Freitag, den 08.06.2018, 11:18 +1000 schrieb Dave Airlie:
> From: Dave Airlie <airlied at redhat.com>
> 
> This adds the renderer support for exposing texture views.
> 
> The change adds support to sampler views and surfaces for the backing
> object to be a separately allocated textureview instead of the actual
> resource. It also adds support for parsing the updated packet where
> we put the sampler view target in the top 8 bits of the format,
> which should be safe as up until now it's always 0.
> 
> This doesn't do the final enable of the extension, but it will
> cause us to use views in a few places we didn't before so could have
> some other side effects
> ---
>  src/vrend_renderer.c | 93
> +++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 85 insertions(+), 8 deletions(-)
> 
> diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> index 80a469b..10aafdc 100644
> --- a/src/vrend_renderer.c
> +++ b/src/vrend_renderer.c
> @@ -119,6 +119,7 @@ struct global_renderer_state {
>     bool have_texture_buffer_range;
>     bool have_polygon_offset_clamp;
>     bool have_texture_storage;
> +   bool have_texture_view;
>  
>     /* these appeared broken on at least one driver */
>     bool use_explicit_locations;
> @@ -230,6 +231,7 @@ struct vrend_sampler_view {
>     struct pipe_reference reference;
>     GLuint id;
>     GLuint format;
> +   GLenum target;
>     GLuint val0, val1;
>     GLuint gl_swizzle_r;
>     GLuint gl_swizzle_g;
> @@ -424,6 +426,8 @@ static void vrend_apply_sampler_state(struct
> vrend_context *ctx,
>                                        struct vrend_resource *res,
>                                        uint32_t shader_type,
>                                        int id, int sampler_id,
> uint32_t srgb_decode);
> +static GLenum tgsitargettogltarget(const enum pipe_texture_target
> target, int nr_samples);
> +
>  void vrend_update_stencil_state(struct vrend_context *ctx);
>  
>  static struct vrend_format_table tex_conv_table[VIRGL_FORMAT_MAX];
> @@ -556,6 +560,8 @@ static inline bool should_invert_viewport(struct
> vrend_context *ctx)
>  
>  static void vrend_destroy_surface(struct vrend_surface *surf)
>  {
> +   if (surf->id != surf->texture->id)
> +      glDeleteTextures(1, &surf->id);
>     vrend_resource_reference(&surf->texture, NULL);
>     free(surf);
>  }
> @@ -572,6 +578,8 @@ vrend_surface_reference(struct vrend_surface
> **ptr, struct vrend_surface *surf)
>  
>  static void vrend_destroy_sampler_view(struct vrend_sampler_view
> *samp)
>  {
> +   if (samp->texture->id != samp->id)
> +      glDeleteTextures(1, &samp->id);
>     vrend_resource_reference(&samp->texture, NULL);
>     free(samp);
>  }
> @@ -1162,6 +1170,28 @@ int vrend_create_surface(struct vrend_context
> *ctx,
>     surf->format = format;
>     surf->val0 = val0;
>     surf->val1 = val1;
> +   surf->id = res->id;
> +
> +   if (vrend_state.have_texture_view && !res->is_buffer) {
> +      /*
> +       * GL has no simple API to map a subrange of layers to a
> framebuffer,
> +       * the method for doing this requires using a texture view,
> and then
> +       * binding the view to a framebuffer. Also need to use this
> path
> +       * if we are binding a different format to the surface.
> +       */
> +      int first_layer = surf->val1 & 0xffff;
> +      int last_layer = (surf->val1 >> 16) & 0xffff;
> +      if ((first_layer != last_layer &&
Shouldn't a one-slice view also be allowed? 


> +           (first_layer != 0 && (last_layer + 1 != res-
> >base.array_size))) ||
> +          surf->format != res->base.format) {
Not quite sure why you don't allow (first_layer == 0) or 
(last_layer + 1 != res->base.array_size), could it be that you meant 

 (!(first_layer == 0) && (last_layer + 1 == res->base.array_size))) 

i.e. the case when the view would be the same like the full texture
array?

> +          surf->format != res->base.format) {
> +         GLenum internalformat = tex_conv_table[surf-
> >format].internalformat;
> +         glGenTextures(1, &surf->id);
> +         glTextureView(surf->id, res->target, res->id,
> internalformat,
> +                       0, res->base.last_level + 1,
> +                       first_layer, last_layer - first_layer + 1);
> +      }
> +   }
> +
>     pipe_reference_init(&surf->reference, 1);
>  
>     vrend_resource_reference(&surf->texture, res);
> @@ -1366,7 +1396,8 @@ int vrend_create_sampler_view(struct
> vrend_context *ctx,
>        return ENOMEM;
>  
>     pipe_reference_init(&view->reference, 1);
> -   view->format = format;
> +   view->format = format & 0xffffff;
> +   view->target = tgsitargettogltarget((format >> 24) & 0xff, res-
> >base.nr_samples);
>     view->val0 = val0;
>     view->val1 = val1;
>     view->cur_base = -1;
> @@ -1379,6 +1410,49 @@ int vrend_create_sampler_view(struct
> vrend_context *ctx,
>  
>     vrend_resource_reference(&view->texture, res);
>  
> +   view->id = view->texture->id;
> +   if (!view->target)
> +      view->target = view->texture->target;
> +
> +   if (vrend_state.have_texture_view) {
> +      enum pipe_format format;
> +      bool needs_view = false;
> +
> +      /*
> +       * Need to use a texture view if the gallium
> +       * view target is different than the underlying
> +       * texture target.
> +       */
> +      if (view->target != view->texture->target)
> +         needs_view = true;
> +
> +      /*
> +       * If the formats are different and this isn't
> +       * a DS texture a view is required.
> +       * DS are special as they use different gallium
> +       * formats for DS views into a combined resource.
> +       * GL texture views can't be use for this, stencil
> +       * texturing is used instead. For DS formats
> +       * aways program the underlying DS format as a
> +       * view could be required for layers.
> +       */
> +      format = view->format;
> +      if (util_format_is_depth_or_stencil(view->texture-
> >base.format))
> +         format = view->texture->base.format;
> +      else if (view->format != view->texture->base.format)
> +         needs_view = true;
> +      if (needs_view) {
> +        glGenTextures(1, &view->id);
> +        GLenum internalformat =
> tex_conv_table[format].internalformat;
> +        unsigned base_layer = view->val0 & 0xffff;
> +        unsigned max_layer = (view->val0 >> 16) & 0xffff;
> +        view->cur_base = view->val1 & 0xff;
> +        view->cur_max = (view->val1 >> 8) & 0xff;
> +        glTextureView(view->id, view->target, view->texture->id,
> internalformat,
> +                      view->cur_base, (view->cur_max - view-
> >cur_base) + 1,
> +                      base_layer, max_layer - base_layer + 1);
> +     }
> +   }
>     view->srgb_decode = GL_DECODE_EXT;
>     if (view->format != view->texture->base.format) {
>        if (util_format_is_srgb(view->texture->base.format) &&
> @@ -1518,8 +1592,8 @@ static void vrend_hw_set_zsurf_texture(struct
> vrend_context *ctx)
>        if (!surf->texture)
>           return;
>  
> -      vrend_fb_bind_texture(surf->texture, 0, surf->val0,
> -                            first_layer != last_layer ? 0xffffffff :
> first_layer);
> +      vrend_fb_bind_texture_id(surf->texture, surf->id, 0, surf-
> >val0,
> +			       first_layer != last_layer ?
> 0xffffffff : first_layer);
>     }
>  }
>  
> @@ -1536,8 +1610,8 @@ static void vrend_hw_set_color_surface(struct
> vrend_context *ctx, int index)
>        int first_layer = surf->val1 & 0xffff;
>        int last_layer = (surf->val1 >> 16) & 0xffff;
>  
> -      vrend_fb_bind_texture(surf->texture, index, surf->val0,
> -                            first_layer != last_layer ? 0xffffffff :
> first_layer);
> +      vrend_fb_bind_texture_id(surf->texture, surf->id, index, surf-
> >val0,
> +                               first_layer != last_layer ?
> 0xffffffff : first_layer);
>     }
>  }
>  
> @@ -2035,7 +2109,7 @@ void vrend_set_single_sampler_view(struct
> vrend_context *ctx,
>           return;
>        }
>        if (!view->texture->is_buffer) {
> -         glBindTexture(view->texture->target, view->texture->id);
> +         glBindTexture(view->target, view->id);
>  
>           if (util_format_is_depth_or_stencil(view->format)) {
>              if (vrend_state.use_core_profile == false) {
> @@ -2854,13 +2928,13 @@ static void vrend_draw_bind_samplers(struct
> vrend_context *ctx)
>           if (tview->texture) {
>              int id;
>              struct vrend_resource *texture = tview->texture;
> -            GLenum target = texture->target;
> +            GLenum target = tview->target;
>  
>              if (texture->is_buffer) {
>                 id = texture->tbo_tex_id;
>                 target = GL_TEXTURE_BUFFER;
>              } else
> -               id = texture->id;
> +               id = tview->id;
>  
>              glBindTexture(target, id);
>              if (ctx->sub->views[shader_type].old_ids[i] != id ||
> ctx->sub->sampler_state_dirty) {
> @@ -4261,6 +4335,9 @@ int vrend_renderer_init(struct vrend_if_cbs
> *cbs, uint32_t flags)
>     if (gl_ver >= 42 ||
> epoxy_has_gl_extension("GL_ARB_texture_storage"))
>        vrend_state.have_texture_storage = true;
>  
> +   if (gl_ver >= 43 ||
> epoxy_has_gl_extension("GL_ARB_texture_view"))
> +      vrend_state.have_texture_view = true;
> +
>     if (gl_ver >= 46 ||
> epoxy_has_gl_extension("GL_ARB_polygon_offset_clamp"))
>        vrend_state.have_polygon_offset_clamp = true;
>  


More information about the virglrenderer-devel mailing list