Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Software: Fix bad backbuffer size #9742

Merged
merged 1 commit into from Jun 23, 2021
Merged

Conversation

Pokechu22
Copy link
Contributor

MAX_XFB_WIDTH/HEIGHT are the largest XFB sizes seen in practice, but do not make sense to use for the backbuffer size, which should be the size of the window. The old code created screenshots with a size of 720x540 on NTSC games when "Dump Frames at Internal Resolution" is unchecked; now, the window size is used. See bug 12519 (though note that this does not entirely fix that).

With "Auto-Adjust Window Size" checked, screenshots of Super Mario Sunshine's title screen are now 640 by 480 with or without "Dump Frames at Internal Resolution" checked. With "Dump Frames at Internal Resolution" unchecked, resizing the window results in the screenshot being saved at a 4:3 aspect ratio based on the window's height (the software renderer itself also stretches the image, unlike other renderers which add bars on the side to maintain 4:3).

This change was based on the following code from OGLRenderer:

Renderer::Renderer(std::unique_ptr<GLContext> main_gl_context, float backbuffer_scale)
: ::Renderer(static_cast<int>(std::max(main_gl_context->GetBackBufferWidth(), 1u)),
static_cast<int>(std::max(main_gl_context->GetBackBufferHeight(), 1u)),
backbuffer_scale, AbstractTextureFormat::RGBA8),

void Renderer::CheckForSurfaceResize()
{
if (!m_surface_resized.TestAndClear())
return;
m_main_gl_context->Update();
m_backbuffer_width = m_main_gl_context->GetBackBufferWidth();
m_backbuffer_height = m_main_gl_context->GetBackBufferHeight();
m_system_framebuffer->UpdateDimensions(m_backbuffer_width, m_backbuffer_height);
}

@phire
Copy link
Member

phire commented May 23, 2021

screenshots of Super Mario Sunshine's title screen are now 640 by 480

They shouldn't be.
For NTSC Super Mario Sunshine, the OpenGL backend does screenshots at 640x476.

It derives this by taking the original xfb/efb resolution of 640x448 and multiplying that by a calculated aspect ratio of 1.3429594, while preserving width: 640 / 1.3429594 = 476.56 and rounding that down to 476.

@phire
Copy link
Member

phire commented May 23, 2021

The 1.3429594 aspect ratio (aka 4.03:3) is an Image Aspect Ratio, not a Screen Aspect Ratio that can be assumed to be 4:3 (aka 1.3333333)

It comes from the fact that Super Mario Sunshine doesn't use the full NTSC resolution of (from memory) 710.5 by 486 and adds black bars of 38 pixels vertically and 70.5 horizontally. It's the 710.5 by 486 area that is defined as the Screen Aspect Ratio of 4:3 and with these black bars "removed" the result image is no-longer 4:3.

While SMS ends up being really close to 4:3, a lot of other games end up further away. The most extreme example are the cut-scenes in Rogue Squadron 2 and 3 that use an Image Aspect Ratio of ~2:1

@Pokechu22
Copy link
Contributor Author

They shouldn't be.
For NTSC Super Mario Sunshine, the OpenGL backend does screenshots at 640x476.

That's odd. I was previously testing by using a fifolog, not by loading the game. Here's some more data. † indicates the default size based on auto-adjust window size, but it changes when the window is resized (which I tested in all cases, even with dump at internal resolution on).

Master, running SMS.

RendererDump Frames at Internal Resolution
OffOn
OpenGL640 by 476†640 by 476
Software720 by 536640 by 476

Master, using a SMS fifolog:

RendererDump Frames at Internal Resolution
OffOn
OpenGL640 by 480†640 by 480
Software720 by 540640 by 480

This PR, running SMS.

RendererDump Frames at Internal Resolution
OffOn
OpenGL640 by 476†640 by 476
Software640 by 476†640 by 476

This PR, using a SMS fifolog.

RendererDump Frames at Internal Resolution
OffOn
OpenGL640 by 480†640 by 480
Software640 by 480†640 by 480

My conclusion is that there's some weird bug with the way the fifo player chooses its size (which affects the size of the window, not just screenshots), but that's separate from the issue this PR fixes: the screenshot size when "Dump Frames at Internal Resolution" is off being different between OGL and Software.

@phire
Copy link
Member

phire commented May 24, 2021

I was previously testing by using a fifolog, not by loading the game.

My conclusion is that there's some weird bug with the way the fifo player chooses its size

The fifo player is limited because the dff file format doesn't save the VI registers, which are essential for calculating the correct aspect ratio.

So fifo player just guesses.

MAX_XFB_WIDTH/HEIGHT are the largest XFB sizes seen in practice, but do not make sense to use for the backbuffer size, which should be the size of the window.  The old code created screenshots with a size of 720x540 on NTSC games when "Dump Frames at Internal Resolution" is unchecked; now, the window size is used.
@JMC47 JMC47 merged commit 46120a6 into dolphin-emu:master Jun 23, 2021
11 checks passed
@Rumi-Larry
Copy link

I was previously testing by using a fifolog, not by loading the game.

My conclusion is that there's some weird bug with the way the fifo player chooses its size

The fifo player is limited because the dff file format doesn't save the VI registers, which are essential for calculating the correct aspect ratio.

So fifo player just guesses.

Well, I guess that making the FIFO player more accurate, with the IV registers is more trouble than it's worth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants