From cd8cb9cced5fd887feaebc8ae92eed499fa45a34 Mon Sep 17 00:00:00 2001 From: kd-11 Date: Thu, 24 Jun 2021 23:46:11 +0300 Subject: [PATCH] rsx: Don't leak data during partial clears - Partial clears either in active clear channels or scissor region must get barrier inserts to load previous data. - Fixes some incorrectly discarded data during clear where data in untouched/uninitialized channels is lost. --- rpcs3/Emu/RSX/GL/GLGSRender.cpp | 48 +++++++++------ rpcs3/Emu/RSX/VK/VKGSRender.cpp | 106 ++++++++++++++++---------------- 2 files changed, 81 insertions(+), 73 deletions(-) diff --git a/rpcs3/Emu/RSX/GL/GLGSRender.cpp b/rpcs3/Emu/RSX/GL/GLGSRender.cpp index 59ee60b945..ff0724f32e 100644 --- a/rpcs3/Emu/RSX/GL/GLGSRender.cpp +++ b/rpcs3/Emu/RSX/GL/GLGSRender.cpp @@ -511,11 +511,11 @@ void GLGSRender::clear_surface(u32 arg) GLbitfield mask = 0; gl::command_context cmd{ gl_state }; - const bool require_mem_load = - rsx::method_registers.scissor_origin_x() > 0 || - rsx::method_registers.scissor_origin_y() > 0 || - rsx::method_registers.scissor_width() < rsx::method_registers.surface_clip_width() || - rsx::method_registers.scissor_height() < rsx::method_registers.surface_clip_height(); + const bool full_frame = + rsx::method_registers.scissor_origin_x() == 0 && + rsx::method_registers.scissor_origin_y() == 0 && + rsx::method_registers.scissor_width() >= rsx::method_registers.surface_clip_width() && + rsx::method_registers.scissor_height() >= rsx::method_registers.surface_clip_height(); bool update_color = false, update_z = false; rsx::surface_depth_format2 surface_depth_format = rsx::method_registers.surface_depth_fmt(); @@ -543,32 +543,38 @@ void GLGSRender::clear_surface(u32 arg) mask |= GLenum(gl::buffers::stencil); } - if ((arg & 0x3) != 0x3 && !require_mem_load && ds->dirty()) + if ((arg & 0x3) != 0x3 || !full_frame) { ensure(mask); - // Only one aspect was cleared. Make sure to memory initialize the other before removing dirty flag - if (arg == 1) + if (ds->state_flags & rsx::surface_state_flags::erase_bkgnd && // Needs initialization + ds->old_contents.empty() && !g_cfg.video.read_depth_buffer) // No way to load data from memory, so no initialization given { - // Depth was cleared, initialize stencil - gl_state.stencil_mask(0xFF); - gl_state.clear_stencil(0xFF); - mask |= GLenum(gl::buffers::stencil); + // Only one aspect was cleared. Make sure to memory initialize the other before removing dirty flag + if (arg == 1) + { + // Depth was cleared, initialize stencil + gl_state.stencil_mask(0xFF); + gl_state.clear_stencil(0xFF); + mask |= GLenum(gl::buffers::stencil); + } + else + { + // Stencil was cleared, initialize depth + gl_state.depth_mask(GL_TRUE); + gl_state.clear_depth(1.f); + mask |= GLenum(gl::buffers::depth); + } } else { - // Stencil was cleared, initialize depth - gl_state.depth_mask(GL_TRUE); - gl_state.clear_depth(1.f); - mask |= GLenum(gl::buffers::depth); + ds->write_barrier(cmd); } } } if (mask) { - if (require_mem_load) ds->write_barrier(cmd); - // Memory has been initialized update_z = true; } @@ -620,7 +626,11 @@ void GLGSRender::clear_surface(u32 arg) count < m_rtts.m_bound_render_targets_config.second; ++count, ++index) { - if (require_mem_load) m_rtts.m_bound_render_targets[index].second->write_barrier(cmd); + if (!full_frame) + { + m_rtts.m_bound_render_targets[index].second->write_barrier(cmd); + } + gl_state.color_maski(count, colormask); } diff --git a/rpcs3/Emu/RSX/VK/VKGSRender.cpp b/rpcs3/Emu/RSX/VK/VKGSRender.cpp index 26f7f511db..240b2df2ab 100644 --- a/rpcs3/Emu/RSX/VK/VKGSRender.cpp +++ b/rpcs3/Emu/RSX/VK/VKGSRender.cpp @@ -1143,7 +1143,7 @@ void VKGSRender::clear_surface(u32 mask) std::tie(scissor_x, scissor_y, scissor_w, scissor_h) = rsx::clip_region(fb_width, fb_height, scissor_x, scissor_y, scissor_w, scissor_h, true); VkClearRect region = { { { scissor_x, scissor_y }, { scissor_w, scissor_h } }, 0, 1 }; - const bool require_mem_load = (scissor_w * scissor_h) < (fb_width * fb_height); + const bool full_frame = (scissor_w == fb_width && scissor_h == fb_height); bool update_color = false, update_z = false; auto surface_depth_format = rsx::method_registers.surface_depth_fmt(); @@ -1173,36 +1173,39 @@ void VKGSRender::clear_surface(u32 mask) if (ds->samples() > 1) { - if (!require_mem_load) ds->stencil_init_flags &= 0xFF; + if (full_frame) ds->stencil_init_flags &= 0xFF; ds->stencil_init_flags |= clear_stencil; } } + } - if ((mask & 0x3) != 0x3 && !require_mem_load && ds->state_flags & rsx::surface_state_flags::erase_bkgnd) + if ((depth_stencil_mask && depth_stencil_mask != ds->aspect()) || !full_frame) + { + // At least one aspect is not being cleared or the clear does not cover the full frame + // Steps to initialize memory are required + + if (ds->state_flags & rsx::surface_state_flags::erase_bkgnd && // Needs initialization + ds->old_contents.empty() && !g_cfg.video.read_depth_buffer) // No way to load data from memory, so no initialization given { - ensure(depth_stencil_mask); - - if (!g_cfg.video.read_depth_buffer) + // Only one aspect was cleared. Make sure to memory initialize the other before removing dirty flag + if (mask == 1) { - // Only one aspect was cleared. Make sure to memory initialize the other before removing dirty flag - if (mask == 1) - { - // Depth was cleared, initialize stencil - depth_stencil_clear_values.depthStencil.stencil = 0xFF; - depth_stencil_mask |= VK_IMAGE_ASPECT_STENCIL_BIT; - } - else - { - // Stencil was cleared, initialize depth - depth_stencil_clear_values.depthStencil.depth = 1.f; - depth_stencil_mask |= VK_IMAGE_ASPECT_DEPTH_BIT; - } + // Depth was cleared, initialize stencil + depth_stencil_clear_values.depthStencil.stencil = 0xFF; + depth_stencil_mask |= VK_IMAGE_ASPECT_STENCIL_BIT; } else { - ds->write_barrier(*m_current_command_buffer); + // Stencil was cleared, initialize depth + depth_stencil_clear_values.depthStencil.depth = 1.f; + depth_stencil_mask |= VK_IMAGE_ASPECT_DEPTH_BIT; } } + else + { + // Barrier required before any writes + ds->write_barrier(*m_current_command_buffer); + } } } @@ -1250,6 +1253,17 @@ void VKGSRender::clear_surface(u32 mask) if (colormask) { + if (!use_fast_clear || !full_frame) + { + // If we're not clobber all the memory, a barrier is required + for (u8 index = m_rtts.m_bound_render_targets_config.first, count = 0; + count < m_rtts.m_bound_render_targets_config.second; + ++count, ++index) + { + m_rtts.m_bound_render_targets[index].second->write_barrier(*m_current_command_buffer); + } + } + color_clear_values.color.float32[0] = static_cast(clear_r) / 255; color_clear_values.color.float32[1] = static_cast(clear_g) / 255; color_clear_values.color.float32[2] = static_cast(clear_b) / 255; @@ -1276,14 +1290,6 @@ void VKGSRender::clear_surface(u32 mask) attachment_clear_pass->run(*m_current_command_buffer, m_draw_fbo, region.rect, colormask, clear_color, get_render_pass()); } - for (u8 index = m_rtts.m_bound_render_targets_config.first, count = 0; - count < m_rtts.m_bound_render_targets_config.second; - ++count, ++index) - { - if (require_mem_load) - m_rtts.m_bound_render_targets[index].second->write_barrier(*m_current_command_buffer); - } - update_color = true; } } @@ -1291,36 +1297,28 @@ void VKGSRender::clear_surface(u32 mask) if (depth_stencil_mask) { - if (m_rtts.m_bound_depth_stencil.first) + if ((depth_stencil_mask & VK_IMAGE_ASPECT_STENCIL_BIT) && + rsx::method_registers.stencil_mask() != 0xff) { - if (require_mem_load) - { - m_rtts.m_bound_depth_stencil.second->write_barrier(*m_current_command_buffer); - } + // Partial stencil clear. Disables fast stencil clear + auto ds = std::get<1>(m_rtts.m_bound_depth_stencil); + auto key = vk::get_renderpass_key({ ds }); + auto renderpass = vk::get_renderpass(*m_device, key); - if ((depth_stencil_mask & VK_IMAGE_ASPECT_STENCIL_BIT) && - rsx::method_registers.stencil_mask() != 0xff) - { - // Partial stencil clear. Disables fast stencil clear - auto ds = std::get<1>(m_rtts.m_bound_depth_stencil); - auto key = vk::get_renderpass_key({ ds }); - auto renderpass = vk::get_renderpass(*m_device, key); + vk::get_overlay_pass()->run( + *m_current_command_buffer, ds, region.rect, + depth_stencil_clear_values.depthStencil.stencil, + rsx::method_registers.stencil_mask(), renderpass); - vk::get_overlay_pass()->run( - *m_current_command_buffer, ds, region.rect, - depth_stencil_clear_values.depthStencil.stencil, - rsx::method_registers.stencil_mask(), renderpass); - - depth_stencil_mask &= ~VK_IMAGE_ASPECT_STENCIL_BIT; - } - - if (depth_stencil_mask) - { - clear_descriptors.push_back({ static_cast(depth_stencil_mask), 0, depth_stencil_clear_values }); - } - - update_z = true; + depth_stencil_mask &= ~VK_IMAGE_ASPECT_STENCIL_BIT; } + + if (depth_stencil_mask) + { + clear_descriptors.push_back({ static_cast(depth_stencil_mask), 0, depth_stencil_clear_values }); + } + + update_z = true; } if (update_color || update_z)