[PATCH 1/6] wined3d: Respect the BO buffer offset in wined3d_context_gl_copy_bo_address().

[Zebediah Figura]

From: Henri Verbeet

Signed-off-by: Zebediah Figura 
---
 dlls/wined3d/context_gl.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c
index b327b7bba8e..3c57bcbe249 100644
--- a/dlls/wined3d/context_gl.c
+++ b/dlls/wined3d/context_gl.c
@@ -2828,7 +2828,8 @@ void wined3d_context_gl_copy_bo_address(struct wined3d_context_gl *context_gl,
             GL_EXTCALL(glBindBuffer(GL_COPY_READ_BUFFER, src_bo->id));
             GL_EXTCALL(glBindBuffer(GL_COPY_WRITE_BUFFER, dst_bo->id));
             GL_EXTCALL(glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER,
-                    (GLintptr)src->addr, (GLintptr)dst->addr, size));
+                    src_bo->b.buffer_offset + (GLintptr)src->addr,
+                    dst_bo->b.buffer_offset + (GLintptr)dst->addr, size));
             checkGLcall("direct buffer copy");

wined3d_context_gl_reference_bo(context_gl, src_bo);
@@ -2850,7 +2851,7 @@ void wined3d_context_gl_copy_bo_address(struct wined3d_context_gl *context_gl,
     else if (!dst_bo && src_bo)
     {
         wined3d_context_gl_bind_bo(context_gl, src_bo->binding, src_bo->id);
-        GL_EXTCALL(glGetBufferSubData(src_bo->binding, (GLintptr)src->addr, size, dst->addr));
+        GL_EXTCALL(glGetBufferSubData(src_bo->binding, src_bo->b.buffer_offset + (GLintptr)src->addr, size, dst->addr));
         checkGLcall("buffer download");

wined3d_context_gl_reference_bo(context_gl, src_bo);
@@ -2858,7 +2859,7 @@ void wined3d_context_gl_copy_bo_address(struct wined3d_context_gl *context_gl,
     else if (dst_bo && !src_bo)
     {
         wined3d_context_gl_bind_bo(context_gl, dst_bo->binding, dst_bo->id);
-        GL_EXTCALL(glBufferSubData(dst_bo->binding, (GLintptr)dst->addr, size, src->addr));
+        GL_EXTCALL(glBufferSubData(dst_bo->binding, dst_bo->b.buffer_offset + (GLintptr)dst->addr, size, src->addr));
         checkGLcall("buffer upload");

wined3d_context_gl_reference_bo(context_gl, dst_bo);

[PATCH 2/6] wined3d: Do not add the BO address offset when calling glFlushMappedBufferRange().

[Zebediah Figura]

From: Henri Verbeet

Fixes: c2f0ae50b4dd70aa9d364ec6a7dd728e65370d8b
Signed-off-by: Zebediah Figura 
---
 dlls/wined3d/context_gl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c
index 3c57bcbe249..0b0d594b4b1 100644
--- a/dlls/wined3d/context_gl.c
+++ b/dlls/wined3d/context_gl.c
@@ -2759,8 +2759,9 @@ static void flush_bo_ranges(struct wined3d_context_gl *context_gl, const struct
     {
         for (i = 0; i < range_count; ++i)
         {
-            GL_EXTCALL(glFlushMappedBufferRange(bo->binding,
-                    (UINT_PTR)data->addr + ranges[i].offset, ranges[i].size));
+            /* The offset passed to glFlushMappedBufferRange() is relative to
+             * the mapped range, so don't add data->addr in this case. */
+            GL_EXTCALL(glFlushMappedBufferRange(bo->binding, ranges[i].offset, ranges[i].size));
         }
     }
     else if (gl_info->supported[APPLE_FLUSH_BUFFER_RANGE])

[PATCH 2/6] wined3d: Do not add the BO address offset when calling glFlushMappedBufferRange().

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=102601

Your paranoid android.

=== debiant2 (build log) ===

Task: Patch failed to apply

=== debiant2 (build log) ===

Task: Patch failed to apply

[PATCH 2/6] wined3d: Do not add the BO address offset when calling glFlushMappedBufferRange().

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[PATCH 3/6] wined3d: Respect the BO buffer offset in flush_bo_ranges().

[Zebediah Figura]

From: Henri Verbeet

Signed-off-by: Zebediah Figura 
---
 dlls/wined3d/context_gl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c
index 0b0d594b4b1..b8bf41d8237 100644
--- a/dlls/wined3d/context_gl.c
+++ b/dlls/wined3d/context_gl.c
@@ -2769,7 +2769,7 @@ static void flush_bo_ranges(struct wined3d_context_gl *context_gl, const struct
         for (i = 0; i < range_count; ++i)
         {
             GL_EXTCALL(glFlushMappedBufferRangeAPPLE(bo->binding,
-                    (uintptr_t)data->addr + ranges[i].offset, ranges[i].size));
+                    bo->b.buffer_offset + (uintptr_t)data->addr + ranges[i].offset, ranges[i].size));
             checkGLcall("glFlushMappedBufferRangeAPPLE");
         }
     }

[PATCH 3/6] wined3d: Respect the BO buffer offset in flush_bo_ranges().

[Henri Verbeet]

On Wed, 24 Nov 2021 at 04:17, Zebediah Figura  wrote:
> @@ -2769,7 +2769,7 @@ static void flush_bo_ranges(struct wined3d_context_gl *context_gl, const struct
>          for (i = 0; i < range_count; ++i)
>          {
>              GL_EXTCALL(glFlushMappedBufferRangeAPPLE(bo->binding,
> -
>                (uintptr_t)data->addr + ranges[i].offset, ranges[i].size));
> -
>                bo->b.buffer_offset + (uintptr_t)data->addr + ranges[i].offset, ranges[i].size));

I'm not sure this change makes sense on its own. Whatever we do here
needs to be consistent with what we do in wined3d_bo_gl_map().

[PATCH 4/6] wined3d: Unbind the source BO after downloading in wined3d_context_gl_copy_bo_address().

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 dlls/wined3d/context_gl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c
index b8bf41d8237..24182c79726 100644
--- a/dlls/wined3d/context_gl.c
+++ b/dlls/wined3d/context_gl.c
@@ -2853,6 +2853,7 @@ void wined3d_context_gl_copy_bo_address(struct wined3d_context_gl *context_gl,
     {
         wined3d_context_gl_bind_bo(context_gl, src_bo->binding, src_bo->id);
         GL_EXTCALL(glGetBufferSubData(src_bo->binding, src_bo->b.buffer_offset + (GLintptr)src->addr, size, dst->addr));
+        wined3d_context_gl_bind_bo(context_gl, src_bo->binding, 0);
         checkGLcall("buffer download");

wined3d_context_gl_reference_bo(context_gl, src_bo);

[PATCH 4/6] wined3d: Unbind the source BO after downloading in wined3d_context_gl_copy_bo_address().

[Henri Verbeet]

On Wed, 24 Nov 2021 at 04:17, Zebediah Figura  wrote:
> @@ -2853,6 +2853,7 @@ void wined3d_context_gl_copy_bo_address(struct wined3d_context_gl *context_gl,
>      {
>          wined3d_context_gl_bind_bo(context_gl, src_bo->binding, src_bo->id);
>          GL_EXTCALL(glGetBufferSubData(src_bo->binding, src_bo->b.buffer_offset + (GLintptr)src->addr, size, dst->addr));
> -
>    wined3d_context_gl_bind_bo(context_gl, src_bo->binding, 0);
>    checkGLcall("buffer download");

Here and in the remainder of the series, does this fix a specific
issue? The general model we're following is to setup the correct
binding before calling GL commands using that bind point, and not
restoring any existing binding afterwards. (And note that in terms of
restoring bindings, binding 0 would be arbitrary; it's not
particularly likely to be the previous binding for a given bind
point.)

[PATCH 4/6] wined3d: Unbind the source BO after downloading in wined3d_context_gl_copy_bo_address().

[Zebediah Figura]

On 11/24/21 8:28 AM, Henri Verbeet wrote:
> On Wed, 24 Nov 2021 at 04:17, Zebediah Figura  wrote:
> > @@ -2853,6 +2853,7 @@ void wined3d_context_gl_copy_bo_address(struct wined3d_context_gl *context_gl,
> >       {
> >           wined3d_context_gl_bind_bo(context_gl, src_bo->binding, src_bo->id);
> >           GL_EXTCALL(glGetBufferSubData(src_bo->binding, src_bo->b.buffer_offset + (GLintptr)src->addr, size, dst->addr));
> > -
> >    wined3d_context_gl_bind_bo(context_gl, src_bo->binding, 0);
> >     checkGLcall("buffer download");
>
> Here and in the remainder of the series, does this fix a specific
> issue? The general model we're following is to setup the correct
> binding before calling GL commands using that bind point, and not
> restoring any existing binding afterwards. (And note that in terms of
> restoring bindings, binding 0 would be arbitrary; it's not
> particularly likely to be the previous binding for a given bind
> point.)

Okay, maybe I'm looking in the wrong place then, but that doesn't match 
what I've seen; everywhere else in this file seems to unbind the BO 
immediately after using it.

[PATCH 4/6] wined3d: Unbind the source BO after downloading in wined3d_context_gl_copy_bo_address().

[Henri Verbeet]

On Wed, 24 Nov 2021 at 17:44, Zebediah Figura  wrote:
> On 11/24/21 8:28 AM, Henri Verbeet wrote:
> > Here and in the remainder of the series, does this fix a specific
> > issue? The general model we're following is to setup the correct
> > binding before calling GL commands using that bind point, and not
> > restoring any existing binding afterwards. (And note that in terms of
> > restoring bindings, binding 0 would be arbitrary; it's not
> > particularly likely to be the previous binding for a given bind
> > point.)
>
> Okay, maybe I'm looking in the wrong place then, but that doesn't match
> what I've seen; everywhere else in this file seems to unbind the BO
> immediately after using it.

I think those are mostly historic, in one form or another. We should
probably get rid of them, but there's some potential for regressions,
because there may be existing code that depends on particular bindings
being 0. E.g., something like glTexImage2D() will use the
GL_PIXEL_UNPACK_BUFFER binding. I'm not confident that our handling of
GL_PIXEL_PACK_BUFFER/GL_PIXEL_UNPACK_BUFFER in particular is correct
everywhere.

[PATCH 4/6] wined3d: Unbind the source BO after downloading in wined3d_context_gl_copy_bo_address().

[Zebediah Figura]

On 11/24/21 11:12 AM, Henri Verbeet wrote:
> On Wed, 24 Nov 2021 at 17:44, Zebediah Figura  wrote:
> > On 11/24/21 8:28 AM, Henri Verbeet wrote:
> > > Here and in the remainder of the series, does this fix a specific
> > > issue? The general model we're following is to setup the correct
> > > binding before calling GL commands using that bind point, and not
> > > restoring any existing binding afterwards. (And note that in terms of
> > > restoring bindings, binding 0 would be arbitrary; it's not
> > > particularly likely to be the previous binding for a given bind
> > > point.)
> >
> > Okay, maybe I'm looking in the wrong place then, but that doesn't match
> > what I've seen; everywhere else in this file seems to unbind the BO
> > immediately after using it.
>
> I think those are mostly historic, in one form or another. We should
> probably get rid of them, but there's some potential for regressions,
> because there may be existing code that depends on particular bindings
> being 0. E.g., something like glTexImage2D() will use the
> GL_PIXEL_UNPACK_BUFFER binding. I'm not confident that our handling of
> GL_PIXEL_PACK_BUFFER/GL_PIXEL_UNPACK_BUFFER in particular is correct
> everywhere.

Indeed, that was the case that prompted these patches, although 
unfortunately I don't remember which function it was now.

I'll go through and try to fix this the "new" way, then.

[PATCH 5/6] wined3d: Unbind the destination BO after uploading in wined3d_context_gl_copy_bo_address().

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 dlls/wined3d/context_gl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c
index 24182c79726..71d6bbd1acf 100644
--- a/dlls/wined3d/context_gl.c
+++ b/dlls/wined3d/context_gl.c
@@ -2862,6 +2862,7 @@ void wined3d_context_gl_copy_bo_address(struct wined3d_context_gl *context_gl,
     {
         wined3d_context_gl_bind_bo(context_gl, dst_bo->binding, dst_bo->id);
         GL_EXTCALL(glBufferSubData(dst_bo->binding, dst_bo->b.buffer_offset + (GLintptr)dst->addr, size, src->addr));
+        wined3d_context_gl_bind_bo(context_gl, dst_bo->binding, 0);
         checkGLcall("buffer upload");

wined3d_context_gl_reference_bo(context_gl, dst_bo);

[PATCH 6/6] wined3d: Unbind the UAV BO after clearing in wined3d_unordered_access_view_gl_clear().

[Zebediah Figura]

Signed-off-by: Zebediah Figura 
---
 dlls/wined3d/view.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/wined3d/view.c b/dlls/wined3d/view.c
index 5905a4bcfb5..5b9a717a832 100644
--- a/dlls/wined3d/view.c
+++ b/dlls/wined3d/view.c
@@ -1599,6 +1599,7 @@ void wined3d_unordered_access_view_gl_clear(struct wined3d_unordered_access_view
     wined3d_context_gl_bind_bo(context_gl, bo_gl->binding, bo_gl->id);
     GL_EXTCALL(glClearBufferSubData(bo_gl->binding, format_gl->internal,
             bo_gl->b.buffer_offset + offset, size, format_gl->format, format_gl->type, clear_value));
+    wined3d_context_gl_bind_bo(context_gl, bo_gl->binding, 0);
     wined3d_context_gl_reference_bo(context_gl, bo_gl);
     checkGLcall("clear unordered access view");
 }

[Henri Verbeet]

Signed-off-by: Henri Verbeet