[PATCH 1/6] wined3d: Respect the BO buffer offset in wined3d_context_gl_copy_bo_address().
From: Henri Verbeet <hverbeet(a)codeweavers.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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); -- 2.33.0
From: Henri Verbeet <hverbeet(a)codeweavers.com> Fixes: c2f0ae50b4dd70aa9d364ec6a7dd728e65370d8b Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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]) -- 2.33.0
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
Signed-off-by: Henri Verbeet <hverbeet(a)codeweavers.com>
From: Henri Verbeet <hverbeet(a)codeweavers.com> Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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"); } } -- 2.33.0
On Wed, 24 Nov 2021 at 04:17, Zebediah Figura <zfigura(a)codeweavers.com> 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().
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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); -- 2.33.0
On Wed, 24 Nov 2021 at 04:17, Zebediah Figura <zfigura(a)codeweavers.com> 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.)
On 11/24/21 8:28 AM, Henri Verbeet wrote:
On Wed, 24 Nov 2021 at 04:17, Zebediah Figura <zfigura(a)codeweavers.com> 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.
On Wed, 24 Nov 2021 at 17:44, Zebediah Figura <zfigura(a)codeweavers.com> 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.
On 11/24/21 11:12 AM, Henri Verbeet wrote:
On Wed, 24 Nov 2021 at 17:44, Zebediah Figura <zfigura(a)codeweavers.com> 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.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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); -- 2.33.0
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- 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"); } -- 2.33.0
participants (4)
-
Henri Verbeet -
Henri Verbeet -
Marvin -
Zebediah Figura