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

[Dave Airlie]

>
> 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?

Hmm I sent a v2 explaining what should be in here, but I'll go back and check
the logic :-)


The conditions we don't need a view if it's a single level or it's all
the levels.

Dave.