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

Decode EFB copies used as paletted textures. #2059

Merged
merged 3 commits into from Feb 20, 2015

Conversation

magumagu
Copy link
Contributor

A number of games make an EFB copy in I4/I8 format, then use it as a
texture in C4/C8 format. Detect when this happens, and decode the copy on
the GPU using the specified palette.

This has a few advantages: it allows using EFB2Tex for a few more games,
it, it preserves the resolution of scaled EFB copies, and it's probably a
bit faster.

D3D only at the moment, but porting to OpenGL should be straightforward. I'm not as familiar with the OpenGL API, though, so I would appreciate some help with that.

The changes to the texture cache (TextureCacheBase.h/cpp) are the part that really needs to be reviewed closely; I'm not really confident this is the right direction.

@JMC47
Copy link
Contributor

JMC47 commented Feb 15, 2015

Pinging @degasus and @Sonicadvance1 for an OpenGL implementation.

@delroth
Copy link
Member

delroth commented Feb 15, 2015

@Mullin
Copy link
Contributor

Mullin commented Feb 16, 2015

Very nice !

RAM_PALETTE,
// A texture with two TMEM addresses.
// May be either decoded with a palette or an RGBA8 texture with two parts.
DUAL_TMEM,

This comment was marked as off-topic.

@degasus
Copy link
Member

degasus commented Feb 16, 2015

Was there a reason to sqaush this implementation with your deferred freeing of textures? imo for the deferred freeing, it's better to just lock this texture for at least a draw call, or just until the next frame, or better don't remove them from the texture cache for at least a frame ( #2001 )

void TextureCache::ConvertTexture(TextureCache::TCacheEntryBase* entry, TCacheEntryBase* unconverted, void* palette, TlutFormat format)
{
// TODO: Implement.
return;

This comment was marked as off-topic.

This comment was marked as off-topic.

@Linktothepast
Copy link
Contributor

Apart from the twilight princess map that is upscaled and working with efb to texture, seems that rogue squadron and rebel strike targeting computer works perfectly fine too with efb to texture, unfortunately those games still need efb to ram for a few effects like distant fog etc. They are quite playable with efb to texture bearing a few such glitches.
Edit: List of affected games
Okami
twilight princess
rogue squadron
rebel strike
FFCC THE CRYSTAL BEARERS

@magumagu
Copy link
Contributor Author

Updated to get rid of the deferred freeing; some quick testing shows it isn't actually necessary. (Hopefully, this doesn't break anything.)

@JMC47
Copy link
Contributor

JMC47 commented Feb 16, 2015

Metroid Prime 1 won't need EFB2RAM at all.

Metroid Prime 1 - 3 will upscale the other visors with this.

@mimimi085181
Copy link
Contributor

Okami is another game that won't need efb2ram anymore after this. Not even the drawing part needs efb2ram.

@@ -137,7 +137,6 @@
<ClCompile Include="..\libusb\hotplug.c" />
<ClCompile Include="..\libusb\io.c" />
<ClCompile Include="..\libusb\os\poll_windows.c" />
<ClCompile Include="..\libusb\strerror.c" />

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@Linktothepast
Copy link
Contributor

JMC47 Metroid Prime still needs efb to ram, there is texture corruption in the environment without it in the Chozo ruins that i tested regardless of scanner working .

@JMC47
Copy link
Contributor

JMC47 commented Feb 18, 2015

Is it worth the slowdown at that point to have EFB2RAM on?

@Linktothepast
Copy link
Contributor

http://prntscr.com/66uv94 See for yourself and judge, i get something like that all over the place.

@Tinob
Copy link
Contributor

Tinob commented Feb 18, 2015

@Linktothepast Have you tested increasing cache samples to see if that fixes the issue?

@Linktothepast
Copy link
Contributor

Nope, it happens in the safest setting too but seems to be fixed temporarily when moving the slider.

@delroth
Copy link
Member

delroth commented Feb 18, 2015

@Linktothepast do you happen to have a fifolog of that?

@JMC47
Copy link
Contributor

JMC47 commented Feb 18, 2015

I haven't run into those bugs in my quick testing of EFB2Tex in Metroid Prime... I guess I'll go through the Chozo Ruins and see what happens.

@Linktothepast
Copy link
Contributor

https://www.mediafire.com/?idb9wwef9n1l99m there you go, a five frame fifo, it happens with efb to texture and d3d that i tested (didn't check opengl since the fix is not implemented there).

@mimimi085181
Copy link
Contributor

@Linktothepast: Can you test if this happens the master build as well? It might be unrelated to this PR. If it works there, 1% chance a recent change fixed it, can you test the master version the PR is based on then?

@Linktothepast
Copy link
Contributor

Same thing in master unfortunately.

@delroth
Copy link
Member

delroth commented Feb 18, 2015

Oh, so that shouldn't be considered a blocker for this PR. Cool.

Can someone maybe port it to GL? :) Or at least fix the GL related bugs that it introduces.

