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

Allows "merging" render targets that overlap on the Y axis. Fixes Juiced 2 #15717

Merged
merged 2 commits into from
Jul 24, 2022

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Jul 24, 2022

Fixes #7295.

Juiced 2 bloom works fine with this. It does some tricky overlap between two render targets.

To be safe, gating this behind the related AllowLargeFBTextureOffsets compat flag, which is also required for the effect to work.

Also improves logging of multiple matches when binding framebuffers as textures (which was the main clue).

Additionally, fixes the offset check for X offsets to take bytes-per-pixel into account, which I guess is a very small risk.

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Jul 24, 2022
@hrydgard hrydgard added this to the v1.13.0 milestone Jul 24, 2022
@hrydgard hrydgard marked this pull request as ready for review July 24, 2022 09:52
…7295 (Juiced 2)

To be safe, gating this behind the related AllowLargeFBTextureOffsets,
which is also required for the effect to work.

Additionally, fixes the offset check for X offsets, which I guess is a
very small risk.
@hrydgard hrydgard force-pushed the render-target-y-offset branch from cf985f4 to 04a85b1 Compare July 24, 2022 09:58
@hrydgard hrydgard changed the title Allows "merging" render targets that overlap on the Y access. Fixes #7295 (Juiced 2) Allows "merging" render targets that overlap on the Y axis. Fixes #7295 (Juiced 2) Jul 24, 2022
@hrydgard hrydgard changed the title Allows "merging" render targets that overlap on the Y axis. Fixes #7295 (Juiced 2) Allows "merging" render targets that overlap on the Y axis. Fixes Juiced 2 Jul 24, 2022
@ghost
Copy link

ghost commented Jul 24, 2022

Nice 👍

@unknownbrackets
Copy link
Collaborator

unknownbrackets commented Jul 24, 2022

Does Breath of Fire 3 still work (or maybe that was the other kind of overlap only?) I think there was some Silent Hill game triggering this too with 1024 wide buffers in a framedump?

I feel like the biggest risk for a change like this being global is that some games will switch between two large 4 byte buffers (for videos or something) to two smaller 2 byte buffers and we get misdetections during that. Not sure how bad it would be to render both frames on the same actual framebuffer...

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Jul 24, 2022

Breath of Fire 3 still works fine, and doesn't seem to trigger this kind of x offset. Don't remember which game did... I guess could find out with some git digging.

But I think I'll just revert that particular check to remove the concern entirely.

@unknownbrackets
Copy link
Collaborator

Looks like it was originally #6324. I remember some game having interleaved renders, though, pretty sure it was a Silent Hill.

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Jul 24, 2022

I reverted the x-offset bpp change, but the shadows in GoW do work fine even pre-revert. Let's re-apply after 1.13.

@unknownbrackets
Copy link
Collaborator

Well, the 4 iirc was just to skip the checks inside if it definitely didn't overlap, the actual x offset and height were checked afterward. I don't think it can break it either way.

-[Unknown]

@hrydgard hrydgard merged commit 3c88183 into master Jul 24, 2022
@hrydgard hrydgard deleted the render-target-y-offset branch July 24, 2022 16:48
@ghost
Copy link

ghost commented Jul 25, 2022

While the graphics glitch is fixed the performance is regress :(
Screenshot_2022-07-26-07-10-13-446_org ppsspp ppsspp

@hrydgard
Copy link
Owner Author

Hmm, indeed :(

Letting the two overlapping render targets merge into one fixed the correctness issue, but as a result, instead we detect "self-texturing" (the render target being drawn into is also the texturing being used) which currently forces us into making a copy of the whole render target, about 30 times... Not great.

There are ways to fix this, but it'll have to wait for after the release. In the meantime, the old cheats to turn off the effect probably work.

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.

Juiced 2: Hot Import Nights, screen artifacts and missing half of race tracks
2 participants