[PATCH v3 0/10] 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). -- v3: wined3d: Don't skip ARB fragment program selection when fog state is dirty. 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: Don't setup FBO and draw buffers in wined3d_context_gl_apply_blit_state(). 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: Prepare/load the destination resource location in arbfp_blitter_blit(). wined3d: Prepare/load the destination resource location in ffp_blitter_blit(). 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 set by wined3d_context_gl_apply_blit_state() should matter for glReadPixels(), aside from the FBO read binding. So just do that instead. Some wined3d git archaeology suggests that the wined3d_context_gl_apply_blit_state() call was added right before Wine 1.2 to workaround various driver issues with glReadPixels() that in practice was erroneously affected by some GL states. If those kind of issues are still a thing, it might be necessary to reintroduce some limited state reset, possibly tied to a quirk. --- dlls/wined3d/surface.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c index fd8db9ce60d..d40f66021aa 100644 --- a/dlls/wined3d/surface.c +++ b/dlls/wined3d/surface.c @@ -371,16 +371,10 @@ 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 - { - wined3d_context_gl_apply_blit_state(context_gl, device); } /* Select the correct read buffer, and give some debug output. @@ -404,6 +398,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> Port of c0ab55700f681ace7ac13f9fdaa6075185fef35c to the FFP blitter. --- dlls/wined3d/texture.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 78b1f52221c..a01b0d631d7 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -6279,6 +6279,11 @@ static DWORD ffp_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_blit wined3d_texture_load(src_texture, context, FALSE); } + if (wined3d_texture_is_full_rect(dst_texture, dst_sub_resource_idx % dst_texture->level_count, dst_rect)) + wined3d_texture_prepare_location(dst_texture, dst_sub_resource_idx, context, dst_location); + else + wined3d_texture_load_location(dst_texture, dst_sub_resource_idx, context, dst_location); + wined3d_context_gl_apply_ffp_blit_state(context_gl, device); if (dst_location == WINED3D_LOCATION_DRAWABLE) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
From: Matteo Bruni <mbruni(a)codeweavers.com> Port of c0ab55700f681ace7ac13f9fdaa6075185fef35c to the ARB FP blitter. --- dlls/wined3d/arb_program_shader.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c index 592216051c2..feb0203dce3 100644 --- a/dlls/wined3d/arb_program_shader.c +++ b/dlls/wined3d/arb_program_shader.c @@ -7892,6 +7892,11 @@ static DWORD arbfp_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_bl wined3d_texture_load(src_texture, context, FALSE); } + if (wined3d_texture_is_full_rect(dst_texture, dst_sub_resource_idx % dst_texture->level_count, dst_rect)) + wined3d_texture_prepare_location(dst_texture, dst_sub_resource_idx, context, dst_location); + else + wined3d_texture_load_location(dst_texture, dst_sub_resource_idx, context, dst_location); + wined3d_context_gl_apply_ffp_blit_state(context_gl, device); if (dst_location == WINED3D_LOCATION_DRAWABLE) -- 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> All the callers (i.e. the blitters) also call context_gl_apply_texture_draw_state() that does that part of the setup anyway. --- dlls/wined3d/arb_program_shader.c | 3 +- dlls/wined3d/context_gl.c | 92 ++++++++++++------------------- dlls/wined3d/glsl_shader.c | 3 +- dlls/wined3d/texture.c | 3 +- 4 files changed, 37 insertions(+), 64 deletions(-) diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c index feb0203dce3..da276922479 100644 --- a/dlls/wined3d/arb_program_shader.c +++ b/dlls/wined3d/arb_program_shader.c @@ -7897,6 +7897,7 @@ static DWORD arbfp_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_bl else wined3d_texture_load_location(dst_texture, dst_sub_resource_idx, context, dst_location); + context_gl_apply_texture_draw_state(context_gl, dst_texture, dst_sub_resource_idx, dst_location); wined3d_context_gl_apply_ffp_blit_state(context_gl, device); if (dst_location == WINED3D_LOCATION_DRAWABLE) @@ -7906,8 +7907,6 @@ static DWORD arbfp_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_bl dst_rect = &d; } - context_gl_apply_texture_draw_state(context_gl, dst_texture, dst_sub_resource_idx, dst_location); - if (op == WINED3D_BLIT_OP_COLOR_BLIT_ALPHATEST) { const struct wined3d_format *fmt = src_texture->resource.format; diff --git a/dlls/wined3d/context_gl.c b/dlls/wined3d/context_gl.c index a0a21a0be20..6881d038b79 100644 --- a/dlls/wined3d/context_gl.c +++ b/dlls/wined3d/context_gl.c @@ -3241,50 +3241,12 @@ void wined3d_context_gl_apply_blit_state(struct wined3d_context_gl *context_gl, { 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); gl_info = context_gl->gl_info; - rt = context->current_rt.texture; - - if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) - { - if (context->render_offscreen) - { - 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 - rt_mask = 0; - } - 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); - } - } - else - { - rt_mask = wined3d_context_gl_generate_rt_mask_no_fbo(context_gl, &rt->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); - *cur_mask = rt_mask; - } - - 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); wined3d_context_gl_get_rt_size(context_gl, &rt_size); @@ -3643,38 +3605,52 @@ 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; + struct wined3d_context *context = &context_gl->c; + uint32_t rt_mask, *cur_mask; - if (wined3d_settings.offscreen_rendering_mode != ORM_FBO) - return; + TRACE("context_gl %p, texture %p, sub_resource_idx %u, location %s.\n", + context_gl, texture, sub_resource_idx, wined3d_debug_location(location)); - if (format->depth_size || format->stencil_size) + if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) { - wined3d_context_gl_apply_fbo_state_blit(context_gl, GL_DRAW_FRAMEBUFFER, - NULL, 0, &texture->resource, sub_resource_idx, location); - - buffer = GL_NONE; + 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); } - 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) + if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) + { + if (location != WINED3D_LOCATION_DRAWABLE) { - TRACE("Texture %p is onscreen.\n", texture); - buffer = wined3d_texture_get_gl_buffer(texture); + if (texture->resource.format->id == WINED3DFMT_NULL || format->depth_size || format->stencil_size) + rt_mask = 0; + else + rt_mask = 1; } else { - TRACE("Texture %p is offscreen.\n", texture); - buffer = GL_COLOR_ATTACHMENT0; + rt_mask = context_generate_rt_mask_from_resource(&texture->resource); } } + else + { + rt_mask = wined3d_context_gl_generate_rt_mask_no_fbo(context_gl, &texture->resource); + } - 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); + 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); + *cur_mask = rt_mask; + } + + 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. */ diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 7dc0de1b596..2002da68d2f 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -13194,6 +13194,7 @@ static DWORD glsl_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_bli else wined3d_texture_load_location(dst_texture, dst_sub_resource_idx, context, dst_location); + context_gl_apply_texture_draw_state(context_gl, dst_texture, dst_sub_resource_idx, dst_location); wined3d_context_gl_apply_blit_state(context_gl, device); if (dst_location == WINED3D_LOCATION_DRAWABLE) @@ -13203,8 +13204,6 @@ static DWORD glsl_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_bli dst_rect = &d; } - context_gl_apply_texture_draw_state(context_gl, dst_texture, dst_sub_resource_idx, dst_location); - if (op == WINED3D_BLIT_OP_COLOR_BLIT_ALPHATEST) { const struct wined3d_format *f = src_texture->resource.format; diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index a01b0d631d7..8711838f15c 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -6284,6 +6284,7 @@ static DWORD ffp_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_blit else wined3d_texture_load_location(dst_texture, dst_sub_resource_idx, context, dst_location); + context_gl_apply_texture_draw_state(context_gl, dst_texture, dst_sub_resource_idx, dst_location); wined3d_context_gl_apply_ffp_blit_state(context_gl, device); if (dst_location == WINED3D_LOCATION_DRAWABLE) @@ -6293,8 +6294,6 @@ static DWORD ffp_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_blit dst_rect = &r; } - context_gl_apply_texture_draw_state(context_gl, dst_texture, dst_sub_resource_idx, dst_location); - gl_info->gl_ops.gl.p_glEnable(src_texture_gl->target); checkGLcall("glEnable(target)"); -- 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 in principle; 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 (currently handled by 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 6881d038b79..bd5ac549075 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) { @@ -3614,10 +3614,10 @@ void context_gl_apply_texture_draw_state(struct wined3d_context_gl *context_gl, 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 d40f66021aa..32370e8999f 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 8711838f15c..f8dfb76ffc4 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -530,7 +530,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()"); @@ -631,7 +631,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 dd12453c66e..06343169206 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2328,7 +2328,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 32370e8999f..64c0de81fcd 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); } /* Select the correct read buffer, and give some debug output. -- 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 64c0de81fcd..2999efed741 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; @@ -381,10 +384,9 @@ void texture2d_read_from_framebuffer(struct wined3d_texture *texture, unsigned i resource, sub_resource_idx, NULL, 0, src_location); } - /* 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 f8dfb76ffc4..1596f2ff6a3 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -2972,18 +2972,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; @@ -3004,8 +3003,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
From: Matteo Bruni <mbruni(a)codeweavers.com> This is in the same vein as e106bbdd39a5f27bce028ed992033eb0a5635f60 and further fallout from 2ddb6b66a7cda0bf6aaddc0c6899e35cc92ceee9. --- dlls/wined3d/arb_program_shader.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/dlls/wined3d/arb_program_shader.c b/dlls/wined3d/arb_program_shader.c index da276922479..a1940ef6289 100644 --- a/dlls/wined3d/arb_program_shader.c +++ b/dlls/wined3d/arb_program_shader.c @@ -6559,28 +6559,6 @@ static void fragment_prog_arbfp(struct wined3d_context *context, const struct wi TRACE("context %p, state %p, state_id %#lx.\n", context, state, state_id); - if (isStateDirty(context, STATE_RENDER(WINED3D_RS_FOGENABLE))) - { - if (!use_pshader && device->shader_backend == &arb_program_shader_backend && context->last_was_pshader) - { - /* Reload fixed function constants since they collide with the - * pixel shader constants. */ - for (i = 0; i < WINED3D_MAX_TEXTURES; ++i) - { - set_bumpmat_arbfp(context, state, STATE_TEXTURESTAGE(i, WINED3D_TSS_BUMPENV_MAT00)); - state_tss_constant_arbfp(context, state, STATE_TEXTURESTAGE(i, WINED3D_TSS_CONSTANT)); - } - state_texfactor_arbfp(context, state, STATE_RENDER(WINED3D_RS_TEXTUREFACTOR)); - state_arb_specularenable(context, state, STATE_RENDER(WINED3D_RS_SPECULARENABLE)); - color_key_arbfp(context, state, STATE_COLOR_KEY); - } - else if (use_pshader) - { - context->shader_update_mask |= 1u << WINED3D_SHADER_TYPE_PIXEL; - } - return; - } - if (!use_pshader) { /* Find or create a shader implementing the fixed function pipeline -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3211
Pushed another update. Probably most notable is that I dropped the wined3d_context_gl_apply_blit_state_fb() change and instead removed the FBO setup part from wined3d_context_gl_apply_blit_state_fb() (which was effectively duplicated in context_gl_apply_texture_draw_state() as you noticed). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3211#note_40321
Zebediah Figura (@zfigura) commented about dlls/wined3d/texture.c:
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)
This should be a part of the previous commit, right? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3211#note_40373
Zebediah Figura (@zfigura) commented about dlls/wined3d/surface.c:
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))
Is this directly related to this patch, or should it be its own? As far as I can tell it's the latter, but maybe I'm missing something. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3211#note_40374
Zebediah Figura (@zfigura) commented about dlls/wined3d/texture.c:
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,
Whitespace error here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3211#note_40375
Thanks, this looks great now. I just have a couple comments related to patch splitting. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3211#note_40376
On Wed Jul 26 18:44:18 2023 +0000, Zebediah Figura wrote:
This should be a part of the previous commit, right? I think so. I guess some rebase gone wrong.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3211#note_44419
On Wed Jul 26 18:44:19 2023 +0000, Zebediah Figura wrote:
Is this directly related to this patch, or should it be its own? As far as I can tell it's the latter, but maybe I'm missing something. It should probably be its own patch, agreed. I feel like this hunk was always part of this patch (i.e. not a rebase accident like the other one) but I can't think of a good reason why it should be here.
I'll move it out. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3211#note_44420
participants (3)
-
Matteo Bruni -
Matteo Bruni (＠Mystral)
-
Zebediah Figura (＠zfigura)