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

Framebuffer manager refactor: Don't cache framebuffer information in TexCacheEntry #13420

Merged
merged 10 commits into from
Sep 13, 2020

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Sep 12, 2020

Instead, re-evaluate framebuffers every time we try to bind a texture and it doesn't find an entry in the texcache map.

This is not that many times per frame normally, so it's really not as bad as it seems. Plus, the code becomes a lot more readable and debuggable without having to take into account all that state.

Even difficult cases like Test Drive and God Of War's shadows work just fine.

Now, this is a draft PR because Breath of Fire III is broken yet again (oops - we lost the detach actions again but since we re-evaluate every time now, that should be ok) but all should be fixable with a little bit of work. And when it's done, it'll be much easier to work on these issues. (UPDATE: fixed)

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Sep 12, 2020
@hrydgard hrydgard added this to the v1.11.0 milestone Sep 12, 2020
@hrydgard
Copy link
Owner Author

hrydgard commented Sep 12, 2020

Added some stats, Test Drive ends up looping through the framebuffers ("evaluates") 96 times per frame, most other games much fewer. I think the possible tiny perf hit is worth the code size and complexity reduction here.

@hrydgard hrydgard marked this pull request as ready for review September 12, 2020 13:38
GPU/Vulkan/TextureCacheVulkan.cpp Outdated Show resolved Hide resolved
GPU/Common/TextureCacheCommon.cpp Outdated Show resolved Hide resolved
GPU/Common/TextureCacheCommon.cpp Show resolved Hide resolved
@hrydgard
Copy link
Owner Author

Made the requested improvements :) The match enum is now pretty much a boolean.

@hrydgard
Copy link
Owner Author

Tested through the usual suspects, everything works.

@hrydgard hrydgard merged commit a6084f6 into master Sep 13, 2020
@hrydgard hrydgard deleted the framebuffer-simplify branch September 13, 2020 14:40
@DonelBueno
Copy link

DonelBueno commented Sep 17, 2020

This PR makes texture load extremely sluggish on all backends.

I don't have a compiler, so I don't know which commit caused it, but from the builds offered by the buildbot, v1.10.3-650-g2b49c3e46 is the last working one and v1.10.3-661-ga6084f6fe is the first problematic build.

Running Windows 10 x64 and Geforce GTX 1070 with the latest drivers.

@hrydgard
Copy link
Owner Author

hrydgard commented Sep 17, 2020

@DonelBueno After 692, the worst issues should be fixed, but yes, there are still some problems. I'm working on it and I'm confident we can get it as fast as it used to be, at least.

@DonelBueno
Copy link

An option to preload custom textures would also be very nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GE emulation Backend-independent GPU issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants