Re: [Mesa-dev] [PATCH 1/3] vl/dri3: use external texture as back buffers(v2)

[Leo Liu]

On 11/04/2016 01:24 PM, Nayan Deshmukh wrote:

On Fri, Nov 04, 2016 at 12:20:51PM -0400, Leo Liu wrote:

Hi Nayan,

With this patch, the resizing corruption is fixed, thanks for that.

Still a few comments below.


On 11/04/2016 03:08 AM, Nayan Deshmukh wrote:

dri3 allows us to send handle of a texture directly to X
so this patch allows a state tracker to directly send its
texture to X to be used as back buffer and avoids extra
copying

v2: use clip width/height to display a portion of the surface

Suggested-by: Leo Liu 
Signed-off-by: Nayan Deshmukh 
---
   src/gallium/auxiliary/vl/vl_winsys.h  |   4 ++
   src/gallium/auxiliary/vl/vl_winsys_dri3.c | 109 
++
   2 files changed, 99 insertions(+), 14 deletions(-)

diff --git a/src/gallium/auxiliary/vl/vl_winsys.h 
b/src/gallium/auxiliary/vl/vl_winsys.h
index 26db9f2..908e9c6 100644
--- a/src/gallium/auxiliary/vl/vl_winsys.h
+++ b/src/gallium/auxiliary/vl/vl_winsys.h
@@ -59,6 +59,10 @@ struct vl_screen
  void *
  (*get_private)(struct vl_screen *vscreen);
+   void
+   (*set_back_texture_from_output)(struct vl_screen *vscreen, struct 
pipe_resource *buffer,
+ uint32_t width, uint32_t height);
+
  struct pipe_screen *pscreen;
  struct pipe_loader_device *dev;
   };
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c 
b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
index 2929928..9739be4 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
@@ -31,6 +31,7 @@
   #include 
   #include 
   #include 
+#include 
   #include "loader.h"
@@ -56,7 +57,9 @@ struct vl_dri3_buffer
  struct xshmfence *shm_fence;
  bool busy;
+   bool is_external_texture;
  uint32_t width, height, pitch;
+   uint32_t clip_width, clip_height;
   };
   struct vl_dri3_screen
@@ -71,6 +74,9 @@ struct vl_dri3_screen
  xcb_special_event_t *special_event;
  struct pipe_context *pipe;
+   struct pipe_resource *output_texture;
+   uint32_t output_texture_width;
+   uint32_t output_texture_height;

Are these clipping sizes?

Yes indeed, may be something like output_texture_clip_width would be more 
appropriate


I think the clipping size is nothing to do with the output texture size.

Is it possible just combine it with below and make it simple.

+   uint32_t clip_width, clip_height;



  struct vl_dri3_buffer *back_buffers[BACK_BUFFER_NUM];
  int cur_back;
