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

More flexible texture matching, bias matching by the bind sequence counters #15857

Merged
merged 7 commits into from
Aug 18, 2022

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Aug 17, 2022

Since we now have the bind sequence numbers, which should almost on its own be a fairly solid way of picking the right framebuffer (or its depth buffer) to texture from, I originally intended to simplify things a lot more, but decided to let the pile of heuristics in MatchFramebuffer live on for now, at least until we get deferred depth copies (and some will still be needed).

In addition to adding a match bias from the sequence number (so the most recent texture is more likely to be chosen if two are very similar), this always tries to match against both color and depth, instead of deciding based on the "depth swizzle" bits in the address. Turns out that games like Kuroyou will texture color from a "depth address" in order to apply the depth swizzle. Since we just ignore swizzle entirely, that's just a copy for us, but it still needs to happen. Some more details here.

The reason I want this in first is that without it, I can't fully test #15777 with the dumps from the game - and splitting it out will make bisection easier, as usual.

@hrydgard hrydgard added this to the v1.14.0 milestone Aug 17, 2022
@hrydgard hrydgard added GE emulation Backend-independent GPU issues Code Cleanup Cleanup to make future work easier. Needs to be done sometimes. labels Aug 17, 2022
candidates.push_back(AttachCandidate{ match, entry, framebuffer, RASTER_COLOR, framebuffer->colorBindSeq });
}
if (gstate_c.Supports(GPU_SUPPORTS_DEPTH_TEXTURE) && MatchFramebuffer(entry, framebuffer, texAddrOffset, RASTER_DEPTH, &match)) {
candidates.push_back(AttachCandidate{ match, entry, framebuffer, RASTER_DEPTH, framebuffer->depthBindSeq });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might want to swap these lines:

vfb->depthBindSeq = GetBindSeqCount();

I'd prefer to give color slight preference if they were bound right next to each other (I realize you checked for the overlap case, but I think it's a general thing.)

Also, I wonder if we should be trying to bind a null texture on !gstate_c.Supports(GPU_SUPPORTS_DEPTH_TEXTURE) rather than going to RAM... it feels potentially more consistent. Not sure.

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those lines will be separated (and in your preferred order) in #15858 :)

Yes, binding a null texture makes more sense, I'll do that.

@unknownbrackets unknownbrackets merged commit 0749f14 into master Aug 18, 2022
@unknownbrackets unknownbrackets deleted the sequence-based-texture-matching branch August 18, 2022 02:28
@unknownbrackets
Copy link
Collaborator

Someone reported on Discord that this broke the menu (new game / load game / etc.) of Crisis Core. Haven't had time to check into it.

-[Unknown]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Cleanup Cleanup to make future work easier. Needs to be done sometimes. GE emulation Backend-independent GPU issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants