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

GS: Fix sub-page addressing of Z formats #9302

Merged
merged 2 commits into from
Aug 26, 2023

Conversation

TellowKrinkle
Copy link
Member

@TellowKrinkle TellowKrinkle commented Jul 22, 2023

Description of Changes

They aren't just an offset of the base value like the color formats, but instead an xor of the associated color format

For example, PSMZ32 bp=16, what address is the block at (4, 0)? The equivalent for CT32 would be (16+16 = 32). Our old algorithm would do (16+(16^0x18) = 24). But the actual value is (16+16)^0x18 = 56.

Note: This removes an optimization we had, as it's not compatible with the real GS algorithm. RIP. Hopefully it doesn't hurt performance too much, but if it does, I don't think there's much we can do about it.

Fixes #4309
Fixes #804

Rationale behind Changes

More accurate

Suggested Testing Steps

  • Test FFX-DepthXor.gs.xz.zip
  • Test other dumps in case I broke something in the refactor
  • Test performance and (maybe) be sad

@bigol83
Copy link

bigol83 commented Jul 22, 2023

I tested it and performance is actually better and the graphic issue is fixed at native resolution, but when upscaling there is now a grid of squares appearing and disappearing.

EDIT. Bit more infos: This grid doesn't appear with DX11 and OGL. It appears with DX12 and Vulkan.

Final.Fantasy.X_SLUS-20312_20230722112618.mp4

@kamfretoz
Copy link
Contributor

Tested the performance of the attached GS Dump against master.

At Native:
image

The performance actually increased substantially.

@stenzek
Copy link
Member

stenzek commented Jul 22, 2023

Seems to change invalidation/dirty behaviour in HW. That's why you're seeing a performance difference.

Also explains the squares - it's because it's no longer uploading the whole depth buffer.

Master versus PR:

image
image

@stenzek
Copy link
Member

stenzek commented Jul 22, 2023

Causes an issue on this dump. Likely the same issue as above, but this could just be a HW thing.

Final_Fantasy_X_International_SLPS-25088_20221120023912.gs.xz.zip

image
image

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jul 22, 2023

Causes an issue on this dump. Likely the same issue as above, but this could just be a HW thing.

Final_Fantasy_X_International_SLPS-25088_20221120023912.gs.xz.zip

image image

That is indeed an invalidation problem. I had a similar result in my local invalidation changes when the Z addressing was broken. I plugged this code in to my local invalidation changes and (along with a whole bunch of bug fixes I had to make) it now looks like this

image

One thing I had to be careful of, if the block pointer is page aligned and the rect is 0,0->16,16, it doesn't write to 0,0->16,16! Which was an assumption I was making, so I had to change my invalidation code to calculate the offset if it's depth and the size of the rect is less than 1 page.

@F0bes
Copy link
Member

F0bes commented Jul 22, 2023

I unfortunately don't have much time to test all the previous offsets we did. (The PS2 SDK has a regression I think and I didn't do myself any favours with how I wrote this code)

Here is the one we left off at:
It uploads an image with a Z24 format onto a CT24 framebuffer at 0x2DEE

Target (PS2):
ct24-2dee-ps2

Master:
z24-2DEE-PCSX2

PR:
ct24-2dee-pr

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Jul 27, 2023

LGTM but I suggest we merge both this and #9315 at the same time, since this breaks FFX without it and #9315 fixes it again :)

Just to save from angry Final Fantasy fans waving pitchforks. I'll try to work on the other PR a bit more this weekend.

Not compatible with the real GS's Z addressing, RIP
They aren't just an offset of the base value like the color formats, but instead an xor of the associated color format
@seta-san
Copy link
Contributor

grid doesn't appear with DX11 and OGL. It appe

i'm still having the grid show up flickering in all hardware renderers except DX11. I've got a geforce 3060

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Aug 24, 2023

can you show what you mean please @seta-san ?

Looks fine to me
image

The transitions seem to have some bugs, but that's my bad.

@seta-san
Copy link
Contributor

I’m at work for the next 9 hours

@refractionpcsx2
Copy link
Member

Then, reply after?

@refractionpcsx2
Copy link
Member

refractionpcsx2 commented Aug 25, 2023

That has nothing to do with this PR.

but fwiw, it happens on every renderer but DX11 for me, but it's an upscaling problem, it doesn't happen in native. I think using texture offsets sorts it out.

@refractionpcsx2 refractionpcsx2 merged commit bced0b9 into PCSX2:master Aug 26, 2023
12 checks passed
@TellowKrinkle TellowKrinkle deleted the BlockZAddr branch August 26, 2023 06:36
@karasuhebi
Copy link
Contributor

karasuhebi commented Aug 31, 2023

vulkan https://github.com/PCSX2/pcsx2/assets/8851653/d197aecc-984e-4e73-888a-14cafc770cd7

ogl https://github.com/PCSX2/pcsx2/assets/8851653/b358fc0c-fc90-482a-9d5f-fed221403771

dx12 https://github.com/PCSX2/pcsx2/assets/8851653/e4f2c260-c888-4c8f-b9ef-ee16d69ff896

dx11
Final.Fantasy.X.DX11.mp4

@refractionpcsx2 How is this not related to this PR? @TellowKrinkle mentions in the OP that this PR fixes #804 but this video shows the bug in #804 still happening.

EDIT - Yes what seta-san is pointing out in the videos is not related to the PR but one of the actual bugs being addressed by this PR still shows in those videos.

@stenzek
Copy link
Member

stenzek commented Aug 31, 2023

It looks fine to me?

@karasuhebi
Copy link
Contributor

In the videos from seta-san? The bug described in #804 is clearly still there.

@stenzek
Copy link
Member

stenzek commented Aug 31, 2023

It's very subtle, and likely the result of upscaling.

@karasuhebi
Copy link
Contributor

I think we're talking about two different things. The issue in #804 is not caused by upscaling. I also just looked at my original video of it and the ones from seta-san and it's exactly the same still, not any more subtle.
image
image

@karasuhebi
Copy link
Contributor

Since this is said to fix #4309, I'm wondering if the issue from this comment is also fixed by this:
#995 (comment)

Does anyone know the answer without needing to test? It doesn't happen every time so it's a bit of a pain to test. I can do it if needed though.

@stenzek
Copy link
Member

stenzek commented Aug 31, 2023

Those videos clearly weren't recorded on master. The corruption in the logo was fixed in software by this PR, and in hardware by a combination of ref's invalidation PR, and this change (for the decoding). As shown in refraction's screenshot.

As for the lines, it's an upscaling issue. You can see the depth buffer gets uploaded in two parts, and you have a gap for the second when upscaling, because 1 pixel in GS space is >1 pixel in HW space:

image
image

@stenzek
Copy link
Member

stenzek commented Aug 31, 2023

Also, I checked master myself - looks fine. Refraction's was NTSC, so here's PAL:
image

@karasuhebi
Copy link
Contributor

Yeah I was suspecting that seta-san's videos might've not been on master since I saw ref's screenshot was fine but I wasn't 100% sure. I should've just retested myself instead of commenting on this. I'm really sorry. This is what I get for staying up past 2 AM 😅

And yes, I know the issue seta-san is pointing out in his videos is from upscaling. It looks like every other upscaling issue that I've ever seen. :) Sorry that I made you elaborate on this point, I definitely didn't have any doubts on this one.

@stenzek
Copy link
Member

stenzek commented Aug 31, 2023

If I remember/have time, I might dig into it a bit deeper, it's a little strange that we're flushing in the middle like that. Unless it's writing the top half of the buffer, drawing some of the sprites in the bottom half, then writes the bottom half. That's why the line shows up - because the write gets split in two, with the sprite draw inbetween...

@refractionpcsx2 refractionpcsx2 mentioned this pull request Dec 6, 2023
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Final Fantasy X random enc graphics Final Fantasy X core (?) bug in intro sequence
10 participants