@@ -105,7 +111,8 @@ dri3_free_back_buffer(struct vl_dri3_screen *scrn,
  xcb_free_pixmap(scrn->conn, buffer->pixmap);
  xcb_sync_destroy_fence(scrn->conn, buffer->sync_fence);
  xshmfence_unmap_shm(buffer->shm_fence);
-   pipe_resource_reference(>texture, NULL);
+   if (!buffer->is_external_texture)
+  pipe_resource_reference(>texture, NULL);
  if (buffer->linear_texture)
  pipe_resource_reference(>linear_texture, NULL);
  FREE(buffer);
@@ -236,14 +243,23 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
  templ.format = PIPE_FORMAT_B8G8R8X8_UNORM;
  templ.target = PIPE_TEXTURE_2D;
  templ.last_level = 0;
-   templ.width0 = scrn->width;
-   templ.height0 = scrn->height;
+   if (scrn->output_texture) {
+  buffer->clip_width = (scrn->output_texture_width) ? scrn->output_texture_width 
: scrn->output_texture->width0;
+  buffer->clip_height = (scrn->output_texture_height) ? 
scrn->output_texture_height : scrn->output_texture->height0;

Can you wrap these lines and many new added lines below?
Normally make a line less than ~80 chars, and make lines from above and
below balanced.
I think the whole purpose of this is to make more readable and looks nicer.


buffer->clip_width = (scrn->output_texture_width) ? 
scrn->output_texture_width
 : scrn->output_texture->width0;

Is something like this fine?


I think looks better if move the ":' up, or maybe 3 lines.
Please fix the same way for the similar long lines below.

Regards,
Leo



+  templ.width0 = scrn->output_texture->width0;
+  templ.height0 = scrn->output_texture->height0;
+   } else {
+   templ.width0 = scrn->width;
+   templ.height0 = scrn->height;
+   buffer->clip_width = scrn->width;
+   buffer->clip_height = scrn->height;
+   }
  templ.depth0 = 1;
  templ.array_size = 1;
  if (scrn->is_different_gpu) {
-  buffer->texture = scrn->base.pscreen->resource_create(scrn->base.pscreen,
-);
+  buffer->texture = (scrn->output_texture) ? scrn->output_texture : 
scrn->base.pscreen->resource_create(scrn->base.pscreen,
+   
 );
 if (!buffer->texture)
goto unmap_shm;
@@ -257,8 +273,8 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
goto no_linear_texture;
  } else {
 

Re: [Mesa-dev] [PATCH 1/3] vl/dri3: use external texture as back buffers(v2)

[Nayan Deshmukh]

On Fri, Nov 04, 2016 at 12:20:51PM -0400, Leo Liu wrote:
> Hi Nayan,
> 
> With this patch, the resizing corruption is fixed, thanks for that.
> 
> Still a few comments below.
> 
> 
> On 11/04/2016 03:08 AM, Nayan Deshmukh wrote:
> > dri3 allows us to send handle of a texture directly to X
> > so this patch allows a state tracker to directly send its
> > texture to X to be used as back buffer and avoids extra
> > copying
> > 
> > v2: use clip width/height to display a portion of the surface
> > 
> > Suggested-by: Leo Liu 
> > Signed-off-by: Nayan Deshmukh 
> > ---
> >   src/gallium/auxiliary/vl/vl_winsys.h  |   4 ++
> >   src/gallium/auxiliary/vl/vl_winsys_dri3.c | 109 
> > ++
> >   2 files changed, 99 insertions(+), 14 deletions(-)
> > 
> > diff --git a/src/gallium/auxiliary/vl/vl_winsys.h 
> > b/src/gallium/auxiliary/vl/vl_winsys.h
> > index 26db9f2..908e9c6 100644
> > --- a/src/gallium/auxiliary/vl/vl_winsys.h
> > +++ b/src/gallium/auxiliary/vl/vl_winsys.h
> > @@ -59,6 +59,10 @@ struct vl_screen
> >  void *
> >  (*get_private)(struct vl_screen *vscreen);
> > +   void
> > +   (*set_back_texture_from_output)(struct vl_screen *vscreen, struct 
> > pipe_resource *buffer,
> > + uint32_t width, uint32_t height);
> > +
> >  struct pipe_screen *pscreen;
> >  struct pipe_loader_device *dev;
> >   };
> > diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c 
> > b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
> > index 2929928..9739be4 100644
> > --- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
> > +++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
> > @@ -31,6 +31,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   #include "loader.h"
> > @@ -56,7 +57,9 @@ struct vl_dri3_buffer
> >  struct xshmfence *shm_fence;
> >  bool busy;
> > +   bool is_external_texture;
> >  uint32_t width, height, pitch;
> > +   uint32_t clip_width, clip_height;
> >   };
> >   struct vl_dri3_screen
> > @@ -71,6 +74,9 @@ struct vl_dri3_screen
> >  xcb_special_event_t *special_event;
> >  struct pipe_context *pipe;
> > +   struct pipe_resource *output_texture;
> > +   uint32_t output_texture_width;
> > +   uint32_t output_texture_height;
> 
> Are these clipping sizes?
Yes indeed, may be something like output_texture_clip_width would be more 
appropriate
> 
> >  struct vl_dri3_buffer *back_buffers[BACK_BUFFER_NUM];
> >  int cur_back;
> > @@ -105,7 +111,8 @@ dri3_free_back_buffer(struct vl_dri3_screen *scrn,
> >  xcb_free_pixmap(scrn->conn, buffer->pixmap);
> >  xcb_sync_destroy_fence(scrn->conn, buffer->sync_fence);
> >  xshmfence_unmap_shm(buffer->shm_fence);
> > -   pipe_resource_reference(>texture, NULL);
> > +   if (!buffer->is_external_texture)
> > +  pipe_resource_reference(>texture, NULL);
> >  if (buffer->linear_texture)
> >  pipe_resource_reference(>linear_texture, NULL);
> >  FREE(buffer);
> > @@ -236,14 +243,23 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
> >  templ.format = PIPE_FORMAT_B8G8R8X8_UNORM;
> >  templ.target = PIPE_TEXTURE_2D;
> >  templ.last_level = 0;
> > -   templ.width0 = scrn->width;
> > -   templ.height0 = scrn->height;
> > +   if (scrn->output_texture) {
> > +  buffer->clip_width = (scrn->output_texture_width) ? 
> > scrn->output_texture_width : scrn->output_texture->width0;
> > +  buffer->clip_height = (scrn->output_texture_height) ? 
> > scrn->output_texture_height : scrn->output_texture->height0;
> 
> Can you wrap these lines and many new added lines below?
> Normally make a line less than ~80 chars, and make lines from above and
> below balanced.
> I think the whole purpose of this is to make more readable and looks nicer.
> 
buffer->clip_width = (scrn->output_texture_width) ? 
scrn->output_texture_width
 : scrn->output_texture->width0;

Is something like this fine?
> > +  templ.width0 = scrn->output_texture->width0;
> > +  templ.height0 = scrn->output_texture->height0;
> > +   } else {
> > +   templ.width0 = scrn->width;
> > +   templ.height0 = scrn->height;
> > +   buffer->clip_width = scrn->width;
> > +   buffer->clip_height = scrn->height;
> > +   }
> >  templ.depth0 = 1;
> >  templ.array_size = 1;
> >  if (scrn->is_different_gpu) {
> > -  buffer->texture = 
> > scrn->base.pscreen->resource_create(scrn->base.pscreen,
> > -);
> > +  buffer->texture = (scrn->output_texture) ? scrn->output_texture : 
> > scrn->base.pscreen->resource_create(scrn->base.pscreen,
> > +   
> >  );
> > if (!buffer->texture)
> >goto unmap_shm;
> > @@ -257,8 +273,8 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
> >goto 

Re: [Mesa-dev] [PATCH 1/3] vl/dri3: use external texture as back buffers(v2)

[Leo Liu]

Hi Nayan,

With this patch, the resizing corruption is fixed, thanks for that.

Still a few comments below.


On 11/04/2016 03:08 AM, Nayan Deshmukh wrote:

dri3 allows us to send handle of a texture directly to X
so this patch allows a state tracker to directly send its
texture to X to be used as back buffer and avoids extra
copying

v2: use clip width/height to display a portion of the surface

Suggested-by: Leo Liu 
Signed-off-by: Nayan Deshmukh 
---
  src/gallium/auxiliary/vl/vl_winsys.h  |   4 ++
  src/gallium/auxiliary/vl/vl_winsys_dri3.c | 109 ++
  2 files changed, 99 insertions(+), 14 deletions(-)

diff --git a/src/gallium/auxiliary/vl/vl_winsys.h 
b/src/gallium/auxiliary/vl/vl_winsys.h
index 26db9f2..908e9c6 100644
--- a/src/gallium/auxiliary/vl/vl_winsys.h
+++ b/src/gallium/auxiliary/vl/vl_winsys.h
@@ -59,6 +59,10 @@ struct vl_screen
 void *
 (*get_private)(struct vl_screen *vscreen);
  
+   void

+   (*set_back_texture_from_output)(struct vl_screen *vscreen, struct 
pipe_resource *buffer,
+ uint32_t width, uint32_t height);
+
 struct pipe_screen *pscreen;
 struct pipe_loader_device *dev;
  };
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c 
b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
index 2929928..9739be4 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
@@ -31,6 +31,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include "loader.h"
  
@@ -56,7 +57,9 @@ struct vl_dri3_buffer

 struct xshmfence *shm_fence;
  
 bool busy;

+   bool is_external_texture;
 uint32_t width, height, pitch;
+   uint32_t clip_width, clip_height;
  };
  
  struct vl_dri3_screen

