Re: [Mesa-dev] [PATCH] st/dri: Add an option to always request a front buffer from the loader

[Thomas Hellstrom]

On 09/18/2017 12:43 PM, Eric Anholt wrote:

Thomas Hellstrom  writes:


When an application decides to read from the front buffer of a window,
typically a fake front is created and initialized with the real front window
contents. However, if there was a window manager reparenting operation between
the last swapbuffer and the read the real front window contents would be
invalid. This hurts piglit applications that read from the front.

So add an option to always request a front buffer from the dri loader. This
makes the fake front initialization typically happen before any drawing- or
swapbuffer operations take place and, as a result, piglit results from tests
that read from the frontbuffer become robust with dri3.
While there is a memory usage overhead with this option enabled, the
performance overhead with dri3 should be minimal.

With dri2 the situation is different and require more work.

Instead of hacking the GL driver to have a special path for these tests,
could we make those piglit tests loop and try again when they get a
failure but have an expose event pending?  That should work for everyone
and handle the race properly.


Given that this is actually what's happening (Keithp thinks that's not 
really the case), I think
finding and changing all potentially affected piglit tests is really not 
worth the effort.


/Thomas


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/dri: Add an option to always request a front buffer from the loader

[Thomas Hellstrom]

On 09/18/2017 01:10 PM, Thomas Hellstrom wrote:

On 09/18/2017 12:43 PM, Eric Anholt wrote:

Thomas Hellstrom  writes:


When an application decides to read from the front buffer of a window,
typically a fake front is created and initialized with the real 
front window
contents. However, if there was a window manager reparenting 
operation between
the last swapbuffer and the read the real front window contents 
would be

invalid. This hurts piglit applications that read from the front.

So add an option to always request a front buffer from the dri 
loader. This
makes the fake front initialization typically happen before any 
drawing- or
swapbuffer operations take place and, as a result, piglit results 
from tests

that read from the frontbuffer become robust with dri3.
While there is a memory usage overhead with this option enabled, the
performance overhead with dri3 should be minimal.

With dri2 the situation is different and require more work.

Instead of hacking the GL driver to have a special path for these tests,
could we make those piglit tests loop and try again when they get a
failure but have an expose event pending?  That should work for everyone
and handle the race properly.


Given that this is actually what's happening (Keithp thinks that's not 
really the case), I think
finding and changing all potentially affected piglit tests is really 
not worth the effort.


Actually, that wouldn't bee to hard. A change in the piglit mainloop. 
I'll take a look at that.

Still confused as to what might be causing the initial problem, though.

Thomas



/Thomas




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/dri: Add an option to always request a front buffer from the loader

[Thomas Hellstrom]

Hi.

On 09/18/2017 12:18 PM, Keith Packard wrote:

Thomas Hellstrom  writes:


When an application decides to read from the front buffer of a window,
typically a fake front is created and initialized with the real front window
contents. However, if there was a window manager reparenting operation between
the last swapbuffer and the read the real front window contents would be
invalid. This hurts piglit applications that read from the front.

What do you mean by 'invalid'? On a running desktop, reparenting is
typically done before the window gets mapped, so there shouldn't be
*anything* done to the window by this operation. If you restart the
window manager, it will have to reparent all existing windows, which
looks like an unmap followed by a map, but those operations all do well
defined things to the window contents.


Hmm,

So what's happening (timing dependent) is that a window that's supposed 
to be all red after a swapbuffer instead has black borders at the bottom 
and to the right.
So my guess as to what was happening was the server executing the 
swapbuffer, then the window manager would reparent and draw window 
decorations.


But if that's not the case, any idea what might be causing this? It 
happens with both dri2 and dri3, more frequently with dri3.


/Thomas



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/dri: Add an option to always request a front buffer from the loader

[Eric Anholt]

Thomas Hellstrom  writes:

> When an application decides to read from the front buffer of a window,
> typically a fake front is created and initialized with the real front window
> contents. However, if there was a window manager reparenting operation between
> the last swapbuffer and the read the real front window contents would be
> invalid. This hurts piglit applications that read from the front.
>
> So add an option to always request a front buffer from the dri loader. This
> makes the fake front initialization typically happen before any drawing- or
> swapbuffer operations take place and, as a result, piglit results from tests
> that read from the frontbuffer become robust with dri3.
> While there is a memory usage overhead with this option enabled, the
> performance overhead with dri3 should be minimal.
>
> With dri2 the situation is different and require more work.

Instead of hacking the GL driver to have a special path for these tests,
could we make those piglit tests loop and try again when they get a
failure but have an expose event pending?  That should work for everyone
and handle the race properly.


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/dri: Add an option to always request a front buffer from the loader

[Keith Packard]

Thomas Hellstrom  writes:

> When an application decides to read from the front buffer of a window,
> typically a fake front is created and initialized with the real front window
> contents. However, if there was a window manager reparenting operation between
> the last swapbuffer and the read the real front window contents would be
> invalid. This hurts piglit applications that read from the front.

What do you mean by 'invalid'? On a running desktop, reparenting is
typically done before the window gets mapped, so there shouldn't be
*anything* done to the window by this operation. If you restart the
window manager, it will have to reparent all existing windows, which
looks like an unmap followed by a map, but those operations all do well
defined things to the window contents.

