From d6230bbad83d1961e31c88cc2da4f2a6eb1edc35 Mon Sep 17 00:00:00 2001 From: Filoppi Date: Mon, 18 Dec 2023 00:52:31 +0200 Subject: Video: Change the frame dumper to actually use the raw emulation output resolution, avoiding any scaling if possible. This should make comparisons much more reliable as pixels wouldn't be smushed together or stretched. --- Source/Core/VideoCommon/Present.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'Source/Core/VideoCommon/Present.cpp') diff --git a/Source/Core/VideoCommon/Present.cpp b/Source/Core/VideoCommon/Present.cpp index 0cc19d6e14..322685084d 100644 --- a/Source/Core/VideoCommon/Present.cpp +++ b/Source/Core/VideoCommon/Present.cpp @@ -214,14 +214,17 @@ void Presenter::ProcessFrameDumping(u64 ticks) const MathUtil::Rectangle target_rect; if (!g_ActiveConfig.bInternalResolutionFrameDumps && !g_gfx->IsHeadless()) { + // This is already scaled by "VIDEO_ENCODER_LCM" target_rect = GetTargetRectangle(); } else { - int width, height; - std::tie(width, height) = - CalculateOutputDimensions(m_xfb_rect.GetWidth(), m_xfb_rect.GetHeight()); - target_rect = MathUtil::Rectangle(0, 0, width, height); + target_rect = m_xfb_rect; + ASSERT(target_rect.top == 0 && target_rect.left == 0); + // Scale positively to make sure the least amount of information is lost. + // TODO: this should be added as black padding on the edges by the frame dumper + target_rect.right += VIDEO_ENCODER_LCM - (target_rect.GetWidth() % VIDEO_ENCODER_LCM); + target_rect.bottom += VIDEO_ENCODER_LCM - (target_rect.GetHeight() % VIDEO_ENCODER_LCM); } g_frame_dumper->DumpCurrentFrame(m_xfb_entry->texture.get(), m_xfb_rect, target_rect, ticks, -- cgit v1.2.3 From 1f34adf216ae4638bf6d420e95513b9e7c56b94f Mon Sep 17 00:00:00 2001 From: Filoppi Date: Fri, 23 Feb 2024 04:09:49 +0200 Subject: Video: move all padding added for frame dumping to a single function, which also avoids the output window from being resized randomly to be a multiple of 4 --- Source/Core/VideoCommon/Present.cpp | 51 +++++++++++++++---------------------- 1 file changed, 21 insertions(+), 30 deletions(-) (limited to 'Source/Core/VideoCommon/Present.cpp') diff --git a/Source/Core/VideoCommon/Present.cpp b/Source/Core/VideoCommon/Present.cpp index 322685084d..d338821dd8 100644 --- a/Source/Core/VideoCommon/Present.cpp +++ b/Source/Core/VideoCommon/Present.cpp @@ -213,19 +213,29 @@ void Presenter::ProcessFrameDumping(u64 ticks) const { MathUtil::Rectangle target_rect; if (!g_ActiveConfig.bInternalResolutionFrameDumps && !g_gfx->IsHeadless()) - { - // This is already scaled by "VIDEO_ENCODER_LCM" target_rect = GetTargetRectangle(); - } else - { target_rect = m_xfb_rect; - ASSERT(target_rect.top == 0 && target_rect.left == 0); - // Scale positively to make sure the least amount of information is lost. - // TODO: this should be added as black padding on the edges by the frame dumper - target_rect.right += VIDEO_ENCODER_LCM - (target_rect.GetWidth() % VIDEO_ENCODER_LCM); - target_rect.bottom += VIDEO_ENCODER_LCM - (target_rect.GetHeight() % VIDEO_ENCODER_LCM); - } + + int width = target_rect.GetWidth(); + int height = target_rect.GetHeight(); + + // Ensure divisibility by "VIDEO_ENCODER_LCM" to make it compatible with all the video + // encoders. Note that this is theoretically only necessary when recording videos and not + // screenshots. + // We always scale positively to make sure the least amount of information is lost. + // + // TODO: this should be added as black padding on the edges by the frame dumper. + if ((width % VIDEO_ENCODER_LCM) != 0) + width += VIDEO_ENCODER_LCM - (width % VIDEO_ENCODER_LCM); + if ((height % VIDEO_ENCODER_LCM) != 0) + height += VIDEO_ENCODER_LCM - (height % VIDEO_ENCODER_LCM); + + // Remove any black borders, there would be no point in including them in the recording + target_rect.left = 0; + target_rect.top = 0; + target_rect.right = width; + target_rect.bottom = height; g_frame_dumper->DumpCurrentFrame(m_xfb_entry->texture.get(), m_xfb_rect, target_rect, ticks, m_frame_count); @@ -607,18 +617,7 @@ void Presenter::UpdateDrawRectangle() int int_draw_width; int int_draw_height; - if (g_frame_dumper->IsFrameDumping()) - { - // ensure divisibility by "VIDEO_ENCODER_LCM" to make it compatible with all the video encoders. - // Note that this is theoretically only necessary when recording videos and not screenshots. - draw_width = - std::ceil(draw_width) - static_cast(std::ceil(draw_width)) % VIDEO_ENCODER_LCM; - draw_height = - std::ceil(draw_height) - static_cast(std::ceil(draw_height)) % VIDEO_ENCODER_LCM; - int_draw_width = static_cast(draw_width); - int_draw_height = static_cast(draw_height); - } - else if (g_ActiveConfig.aspect_mode != AspectMode::Raw || !m_xfb_entry) + if (g_ActiveConfig.aspect_mode != AspectMode::Raw || !m_xfb_entry) { // Find the best integer resolution: the closest aspect ratio with the least black bars. // This should have no influence if "AspectMode::Stretch" is active. @@ -703,14 +702,6 @@ std::tuple Presenter::CalculateOutputDimensions(int width, int height, height = static_cast(std::ceil(scaled_height)); } - if (g_frame_dumper->IsFrameDumping()) - { - // UpdateDrawRectangle() makes sure that the rendered image is divisible by "VIDEO_ENCODER_LCM" - // for video encoders, so do that here too to match it - width -= width % VIDEO_ENCODER_LCM; - height -= height % VIDEO_ENCODER_LCM; - } - return std::make_tuple(width, height); } -- cgit v1.2.3 From 72db62e17856265adefaaff6c7827989a2b362a4 Mon Sep 17 00:00:00 2001 From: Filoppi Date: Thu, 22 Feb 2024 02:11:31 +0200 Subject: Video: split frame dumping settings into 3 resolution dumping modes also polish aspect ratio related code for clarity --- Source/Core/VideoCommon/Present.cpp | 51 +++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 10 deletions(-) (limited to 'Source/Core/VideoCommon/Present.cpp') diff --git a/Source/Core/VideoCommon/Present.cpp b/Source/Core/VideoCommon/Present.cpp index d338821dd8..3a026bedd5 100644 --- a/Source/Core/VideoCommon/Present.cpp +++ b/Source/Core/VideoCommon/Present.cpp @@ -212,23 +212,49 @@ void Presenter::ProcessFrameDumping(u64 ticks) const if (g_frame_dumper->IsFrameDumping() && m_xfb_entry) { MathUtil::Rectangle target_rect; - if (!g_ActiveConfig.bInternalResolutionFrameDumps && !g_gfx->IsHeadless()) - target_rect = GetTargetRectangle(); - else + switch (g_ActiveConfig.frame_dumps_resolution_type) + { + default: + case FrameDumpResolutionType::WINDOW_RESOLUTION: + { + if (!g_gfx->IsHeadless()) + { + target_rect = GetTargetRectangle(); + break; + } + [[fallthrough]]; + } + case FrameDumpResolutionType::XFB_ASPECT_RATIO_CORRECTED_RESOLUTION: + { + target_rect = m_xfb_rect; + const bool allow_stretch = false; + auto [float_width, float_height] = + ScaleToDisplayAspectRatio(m_xfb_rect.GetWidth(), m_xfb_rect.GetHeight(), allow_stretch); + const float draw_aspect_ratio = CalculateDrawAspectRatio(allow_stretch); + auto [int_width, int_height] = + FindClosestIntegerResolution(float_width, float_height, draw_aspect_ratio); + target_rect = MathUtil::Rectangle(0, 0, int_width, int_height); + break; + } + case FrameDumpResolutionType::XFB_RAW_RESOLUTION: + { target_rect = m_xfb_rect; + break; + } + } int width = target_rect.GetWidth(); int height = target_rect.GetHeight(); - // Ensure divisibility by "VIDEO_ENCODER_LCM" to make it compatible with all the video - // encoders. Note that this is theoretically only necessary when recording videos and not + // Ensure divisibility by "VIDEO_ENCODER_LCM" and a min of 1 to make it compatible with all the + // video encoders. Note that this is theoretically only necessary when recording videos and not // screenshots. // We always scale positively to make sure the least amount of information is lost. // // TODO: this should be added as black padding on the edges by the frame dumper. - if ((width % VIDEO_ENCODER_LCM) != 0) + if ((width % VIDEO_ENCODER_LCM) != 0 || width == 0) width += VIDEO_ENCODER_LCM - (width % VIDEO_ENCODER_LCM); - if ((height % VIDEO_ENCODER_LCM) != 0) + if ((height % VIDEO_ENCODER_LCM) != 0 || height == 0) height += VIDEO_ENCODER_LCM - (height % VIDEO_ENCODER_LCM); // Remove any black borders, there would be no point in including them in the recording @@ -237,6 +263,8 @@ void Presenter::ProcessFrameDumping(u64 ticks) const target_rect.right = width; target_rect.bottom = height; + // TODO: any scaling done by this won't be gamma corrected, + // we should either apply post processing as well, or port its gamma correction code g_frame_dumper->DumpCurrentFrame(m_xfb_entry->texture.get(), m_xfb_rect, target_rect, ticks, m_frame_count); } @@ -360,7 +388,8 @@ float Presenter::CalculateDrawAspectRatio(bool allow_stretch) const if (aspect_mode == AspectMode::Stretch) return (static_cast(m_backbuffer_width) / static_cast(m_backbuffer_height)); - auto& vi = Core::System::GetInstance().GetVideoInterface(); + // The actual aspect ratio of the XFB texture is irrelevant, the VI one is the one that matters + const auto& vi = Core::System::GetInstance().GetVideoInterface(); const float source_aspect_ratio = vi.GetAspectRatio(); // This will scale up the source ~4:3 resolution to its equivalent ~16:9 resolution @@ -551,7 +580,7 @@ void Presenter::UpdateDrawRectangle() // Don't know if there is a better place for this code so there isn't a 1 frame delay if (g_ActiveConfig.bWidescreenHack) { - auto& vi = Core::System::GetInstance().GetVideoInterface(); + const auto& vi = Core::System::GetInstance().GetVideoInterface(); float source_aspect_ratio = vi.GetAspectRatio(); // If the game is meant to be in widescreen (or forced to), // scale the source aspect ratio to it. @@ -595,9 +624,10 @@ void Presenter::UpdateDrawRectangle() // Crop the picture to a standard aspect ratio. (if enabled) auto [crop_width, crop_height] = ApplyStandardAspectCrop(draw_width, draw_height); + const float crop_aspect_ratio = crop_width / crop_height; // scale the picture to fit the rendering window - if (win_aspect_ratio >= crop_width / crop_height) + if (win_aspect_ratio >= crop_aspect_ratio) { // the window is flatter than the picture draw_width *= win_height / crop_height; @@ -668,6 +698,7 @@ std::tuple Presenter::ScaleToDisplayAspectRatio(const int width, c std::tuple Presenter::CalculateOutputDimensions(int width, int height, bool allow_stretch) const { + // Protect against zero width and height, a minimum of 1 will do width = std::max(width, 1); height = std::max(height, 1); -- cgit v1.2.3 From 66592f79f2230bf5589bccb1f51da72150bf89c9 Mon Sep 17 00:00:00 2001 From: Filoppi Date: Sun, 3 Mar 2024 15:10:23 +0200 Subject: Video: remove enforced resolution least common multiple of 4 when dumping screenshots and not videos (only videos encoders have this limit). NOTE: this will likely trigger FIFOCI differences. --- Source/Core/VideoCommon/Present.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'Source/Core/VideoCommon/Present.cpp') diff --git a/Source/Core/VideoCommon/Present.cpp b/Source/Core/VideoCommon/Present.cpp index 3a026bedd5..7de27f5049 100644 --- a/Source/Core/VideoCommon/Present.cpp +++ b/Source/Core/VideoCommon/Present.cpp @@ -25,9 +25,6 @@ std::unique_ptr g_presenter; -// The video encoder needs the image to be a multiple of x samples. -static constexpr int VIDEO_ENCODER_LCM = 4; - namespace VideoCommon { // Stretches the native/internal analog resolution aspect ratio from ~4:3 to ~16:9 @@ -246,16 +243,18 @@ void Presenter::ProcessFrameDumping(u64 ticks) const int width = target_rect.GetWidth(); int height = target_rect.GetHeight(); - // Ensure divisibility by "VIDEO_ENCODER_LCM" and a min of 1 to make it compatible with all the + const int resolution_lcm = g_frame_dumper->GetRequiredResolutionLeastCommonMultiple(); + + // Ensure divisibility by the dumper LCM and a min of 1 to make it compatible with all the // video encoders. Note that this is theoretically only necessary when recording videos and not // screenshots. // We always scale positively to make sure the least amount of information is lost. // // TODO: this should be added as black padding on the edges by the frame dumper. - if ((width % VIDEO_ENCODER_LCM) != 0 || width == 0) - width += VIDEO_ENCODER_LCM - (width % VIDEO_ENCODER_LCM); - if ((height % VIDEO_ENCODER_LCM) != 0 || height == 0) - height += VIDEO_ENCODER_LCM - (height % VIDEO_ENCODER_LCM); + if ((width % resolution_lcm) != 0 || width == 0) + width += resolution_lcm - (width % resolution_lcm); + if ((height % resolution_lcm) != 0 || height == 0) + height += resolution_lcm - (height % resolution_lcm); // Remove any black borders, there would be no point in including them in the recording target_rect.left = 0; -- cgit v1.2.3 From 2f13be5a2d05b41ba91a2abfa4af2c79a85bb34f Mon Sep 17 00:00:00 2001 From: "Admiral H. Curtiss" Date: Sat, 13 Apr 2024 03:20:06 +0200 Subject: VideoConfig: Adjust FrameDumpResolutionType enum class to style guidelines --- Source/Core/VideoCommon/Present.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'Source/Core/VideoCommon/Present.cpp') diff --git a/Source/Core/VideoCommon/Present.cpp b/Source/Core/VideoCommon/Present.cpp index 7de27f5049..d6794a5d54 100644 --- a/Source/Core/VideoCommon/Present.cpp +++ b/Source/Core/VideoCommon/Present.cpp @@ -212,7 +212,7 @@ void Presenter::ProcessFrameDumping(u64 ticks) const switch (g_ActiveConfig.frame_dumps_resolution_type) { default: - case FrameDumpResolutionType::WINDOW_RESOLUTION: + case FrameDumpResolutionType::WindowResolution: { if (!g_gfx->IsHeadless()) { @@ -221,7 +221,7 @@ void Presenter::ProcessFrameDumping(u64 ticks) const } [[fallthrough]]; } - case FrameDumpResolutionType::XFB_ASPECT_RATIO_CORRECTED_RESOLUTION: + case FrameDumpResolutionType::XFBAspectRatioCorrectedResolution: { target_rect = m_xfb_rect; const bool allow_stretch = false; @@ -233,7 +233,7 @@ void Presenter::ProcessFrameDumping(u64 ticks) const target_rect = MathUtil::Rectangle(0, 0, int_width, int_height); break; } - case FrameDumpResolutionType::XFB_RAW_RESOLUTION: + case FrameDumpResolutionType::XFBRawResolution: { target_rect = m_xfb_rect; break; -- cgit v1.2.3