@Linktothepast
Copy link
Contributor

Eh, no it is not a blocker, i just told jmc that efb to ram is still needed for the game due to the above issue.

@magumagu magumagu force-pushed the palette-convert branch 3 times, most recently from 29f3fad to 588d9dd Compare February 18, 2015 23:21
@magumagu
Copy link
Contributor Author

Added fallback path for OpenGL; should be unchanged from master.

@magumagu
Copy link
Contributor Author

This could probably use one more round of testing to make sure I didn't break anything merging with PR #2001.

Also, JMC47 discovered an issue with the RS2 pause screen with EFB2Tex; I'm not sure what's causing that.

@magumagu
Copy link
Contributor Author

Figured out what's going on with RS2: it has to do with the interaction between BPMEM_PRELOAD_MODE and EFB copies. The way we implement BPMEM_PRELOAD_MODE is a memcpy from main memory to texMem; however, we don't notify the texcache that this is happening, so it loses track of the relationship. Or rather, it should lose track of the relationship, but on master we use the specified address in main memory as a hint to find the right entry even though it doesn't actually have any meaning (?).

Anyway, I'm going to have to mess with the texture cache side of this a bit more before merging this.

@mimimi085181
Copy link
Contributor

As far as i can tell, the rebase on top of #2001 went well. Tested Castlevania 3, Metroid Prime 3(fonts), New Super Mario Bros, Okami and RS2, all ran as expected of #2001 and this PR.

A number of games make an EFB copy in I4/I8 format, then use it as a
texture in C4/C8 format.  Detect when this happens, and decode the copy on
the GPU using the specified palette.

This has a few advantages: it allows using EFB2Tex for a few more games,
it, it preserves the resolution of scaled EFB copies, and it's probably a
bit faster.

D3D only at the moment, but porting to OpenGL should be straightforward..
@magumagu
Copy link
Contributor Author

Rewrote the texture cache stuff; should fix the RS2 regression.

{
// Perform palette decoding.
TCacheEntryBase *entry = unconverted_copy->second;
TCacheEntryBase *decoded_entry = AllocateTexture(entry->config);

This comment was marked as off-topic.

This comment was marked as off-topic.

delroth added a commit that referenced this pull request Feb 20, 2015
Decode EFB copies used as paletted textures.
@delroth delroth merged commit 8b095a0 into dolphin-emu:master Feb 20, 2015
config.rendertarget = true;
config.width = entry->config.width;
config.height = entry->config.height;
config.layers = FramebufferManagerBase::GetEFBLayers();

This comment was marked as off-topic.

@DarthVitrial
Copy link

So, does this PR fix it with OpenGL too, or is that coming in the future?
(Sorry if I'm not supposed to post here)

JosJuice added a commit to JosJuice/dolphin that referenced this pull request Sep 26, 2015
Made unnecessary by PRs dolphin-emu#2059 and dolphin-emu#2960. MP2 and MP3 still need it.
JosJuice added a commit to JosJuice/dolphin that referenced this pull request Sep 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
10 participants