-- 
-keith


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] st/dri: Add an option to always request a front buffer from the loader

[Thomas Hellstrom]

When an application decides to read from the front buffer of a window,
typically a fake front is created and initialized with the real front window
contents. However, if there was a window manager reparenting operation between
the last swapbuffer and the read the real front window contents would be
invalid. This hurts piglit applications that read from the front.

So add an option to always request a front buffer from the dri loader. This
makes the fake front initialization typically happen before any drawing- or
swapbuffer operations take place and, as a result, piglit results from tests
that read from the frontbuffer become robust with dri3.
While there is a memory usage overhead with this option enabled, the
performance overhead with dri3 should be minimal.

With dri2 the situation is different and require more work.

Signed-off-by: Thomas Hellstrom 
---
 .../auxiliary/pipe-loader/driinfo_gallium.h|  1 +
 src/gallium/state_trackers/dri/dri2.c  | 22 +-
 src/gallium/state_trackers/dri/dri_screen.c|  2 ++
 src/gallium/state_trackers/dri/dri_screen.h|  1 +
 src/util/xmlpool/t_options.h   |  6 ++
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h 
b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
index 48a57c9..fe69570 100644
--- a/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
+++ b/src/gallium/auxiliary/pipe-loader/driinfo_gallium.h
@@ -32,4 +32,5 @@ DRI_CONF_SECTION_END
 DRI_CONF_SECTION_MISCELLANEOUS
DRI_CONF_ALWAYS_HAVE_DEPTH_BUFFER("false")
DRI_CONF_GLSL_ZERO_INIT("false")
+   DRI_CONF_ALWAYS_REQUEST_FRONT_BUFFER("false")
 DRI_CONF_SECTION_END
diff --git a/src/gallium/state_trackers/dri/dri2.c 
b/src/gallium/state_trackers/dri/dri2.c
index 1e8bb48..839 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -312,6 +312,8 @@ dri2_drawable_get_buffers(struct dri_drawable *drawable,
int num_buffers;
unsigned attachments[10];
unsigned num_attachments, i;
+   boolean have_front = FALSE;
+   int back_depth = 0;
 
assert(loader);
with_format = dri_with_format(drawable->sPriv);
@@ -319,8 +321,10 @@ dri2_drawable_get_buffers(struct dri_drawable *drawable,
num_attachments = 0;
 
/* for Xserver 1.6.0 (DRI2 version 1) we always need to ask for the front */
-   if (!with_format)
+   if (!with_format) {
   attachments[num_attachments++] = __DRI_BUFFER_FRONT_LEFT;
+  have_front = TRUE;
+   }
 
for (i = 0; i < *count; i++) {
   enum pipe_format format;
@@ -333,6 +337,7 @@ dri2_drawable_get_buffers(struct dri_drawable *drawable,
 
   switch (atts[i]) {
   case ST_ATTACHMENT_FRONT_LEFT:
+ have_front = TRUE;
  /* already added */
  if (!with_format)
 continue;
@@ -376,6 +381,17 @@ dri2_drawable_get_buffers(struct dri_drawable *drawable,
   if (with_format) {
  attachments[num_attachments++] = depth;
   }
+
+  if (att == __DRI_BUFFER_BACK_LEFT)
+ back_depth = depth;
+   }
+
+   if (dri_screen(drawable->sPriv)->always_request_front &&
+   !have_front &&
+   back_depth != 0) {
+  attachments[num_attachments++] = __DRI_BUFFER_FRONT_LEFT;
+  if (with_format)
+ attachments[num_attachments++] = back_depth;
}
 
if (with_format) {
@@ -449,6 +465,10 @@ dri_image_drawable_get_buffers(struct dri_drawable 
*drawable,
   }
}
 
+   if (dri_screen(sPriv)->always_request_front &&
+   image_format != __DRI_IMAGE_FORMAT_NONE)
+  buffer_mask |= __DRI_IMAGE_BUFFER_FRONT;
+
return (*sPriv->image.loader->getBuffers) (dPriv, image_format,
(uint32_t *) >base.stamp,
dPriv->loaderPrivate, buffer_mask,
diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
b/src/gallium/state_trackers/dri/dri_screen.c
index 1d9f441..eeb125d 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -485,6 +485,8 @@ dri_init_options(struct dri_screen *screen)
pipe_loader_load_options(screen->dev);
 
dri_fill_st_options(screen);
+   screen->always_request_front = driQueryOptionb(>dev->option_cache,
+  "always_request_front");
 }
 
 const __DRIconfig **
diff --git a/src/gallium/state_trackers/dri/dri_screen.h 
b/src/gallium/state_trackers/dri/dri_screen.h
index 677e945..5b076ce 100644
--- a/src/gallium/state_trackers/dri/dri_screen.h
+++ b/src/gallium/state_trackers/dri/dri_screen.h
@@ -59,6 +59,7 @@ struct dri_screen
__DRIscreen *sPriv;
boolean throttling_enabled;
int default_throttle_frames;
+   boolean always_request_front;
 
struct st_config_options options;
 
diff --git a/src/util/xmlpool/t_options.h b/src/util/xmlpool/t_options.h
index d3f31fc..2744024 100644
---