@@ -71,6 +74,9 @@ struct vl_dri3_screen
 xcb_special_event_t *special_event;
  
 struct pipe_context *pipe;

+   struct pipe_resource *output_texture;
+   uint32_t output_texture_width;
+   uint32_t output_texture_height;


Are these clipping sizes?

  
 struct vl_dri3_buffer *back_buffers[BACK_BUFFER_NUM];

 int cur_back;
@@ -105,7 +111,8 @@ dri3_free_back_buffer(struct vl_dri3_screen *scrn,
 xcb_free_pixmap(scrn->conn, buffer->pixmap);
 xcb_sync_destroy_fence(scrn->conn, buffer->sync_fence);
 xshmfence_unmap_shm(buffer->shm_fence);
-   pipe_resource_reference(>texture, NULL);
+   if (!buffer->is_external_texture)
+  pipe_resource_reference(>texture, NULL);
 if (buffer->linear_texture)
 pipe_resource_reference(>linear_texture, NULL);
 FREE(buffer);
@@ -236,14 +243,23 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
 templ.format = PIPE_FORMAT_B8G8R8X8_UNORM;
 templ.target = PIPE_TEXTURE_2D;
 templ.last_level = 0;
-   templ.width0 = scrn->width;
-   templ.height0 = scrn->height;
+   if (scrn->output_texture) {
+  buffer->clip_width = (scrn->output_texture_width) ? scrn->output_texture_width 
: scrn->output_texture->width0;
+  buffer->clip_height = (scrn->output_texture_height) ? 
scrn->output_texture_height : scrn->output_texture->height0;


Can you wrap these lines and many new added lines below?
Normally make a line less than ~80 chars, and make lines from above and 
below balanced.

I think the whole purpose of this is to make more readable and looks nicer.


+  templ.width0 = scrn->output_texture->width0;
+  templ.height0 = scrn->output_texture->height0;
+   } else {
+   templ.width0 = scrn->width;
+   templ.height0 = scrn->height;
+   buffer->clip_width = scrn->width;
+   buffer->clip_height = scrn->height;
+   }
 templ.depth0 = 1;
 templ.array_size = 1;
  
 if (scrn->is_different_gpu) {

-  buffer->texture = scrn->base.pscreen->resource_create(scrn->base.pscreen,
-);
+  buffer->texture = (scrn->output_texture) ? scrn->output_texture : 
scrn->base.pscreen->resource_create(scrn->base.pscreen,
+   
 );
if (!buffer->texture)
   goto unmap_shm;
  
@@ -257,8 +273,8 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)

   goto no_linear_texture;
 } else {
templ.bind |= PIPE_BIND_SCANOUT | PIPE_BIND_SHARED;
-  buffer->texture = scrn->base.pscreen->resource_create(scrn->base.pscreen,
-);
+  buffer->texture = (scrn->output_texture) ? scrn->output_texture : 
scrn->base.pscreen->resource_create(scrn->base.pscreen,
+   
 );
if (!buffer->texture)
   goto unmap_shm;
pixmap_buffer_texture = buffer->texture;
@@ -271,11 +287,15 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
 usage);
 buffer_fd = 

[Mesa-dev] [PATCH 1/3] vl/dri3: use external texture as back buffers(v2)

[Nayan Deshmukh]

dri3 allows us to send handle of a texture directly to X
so this patch allows a state tracker to directly send its
texture to X to be used as back buffer and avoids extra
copying

v2: use clip width/height to display a portion of the surface

Suggested-by: Leo Liu 
Signed-off-by: Nayan Deshmukh 
---
 src/gallium/auxiliary/vl/vl_winsys.h  |   4 ++
 src/gallium/auxiliary/vl/vl_winsys_dri3.c | 109 ++
 2 files changed, 99 insertions(+), 14 deletions(-)

diff --git a/src/gallium/auxiliary/vl/vl_winsys.h 
b/src/gallium/auxiliary/vl/vl_winsys.h
index 26db9f2..908e9c6 100644
--- a/src/gallium/auxiliary/vl/vl_winsys.h
+++ b/src/gallium/auxiliary/vl/vl_winsys.h
@@ -59,6 +59,10 @@ struct vl_screen
void *
(*get_private)(struct vl_screen *vscreen);
 
+   void
+   (*set_back_texture_from_output)(struct vl_screen *vscreen, struct 
pipe_resource *buffer,
+ uint32_t width, uint32_t height);
+
struct pipe_screen *pscreen;
struct pipe_loader_device *dev;
 };
diff --git a/src/gallium/auxiliary/vl/vl_winsys_dri3.c 
b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
index 2929928..9739be4 100644
--- a/src/gallium/auxiliary/vl/vl_winsys_dri3.c
+++ b/src/gallium/auxiliary/vl/vl_winsys_dri3.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "loader.h"
 
@@ -56,7 +57,9 @@ struct vl_dri3_buffer
struct xshmfence *shm_fence;
 
bool busy;
+   bool is_external_texture;
uint32_t width, height, pitch;
+   uint32_t clip_width, clip_height;
 };
 
 struct vl_dri3_screen
@@ -71,6 +74,9 @@ struct vl_dri3_screen
xcb_special_event_t *special_event;
 
struct pipe_context *pipe;
+   struct pipe_resource *output_texture;
+   uint32_t output_texture_width;
+   uint32_t output_texture_height;
 
struct vl_dri3_buffer *back_buffers[BACK_BUFFER_NUM];
int cur_back;
@@ -105,7 +111,8 @@ dri3_free_back_buffer(struct vl_dri3_screen *scrn,
xcb_free_pixmap(scrn->conn, buffer->pixmap);
xcb_sync_destroy_fence(scrn->conn, buffer->sync_fence);
xshmfence_unmap_shm(buffer->shm_fence);
-   pipe_resource_reference(>texture, NULL);
+   if (!buffer->is_external_texture)
+  pipe_resource_reference(>texture, NULL);
if (buffer->linear_texture)
pipe_resource_reference(>linear_texture, NULL);
FREE(buffer);
@@ -236,14 +243,23 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
templ.format = PIPE_FORMAT_B8G8R8X8_UNORM;
templ.target = PIPE_TEXTURE_2D;
templ.last_level = 0;
-   templ.width0 = scrn->width;
-   templ.height0 = scrn->height;
+   if (scrn->output_texture) {
+  buffer->clip_width = (scrn->output_texture_width) ? 
scrn->output_texture_width : scrn->output_texture->width0;
+  buffer->clip_height = (scrn->output_texture_height) ? 
scrn->output_texture_height : scrn->output_texture->height0;
+  templ.width0 = scrn->output_texture->width0;
+  templ.height0 = scrn->output_texture->height0;
+   } else {
+   templ.width0 = scrn->width;
+   templ.height0 = scrn->height;
+   buffer->clip_width = scrn->width;
+   buffer->clip_height = scrn->height;
+   }
templ.depth0 = 1;
templ.array_size = 1;
 
if (scrn->is_different_gpu) {
-  buffer->texture = scrn->base.pscreen->resource_create(scrn->base.pscreen,
-);
+  buffer->texture = (scrn->output_texture) ? scrn->output_texture : 
scrn->base.pscreen->resource_create(scrn->base.pscreen,
+   
 );
   if (!buffer->texture)
  goto unmap_shm;
 
@@ -257,8 +273,8 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
  goto no_linear_texture;
} else {
   templ.bind |= PIPE_BIND_SCANOUT | PIPE_BIND_SHARED;
-  buffer->texture = scrn->base.pscreen->resource_create(scrn->base.pscreen,
-);
+  buffer->texture = (scrn->output_texture) ? scrn->output_texture : 
scrn->base.pscreen->resource_create(scrn->base.pscreen,
+   
 );
   if (!buffer->texture)
  goto unmap_shm;
   pixmap_buffer_texture = buffer->texture;
@@ -271,11 +287,15 @@ dri3_alloc_back_buffer(struct vl_dri3_screen *scrn)
usage);
buffer_fd = whandle.handle;
buffer->pitch = whandle.stride;
+   buffer->width = templ.width0;
+   buffer->height = templ.height0;
+   buffer->is_external_texture = (scrn->output_texture) ? true : false;
+
xcb_dri3_pixmap_from_buffer(scrn->conn,
(pixmap = xcb_generate_id(scrn->conn)),
scrn->drawable,
0,
-   scrn->width, scrn->height, buffer->pitch,
+