From 54ec363e88198fe0b866876d9dfaf964f3d03542 Mon Sep 17 00:00:00 2001 From: kd-11 Date: Sun, 28 Oct 2018 15:20:53 +0300 Subject: [PATCH] rsx: Critical pipeline fixes - Fix scissor and viewport binding behavior - Fixes recovery if empty scissor is specified and then 'fixed' later - Optimizes state binding a bit --- rpcs3/Emu/RSX/GL/GLGSRender.cpp | 30 ++++++++++--- rpcs3/Emu/RSX/GL/GLGSRender.h | 1 + rpcs3/Emu/RSX/GL/GLRenderTargets.cpp | 3 +- rpcs3/Emu/RSX/RSXThread.h | 5 ++- rpcs3/Emu/RSX/VK/VKGSRender.cpp | 64 +++++++++++++++++++--------- rpcs3/Emu/RSX/VK/VKGSRender.h | 5 +++ rpcs3/Emu/RSX/rsx_methods.cpp | 14 +++++- 7 files changed, 93 insertions(+), 29 deletions(-) diff --git a/rpcs3/Emu/RSX/GL/GLGSRender.cpp b/rpcs3/Emu/RSX/GL/GLGSRender.cpp index 1423f0df1a..5b339161ee 100644 --- a/rpcs3/Emu/RSX/GL/GLGSRender.cpp +++ b/rpcs3/Emu/RSX/GL/GLGSRender.cpp @@ -640,29 +640,49 @@ void GLGSRender::end() void GLGSRender::set_viewport() { - //NOTE: scale offset matrix already contains the viewport transformation + // NOTE: scale offset matrix already contains the viewport transformation const auto clip_width = rsx::apply_resolution_scale(rsx::method_registers.surface_clip_width(), true); const auto clip_height = rsx::apply_resolution_scale(rsx::method_registers.surface_clip_height(), true); glViewport(0, 0, clip_width, clip_height); +} + +void GLGSRender::set_scissor() +{ + if (m_graphics_state & rsx::pipeline_state::scissor_config_state_dirty) + { + // Optimistic that the new config will allow us to render + framebuffer_status_valid = true; + } + else if (!(m_graphics_state & rsx::pipeline_state::scissor_config_state_dirty)) + { + // Nothing to do + return; + } + + m_graphics_state &= ~(rsx::pipeline_state::scissor_config_state_dirty | rsx::pipeline_state::scissor_config_state_dirty); + + const auto clip_width = rsx::apply_resolution_scale(rsx::method_registers.surface_clip_width(), true); + const auto clip_height = rsx::apply_resolution_scale(rsx::method_registers.surface_clip_height(), true); u16 scissor_x = rsx::apply_resolution_scale(rsx::method_registers.scissor_origin_x(), false); u16 scissor_w = rsx::apply_resolution_scale(rsx::method_registers.scissor_width(), true); u16 scissor_y = rsx::apply_resolution_scale(rsx::method_registers.scissor_origin_y(), false); u16 scissor_h = rsx::apply_resolution_scale(rsx::method_registers.scissor_height(), true); - //Do not bother drawing anything if output is zero sized - //TODO: Clip scissor region + // Do not bother drawing anything if output is zero sized + // TODO: Clip scissor region if (scissor_x >= clip_width || scissor_y >= clip_height || scissor_w == 0 || scissor_h == 0) { if (!g_cfg.video.strict_rendering_mode) { + m_graphics_state |= rsx::pipeline_state::scissor_setup_invalid; framebuffer_status_valid = false; return; } } - //NOTE: window origin does not affect scissor region (probably only affects viewport matrix; already applied) - //See LIMBO [NPUB-30373] which uses shader window origin = top + // NOTE: window origin does not affect scissor region (probably only affects viewport matrix; already applied) + // See LIMBO [NPUB-30373] which uses shader window origin = top glScissor(scissor_x, scissor_y, scissor_w, scissor_h); glEnable(GL_SCISSOR_TEST); } diff --git a/rpcs3/Emu/RSX/GL/GLGSRender.h b/rpcs3/Emu/RSX/GL/GLGSRender.h index 2550934ca9..a65090d4db 100644 --- a/rpcs3/Emu/RSX/GL/GLGSRender.h +++ b/rpcs3/Emu/RSX/GL/GLGSRender.h @@ -371,6 +371,7 @@ private: public: void read_buffers(); void set_viewport(); + void set_scissor(); work_item& post_flush_request(u32 address, gl::texture_cache::thrashed_set& flush_data); diff --git a/rpcs3/Emu/RSX/GL/GLRenderTargets.cpp b/rpcs3/Emu/RSX/GL/GLRenderTargets.cpp index e0f13931c7..7ac39d196d 100644 --- a/rpcs3/Emu/RSX/GL/GLRenderTargets.cpp +++ b/rpcs3/Emu/RSX/GL/GLRenderTargets.cpp @@ -181,7 +181,7 @@ void GLGSRender::init_buffers(rsx::framebuffer_creation_context context, bool sk if (m_draw_fbo && !m_rtts_dirty) { - set_viewport(); + set_scissor(); return; } @@ -371,6 +371,7 @@ void GLGSRender::init_buffers(rsx::framebuffer_creation_context context, bool sk check_zcull_status(true); set_viewport(); + set_scissor(); m_gl_texture_cache.clear_ro_tex_invalidate_intr(); diff --git a/rpcs3/Emu/RSX/RSXThread.h b/rpcs3/Emu/RSX/RSXThread.h index 2813c31dbb..c8e94e732f 100644 --- a/rpcs3/Emu/RSX/RSXThread.h +++ b/rpcs3/Emu/RSX/RSXThread.h @@ -98,7 +98,10 @@ namespace rsx fragment_constants_dirty = 0x20, // Fragment constants changed framebuffer_reads_dirty = 0x40, // Framebuffer contents changed fragment_texture_state_dirty = 0x80, // Fragment texture parameters changed - vertex_texture_state_dirty = 0x80, // Fragment texture parameters changed + vertex_texture_state_dirty = 0x100, // Fragment texture parameters changed + scissor_config_state_dirty = 0x200, // Scissor region changed + + scissor_setup_invalid = 0x400, // Scissor configuration is broken invalidate_pipeline_bits = fragment_program_dirty | vertex_program_dirty, memory_barrier_bits = framebuffer_reads_dirty, diff --git a/rpcs3/Emu/RSX/VK/VKGSRender.cpp b/rpcs3/Emu/RSX/VK/VKGSRender.cpp index 0f86ce0b75..011392b216 100644 --- a/rpcs3/Emu/RSX/VK/VKGSRender.cpp +++ b/rpcs3/Emu/RSX/VK/VKGSRender.cpp @@ -1121,7 +1121,7 @@ void VKGSRender::update_draw_state() vkCmdSetDepthBounds(*m_current_command_buffer, 0.f, 1.f); } - set_viewport(); + bind_viewport(); //TODO: Set up other render-state parameters into the program pipeline @@ -1678,40 +1678,58 @@ void VKGSRender::set_viewport() { const auto clip_width = rsx::apply_resolution_scale(rsx::method_registers.surface_clip_width(), true); const auto clip_height = rsx::apply_resolution_scale(rsx::method_registers.surface_clip_height(), true); + + //NOTE: The scale_offset matrix already has viewport matrix factored in + m_viewport.x = 0; + m_viewport.y = 0; + m_viewport.width = clip_width; + m_viewport.height = clip_height; + m_viewport.minDepth = 0.f; + m_viewport.maxDepth = 1.f; +} + +void VKGSRender::set_scissor() +{ + if (m_graphics_state & rsx::pipeline_state::scissor_config_state_dirty) + { + // Optimistic that the new config will allow us to render + framebuffer_status_valid = true; + } + else if (!(m_graphics_state & rsx::pipeline_state::scissor_config_state_dirty)) + { + // Nothing to do + return; + } + + m_graphics_state &= ~(rsx::pipeline_state::scissor_config_state_dirty | rsx::pipeline_state::scissor_config_state_dirty); + u16 scissor_x = rsx::apply_resolution_scale(rsx::method_registers.scissor_origin_x(), false); u16 scissor_w = rsx::apply_resolution_scale(rsx::method_registers.scissor_width(), true); u16 scissor_y = rsx::apply_resolution_scale(rsx::method_registers.scissor_origin_y(), false); u16 scissor_h = rsx::apply_resolution_scale(rsx::method_registers.scissor_height(), true); - //NOTE: The scale_offset matrix already has viewport matrix factored in - VkViewport viewport = {}; - viewport.x = 0; - viewport.y = 0; - viewport.width = clip_width; - viewport.height = clip_height; - viewport.minDepth = 0.f; - viewport.maxDepth = 1.f; + m_scissor.extent.height = scissor_h; + m_scissor.extent.width = scissor_w; + m_scissor.offset.x = scissor_x; + m_scissor.offset.y = scissor_y; - vkCmdSetViewport(*m_current_command_buffer, 0, 1, &viewport); - - VkRect2D scissor = {}; - scissor.extent.height = scissor_h; - scissor.extent.width = scissor_w; - scissor.offset.x = scissor_x; - scissor.offset.y = scissor_y; - - vkCmdSetScissor(*m_current_command_buffer, 0, 1, &scissor); - - if (scissor_x >= viewport.width || scissor_y >= viewport.height || scissor_w == 0 || scissor_h == 0) + if (scissor_x >= m_viewport.width || scissor_y >= m_viewport.height || scissor_w == 0 || scissor_h == 0) { if (!g_cfg.video.strict_rendering_mode) { + m_graphics_state |= rsx::pipeline_state::scissor_setup_invalid; framebuffer_status_valid = false; return; } } } +void VKGSRender::bind_viewport() +{ + vkCmdSetViewport(*m_current_command_buffer, 0, 1, &m_viewport); + vkCmdSetScissor(*m_current_command_buffer, 0, 1, &m_scissor); +} + void VKGSRender::on_init_thread() { if (m_device == VK_NULL_HANDLE) @@ -2743,7 +2761,10 @@ void VKGSRender::prepare_rtts(rsx::framebuffer_creation_context context) } if (m_draw_fbo && !m_rtts_dirty) + { + set_scissor(); return; + } m_rtts_dirty = false; framebuffer_status_valid = false; @@ -2946,6 +2967,9 @@ void VKGSRender::prepare_rtts(rsx::framebuffer_creation_context context) m_draw_fbo.reset(new vk::framebuffer_holder(*m_device, current_render_pass, fbo_width, fbo_height, std::move(fbo_images))); } + set_viewport(); + set_scissor(); + check_zcull_status(true); } diff --git a/rpcs3/Emu/RSX/VK/VKGSRender.h b/rpcs3/Emu/RSX/VK/VKGSRender.h index ce567b3a43..b206d7072f 100644 --- a/rpcs3/Emu/RSX/VK/VKGSRender.h +++ b/rpcs3/Emu/RSX/VK/VKGSRender.h @@ -369,6 +369,9 @@ private: u32 m_client_width = 0; u32 m_client_height = 0; + VkViewport m_viewport{}; + VkRect2D m_scissor{}; + // Draw call stats u32 m_draw_calls = 0; @@ -432,6 +435,8 @@ public: void read_buffers(); void write_buffers(); void set_viewport(); + void set_scissor(); + void bind_viewport(); void sync_hint(rsx::FIFO_hint hint) override; diff --git a/rpcs3/Emu/RSX/rsx_methods.cpp b/rpcs3/Emu/RSX/rsx_methods.cpp index f9a61229c0..79289567ca 100644 --- a/rpcs3/Emu/RSX/rsx_methods.cpp +++ b/rpcs3/Emu/RSX/rsx_methods.cpp @@ -650,7 +650,7 @@ namespace rsx } } - void set_vertex_env_dirty_bit(thread* rsx, u32 reg, u32 arg) + void set_vertex_env_dirty_bit(thread* rsx, u32, u32 arg) { if (arg != method_registers.register_previous_value) { @@ -658,7 +658,7 @@ namespace rsx } } - void set_fragment_env_dirty_bit(thread* rsx, u32 reg, u32 arg) + void set_fragment_env_dirty_bit(thread* rsx, u32, u32 arg) { if (arg != method_registers.register_previous_value) { @@ -666,6 +666,14 @@ namespace rsx } } + void set_scissor_dirty_bit(thread* rsx, u32 reg, u32 arg) + { + if (arg != method_registers.register_previous_value) + { + rsx->m_graphics_state |= rsx::pipeline_state::scissor_config_state_dirty; + } + } + template struct set_texture_dirty_bit { @@ -2729,6 +2737,8 @@ namespace rsx bind(); bind(); bind(); + bind(); + bind(); bind_array(); bind_range(); bind_range();