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

TextureCache: Simplify XFB reconstruction #7925

Merged
merged 4 commits into from Apr 21, 2019

Conversation

3 participants
@stenzek
Copy link
Contributor

stenzek commented Mar 24, 2019

Currently, we don't consider the stride from the video interface when loading the XFB texture from memory. This results in textures which are twice the width being created, with both fields side-by-side. The data is then ignored as only the first half of the texture is actually displayed, but it's still incorrect.

This PR changes things to create two cache entries, one for each field, and display them as the real hardware would.

ARM also had no CPU XFB decoding function, and with the GPUs not supporting texel buffers large enough, GPU decoding wasn't possible. So on Android, you likely just saw black screens for in-memory XFBs.

Lastly, we're still reliant on the force progressive hack, unless XFB2RAM is enabled. If XFB2RAM is disabled, you'll just get a purple screen, as for interlaced scanout, the stride won't match (copy stride will be half the scanout stride, as it's only displaying every second line). Otherwise, you can disable the hack, and see the half-vertical-resolution interlaced video in all its glory. See the comment in TextureCacheBase.cpp in StitchXFBCopy() for more info.

@iwubcode

This comment has been minimized.

Copy link
Contributor

iwubcode commented Mar 24, 2019

I tested the same games I tested for Hybrid XFB. On D3D, I don't notice any obvious regressions. However, Vulkan complains about "Dest rect is too large for CopyRectangleFromTexture", this does not happen on master.

I noticed that DoPartialTextureUpdates is used for VRAM textures, that might be the issue. I believe the whole reason I had created GetTextureFromOverlappingTextures (besides disliking the name of the previous function) was because some of the math for DoPartialTextureUpdates did not work for XFB copies. I thought of fixing DoPartialTextureUpdates but was afraid the change would potentially cause ill effects in EFB-based games.

Compare the two blobs of code:

// GetTextureFromOverlappingTextures
if (entry->addr >= stitched_entry->addr)
{
	s32 block_offset = (entry->addr - stitched_entry->addr) / tex_info.bytes_per_block;
	s32 block_x = block_offset % numBlocksX;
	s32 block_y = block_offset / numBlocksX;
	src_x = 0;
	src_y = 0;
	dst_x = block_x * tex_info.block_width;
	dst_y = block_y * tex_info.block_height;
}
else
{
	s32 srcNumBlocksX = entry->native_width / tex_info.block_width;
	s32 block_offset = (stitched_entry->addr - entry->addr) / tex_info.bytes_per_block;
	s32 block_x = block_offset % srcNumBlocksX;
	s32 block_y = block_offset / srcNumBlocksX;
	src_x = block_x * tex_info.block_width;
	src_y = block_y * tex_info.block_height;
	dst_x = 0;
	dst_y = 0;
}


// DoPartialTextureUpdates
if (entry->addr >= entry_to_update->addr)
{
	u32 block_offset = (entry->addr - entry_to_update->addr) / block_size;
	u32 block_x = block_offset % numBlocksX;
	u32 block_y = block_offset / numBlocksX;
	src_x = 0;
	src_y = 0;
	dst_x = block_x * block_width;
	dst_y = block_y * block_height;
}
else
{
	u32 block_offset = (entry_to_update->addr - entry->addr) / block_size;
	u32 block_x = (~block_offset + 1) % numBlocksX;
	u32 block_y = (block_offset + block_x) / numBlocksX;
	src_x = 0;
	src_y = block_y * block_height;
	dst_x = block_x * block_width;
	dst_y = 0;
}
@stenzek

This comment has been minimized.

Copy link
Contributor Author

stenzek commented Mar 24, 2019

Yeah, my original change used DoPartialTextureUpdates for XFB copies too. But that doesn't consider stride, or the order of copies (re rogue leader comment), so there still needs to be a second path.

Using block math doesn't really make sense to me, as the XFBs aren't tiled in memory.

edit: ahh, I think I see how it's getting an out-of-range rectangle, the size is checked before scaling..

@iwubcode

This comment has been minimized.

Copy link
Contributor

iwubcode commented Mar 24, 2019

Using block math doesn't really make sense to me, as the XFBs aren't tiled in memory.

True but my point wasn't about the block math, it was to call out that the DoPartialTextureUpdates and GetTextureFromOverlappingTextures used different logic to compute the rectangles and that reverting that change might have adverse effects.

Then again, D3D runs fine so maybe Vulkan is just being extra picky :)

@stenzek stenzek force-pushed the stenzek:xfb-stride branch from 49fb952 to 5b093b0 Mar 25, 2019

@stenzek

This comment has been minimized.

Copy link
Contributor Author

stenzek commented Mar 25, 2019

@iwubcode had some errors in my implementation, it was leaking cache entries, as well as not handling mixed native/internal resolution XFB data. Should hopefully be fixed now.

@iwubcode

This comment has been minimized.

Copy link
Contributor

iwubcode commented Mar 26, 2019

Retested numerous XFB games on Vulkan and D3D. Not seeing any regressions anymore.

@stenzek stenzek added the WIP label Mar 29, 2019

@stenzek stenzek force-pushed the stenzek:xfb-stride branch 2 times, most recently from 8562227 to 077eeda Mar 31, 2019

stenzek added some commits Mar 24, 2019

TextureDecoder: Move XFB decoding to Common
This was previously missing for generic (which is used on ARM).
TextureCache: Simplify XFB reconstruction
This also better handles in-memory interlaced XFB data placed by the CPU
by considering the stride from the VI.

@stenzek stenzek force-pushed the stenzek:xfb-stride branch from 077eeda to fa76f0a Apr 21, 2019

@stenzek stenzek force-pushed the stenzek:xfb-stride branch from fa76f0a to b09a0e1 Apr 21, 2019

@JMC47 JMC47 merged commit 18589e5 into dolphin-emu:master Apr 21, 2019

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.