[PATCH v2 0/7] MR3211: wined3d: Don't force going through a texture when downloading from renderbuffers.
It can be unnecessary at best and unsupported at worst (e.g. no ARB_texture_multisample). -- v2: wined3d: Don't force going through a texture when downloading from renderbuffers. wined3d: Handle depth textures in texture2d_read_from_framebuffer(). wined3d: Rename wined3d_context_gl_apply_fbo_state_blit() function. wined3d: Factor out a wined3d_context_gl_apply_blit_state_fb() function. wined3d: Don't call wined3d_texture_load() from wined3d_context_gl_apply_blit_state(). wined3d: Don't bind the FBO to GL_READ_FRAMEBUFFER in wined3d_context_gl_apply_blit_state(). wined3d: Don't call wined3d_context_gl_apply_blit_state() from texture2d_read_from_framebuffer(). https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
From: Matteo Bruni <mbruni(a)codeweavers.com> None of the GL states being set by wined3d_context_gl_apply_blit_state() should matter for glReadPixels(), aside from the FBO read binding and GL_FRAMEBUFFER_SRGB. So just do that instead. --- dlls/wined3d/surface.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index fd8db9ce60d..032485af0a5 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -371,16 +371,19 @@ void texture2d_read_from_framebuffer(struct wined3d_texture *texture, unsigned i context_gl = wined3d_context_gl(context); gl_info = context_gl->gl_info; - if (src_location != resource->draw_binding) + if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) { wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_READ_FRAMEBUFFER, resource, sub_resource_idx, NULL, 0, src_location); - wined3d_context_gl_check_fbo_status(context_gl, GL_READ_FRAMEBUFFER); - context_invalidate_state(context, STATE_FRAMEBUFFER); } - else + + if (src_location == resource->draw_binding) { - wined3d_context_gl_apply_blit_state(context_gl, device); + if (gl_info->supported[ARB_FRAMEBUFFER_SRGB]) + { + gl_info->gl_ops.gl.p_glDisable(GL_FRAMEBUFFER_SRGB); + context_invalidate_state(context, STATE_RENDER(WINED3D_RS_SRGBWRITEENABLE)); + } } /* Select the correct read buffer, and give some debug output. @@ -404,6 +407,8 @@ void texture2d_read_from_framebuffer(struct wined3d_texture *texture, unsigned i src_is_upside_down = FALSE; } checkGLcall("glReadBuffer"); + if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) + wined3d_context_gl_check_fbo_status(context_gl, GL_READ_FRAMEBUFFER); if (data.buffer_object) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/wined3d/context_gl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index a8c5160a5b3..c892c2adbeb 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -3257,7 +3257,7 @@ void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, { wined3d_texture_load(rt, context, FALSE); - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_FRAMEBUFFER, &rt->resource, + wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, &rt->resource, context->current_rt.sub_resource_idx, NULL, 0, rt->resource.draw_binding); if (rt->resource.format->id != WINED3DFMT_NULL) rt_mask = 1; @@ -3267,7 +3267,7 @@ void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, else { context_gl->current_fbo = NULL; - wined3d_context_gl_bind_fbo(context_gl, GL_FRAMEBUFFER, 0); + wined3d_context_gl_bind_fbo(context_gl, GL_DRAW_FRAMEBUFFER, 0); rt_mask = context_generate_rt_mask_from_resource(&rt->resource); } } @@ -3285,7 +3285,7 @@ void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, } if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) - wined3d_context_gl_check_fbo_status(context_gl, GL_FRAMEBUFFER); + wined3d_context_gl_check_fbo_status(context_gl, GL_DRAW_FRAMEBUFFER); context_invalidate_state(context, STATE_FRAMEBUFFER); wined3d_context_gl_get_rt_size(context_gl, &rt_size); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
From: Matteo Bruni <mbruni(a)codeweavers.com> It really isn't supposed to be its responsibility. All the callers of the function (which are blitters) take care of it already. --- dlls/wined3d/context_gl.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index c892c2adbeb..a0a21a0be20 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -3255,8 +3255,6 @@ void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, { if (context->render_offscreen) { - wined3d_texture_load(rt, context, FALSE); - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, &rt->resource, context->current_rt.sub_resource_idx, NULL, 0, rt->resource.draw_binding); if (rt->resource.format->id != WINED3DFMT_NULL) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/wined3d/context_gl.c | 90 +++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 50 deletions(-) diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index a0a21a0be20..5597ef9e281 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -3236,46 +3236,46 @@ static uint32_t wined3d_context_gl_generate_rt_mask_no_fbo(const struct wined3d_ return context_generate_rt_mask(wined3d_context_gl_get_offscreen_gl_buffer(context_gl)); } -/* Context activation is done by the caller. */ -void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, const struct wined3d_device *device) +static void wined3d_context_gl_apply_blit_state_fb(struct wined3d_context_gl *context_gl, + struct wined3d_texture *texture, unsigned int sub_resource_idx, DWORD location) { + const struct wined3d_format *format = texture->resource.format; struct wined3d_context *context = &context_gl->c; - const struct wined3d_gl_info *gl_info; uint32_t rt_mask, *cur_mask; - struct wined3d_texture *rt; - unsigned int sampler; - SIZE rt_size; - TRACE("Setting up context %p for blitting.\n", context); + TRACE("context_gl %p, texture %p, sub_resource_idx %u, location %s.\n", + context_gl, texture, sub_resource_idx, wined3d_debug_location(location)); - gl_info = context_gl->gl_info; - rt = context->current_rt.texture; + if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) + { + if (format->depth_size || format->stencil_size) + wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, NULL, + 0, &texture->resource, sub_resource_idx, location); + else + wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, &texture->resource, + sub_resource_idx, NULL, 0, location); + } if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) { - if (context->render_offscreen) + if (location != WINED3D_LOCATION_DRAWABLE) { - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, &rt->resource, - context->current_rt.sub_resource_idx, NULL, 0, rt->resource.draw_binding); - if (rt->resource.format->id != WINED3DFMT_NULL) - rt_mask = 1; - else + if (texture->resource.format->id == WINED3DFMT_NULL || format->depth_size || format->stencil_size) rt_mask = 0; + else + rt_mask = 1; } else { - context_gl->current_fbo = NULL; - wined3d_context_gl_bind_fbo(context_gl, GL_DRAW_FRAMEBUFFER, 0); - rt_mask = context_generate_rt_mask_from_resource(&rt->resource); + rt_mask = context_generate_rt_mask_from_resource(&texture->resource); } } else { - rt_mask = wined3d_context_gl_generate_rt_mask_no_fbo(context_gl, &rt->resource); + rt_mask = wined3d_context_gl_generate_rt_mask_no_fbo(context_gl, &texture->resource); } cur_mask = context_gl->current_fbo ? &context_gl->current_fbo->rt_mask : &context_gl->draw_buffers_mask; - if (rt_mask != *cur_mask) { wined3d_context_gl_apply_draw_buffers(context_gl, rt_mask); @@ -3284,7 +3284,26 @@ void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) wined3d_context_gl_check_fbo_status(context_gl, GL_DRAW_FRAMEBUFFER); + context_invalidate_state(context, STATE_FRAMEBUFFER); +} + +/* Context activation is done by the caller. */ +void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, const struct wined3d_device *device) +{ + struct wined3d_context *context = &context_gl->c; + const struct wined3d_gl_info *gl_info; + struct wined3d_texture *rt; + unsigned int sampler; + SIZE rt_size; + + TRACE("Setting up context %p for blitting.\n", context); + + gl_info = context_gl->gl_info; + rt = context->current_rt.texture; + + wined3d_context_gl_apply_blit_state_fb(context_gl, rt, context->current_rt.sub_resource_idx, + rt->resource.draw_binding); wined3d_context_gl_get_rt_size(context_gl, &rt_size); @@ -3642,39 +3661,10 @@ static uint32_t find_draw_buffers_mask(const struct wined3d_context_gl *context_ void context_gl_apply_texture_draw_state(struct wined3d_context_gl *context_gl, struct wined3d_texture *texture, unsigned int sub_resource_idx, unsigned int location) { - const struct wined3d_format *format = texture->resource.format; - GLenum buffer; - if (wined3d_settings.offscreen_rendering_mode != ORM_FBO) return; - if (format->depth_size || format->stencil_size) - { - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, - NULL, 0, &texture->resource, sub_resource_idx, location); - - buffer = GL_NONE; - } - else - { - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, - &texture->resource, sub_resource_idx, NULL, 0, location); - - if (location == WINED3D_LOCATION_DRAWABLE) - { - TRACE("Texture %p is onscreen.\n", texture); - buffer = wined3d_texture_get_gl_buffer(texture); - } - else - { - TRACE("Texture %p is offscreen.\n", texture); - buffer = GL_COLOR_ATTACHMENT0; - } - } - - wined3d_context_gl_set_draw_buffer(context_gl, buffer); - wined3d_context_gl_check_fbo_status(context_gl, GL_DRAW_FRAMEBUFFER); - context_invalidate_state(&context_gl->c, STATE_FRAMEBUFFER); + wined3d_context_gl_apply_blit_state_fb(context_gl, texture, sub_resource_idx, location); } /* Context activation is done by the caller. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
From: Matteo Bruni <mbruni(a)codeweavers.com> To wined3d_context_gl_apply_fbo_state_explicit(). It's not really related to blitting (i.e. it's used by blitting functions but that's about it), what it does is attaching specific textures to the FBO instead of the d3d render targets, which was the "original" use of FBOs in wined3d. BTW even that original use case (now in context_state_fb()) is not using render_targets[] directly anymore and we end up kind of abusing the blit_targets[] arrays in struct wined3d_context_gl. Maybe we could rename that array as well. --- dlls/wined3d/context_gl.c | 6 +++--- dlls/wined3d/surface.c | 2 +- dlls/wined3d/texture.c | 4 ++-- dlls/wined3d/wined3d_private.h | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index 5597ef9e281..4e6ce1d5dc2 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -849,7 +849,7 @@ static void wined3d_context_gl_apply_fbo_state(struct wined3d_context_gl *contex } /* Context activation is done by the caller. */ -void wined3d_context_gl_apply_fbo_state_blit(struct wined3d_context_gl *context_gl, GLenum target, +void wined3d_context_gl_apply_fbo_state_explicit(struct wined3d_context_gl *context_gl, GLenum target, struct wined3d_resource *rt, unsigned int rt_sub_resource_idx, struct wined3d_resource *ds, unsigned int ds_sub_resource_idx, uint32_t location) { @@ -3249,10 +3249,10 @@ static void wined3d_context_gl_apply_blit_state_fb(struct wined3d_context_gl *co if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) { if (format->depth_size || format->stencil_size) - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, NULL, + wined3d_context_gl_apply_fbo_state_explicit(context_gl, GL_DRAW_FRAMEBUFFER, NULL, 0, &texture->resource, sub_resource_idx, location); else - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, &texture->resource, + wined3d_context_gl_apply_fbo_state_explicit(context_gl, GL_DRAW_FRAMEBUFFER, &texture->resource, sub_resource_idx, NULL, 0, location); } diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index 032485af0a5..8d87900537d 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -373,7 +373,7 @@ void texture2d_read_from_framebuffer(struct wined3d_texture *texture, unsigned i if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) { - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_READ_FRAMEBUFFER, + wined3d_context_gl_apply_fbo_state_explicit(context_gl, GL_READ_FRAMEBUFFER, resource, sub_resource_idx, NULL, 0, src_location); } diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 32923349c6b..5d408f67199 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -529,7 +529,7 @@ static void texture2d_blt_fbo(struct wined3d_device *device, struct wined3d_cont buffer = GL_COLOR_ATTACHMENT0; } - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_READ_FRAMEBUFFER, + wined3d_context_gl_apply_fbo_state_explicit(context_gl, GL_READ_FRAMEBUFFER, &src_texture->resource, src_sub_resource_idx, NULL, 0, src_location); gl_info->gl_ops.gl.p_glReadBuffer(buffer); checkGLcall("glReadBuffer()"); @@ -630,7 +630,7 @@ static void texture2d_depth_blt_fbo(const struct wined3d_device *device, struct else wined3d_texture_prepare_location(dst_texture, dst_sub_resource_idx, context, dst_location); - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_READ_FRAMEBUFFER, NULL, 0, + wined3d_context_gl_apply_fbo_state_explicit(context_gl, GL_READ_FRAMEBUFFER, NULL, 0, &src_texture->resource, src_sub_resource_idx, src_location); wined3d_context_gl_check_fbo_status(context_gl, GL_READ_FRAMEBUFFER); diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index aedfc3f789f..4917803e8dc 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2331,7 +2331,7 @@ void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, const struct wined3d_device *device) DECLSPEC_HIDDEN; BOOL wined3d_context_gl_apply_clear_state(struct wined3d_context_gl *context_gl, const struct wined3d_state *state, unsigned int rt_count, const struct wined3d_fb_state *fb) DECLSPEC_HIDDEN; -void wined3d_context_gl_apply_fbo_state_blit(struct wined3d_context_gl *context_gl, GLenum target, +void wined3d_context_gl_apply_fbo_state_explicit(struct wined3d_context_gl *context_gl, GLenum target, struct wined3d_resource *rt, unsigned int rt_sub_resource_idx, struct wined3d_resource *ds, unsigned int ds_sub_resource_idx, uint32_t location) DECLSPEC_HIDDEN; void wined3d_context_gl_apply_ffp_blit_state(struct wined3d_context_gl *context_gl, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
From: Matteo Bruni <mbruni(a)codeweavers.com> --- dlls/wined3d/surface.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index 8d87900537d..fa91961296f 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -373,8 +373,12 @@ void texture2d_read_from_framebuffer(struct wined3d_texture *texture, unsigned i if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) { - wined3d_context_gl_apply_fbo_state_explicit(context_gl, GL_READ_FRAMEBUFFER, - resource, sub_resource_idx, NULL, 0, src_location); + if (resource->format->depth_size || resource->format->stencil_size) + wined3d_context_gl_apply_fbo_state_explicit(context_gl, GL_READ_FRAMEBUFFER, + NULL, 0, resource, sub_resource_idx, src_location); + else + wined3d_context_gl_apply_fbo_state_explicit(context_gl, GL_READ_FRAMEBUFFER, + resource, sub_resource_idx, NULL, 0, src_location); } if (src_location == resource->draw_binding) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
From: Matteo Bruni <mbruni(a)codeweavers.com> It can be unnecessary at best and unsupported at worst (e.g. no ARB_texture_multisample). --- dlls/wined3d/surface.c | 12 +++++++----- dlls/wined3d/texture.c | 16 +++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index fa91961296f..a5bb16c2239 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -358,13 +358,16 @@ void texture2d_read_from_framebuffer(struct wined3d_texture *texture, unsigned i uint8_t *offset; unsigned int i; + TRACE("texture %p, sub_resource_idx %u, context %p, src_location %s, dst_location %s.\n", + texture, sub_resource_idx, context, wined3d_debug_location(src_location), wined3d_debug_location(dst_location)); + /* dst_location was already prepared by the caller. */ wined3d_texture_get_bo_address(texture, sub_resource_idx, &data, dst_location); offset = data.addr; restore_texture = context->current_rt.texture; restore_idx = context->current_rt.sub_resource_idx; - if (restore_texture != texture || restore_idx != sub_resource_idx) + if (!wined3d_resource_is_offscreen(resource) && (restore_texture != texture || restore_idx != sub_resource_idx)) context = context_acquire(device, texture, sub_resource_idx); else restore_texture = NULL; @@ -390,10 +393,9 @@ void texture2d_read_from_framebuffer(struct wined3d_texture *texture, unsigned i } } - /* Select the correct read buffer, and give some debug output. - * There is no need to keep track of the current read buffer or reset it, - * every part of the code that reads sets the read buffer as desired. - */ + /* Select the correct read buffer, and give some debug output. There is no + * need to keep track of the current read buffer or reset it, every part + * of the code that reads pixels sets the read buffer as desired. */ if (src_location != WINED3D_LOCATION_DRAWABLE || wined3d_resource_is_offscreen(resource)) { /* Mapping the primary render target which is not on a swapchain. diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 5d408f67199..74eb6a7b700 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -2971,18 +2971,17 @@ static BOOL wined3d_texture_gl_load_sysmem(struct wined3d_texture_gl *texture_gl sub_resource = &texture_gl->t.sub_resources[sub_resource_idx]; /* We cannot download data from multisample textures directly. */ - if (wined3d_texture_gl_is_multisample_location(texture_gl, WINED3D_LOCATION_TEXTURE_RGB)) - { + if (wined3d_texture_gl_is_multisample_location(texture_gl, WINED3D_LOCATION_TEXTURE_RGB) + || sub_resource->locations & WINED3D_LOCATION_RB_MULTISAMPLE) wined3d_texture_load_location(&texture_gl->t, sub_resource_idx, &context_gl->c, WINED3D_LOCATION_RB_RESOLVED); - texture2d_read_from_framebuffer(&texture_gl->t, sub_resource_idx, &context_gl->c, + + if (sub_resource->locations & WINED3D_LOCATION_RB_RESOLVED) + { + texture2d_read_from_framebuffer(&texture_gl->t, sub_resource_idx, &context_gl->c, WINED3D_LOCATION_RB_RESOLVED, dst_location); return TRUE; } - if (sub_resource->locations & (WINED3D_LOCATION_RB_MULTISAMPLE | WINED3D_LOCATION_RB_RESOLVED)) - wined3d_texture_load_location(&texture_gl->t, sub_resource_idx, &context_gl->c, WINED3D_LOCATION_TEXTURE_RGB); - - /* Download the sub-resource to system memory. */ if (sub_resource->locations & (WINED3D_LOCATION_TEXTURE_RGB | WINED3D_LOCATION_TEXTURE_SRGB)) { unsigned int row_pitch, slice_pitch, level; @@ -3003,8 +3002,7 @@ static BOOL wined3d_texture_gl_load_sysmem(struct wined3d_texture_gl *texture_gl return TRUE; } - if (!(texture_gl->t.resource.bind_flags & WINED3D_BIND_DEPTH_STENCIL) - && (sub_resource->locations & WINED3D_LOCATION_DRAWABLE)) + if (sub_resource->locations & WINED3D_LOCATION_DRAWABLE) { texture2d_read_from_framebuffer(&texture_gl->t, sub_resource_idx, &context_gl->c, texture_gl->t.resource.draw_binding, dst_location); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
participants (2)
-
Matteo Bruni -
Matteo Bruni (＠Mystral)