Skip to content

More precise buffer size calculation #2950

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mikusp
Copy link
Contributor

@mikusp mikusp commented May 17, 2025

Another attempt at fixing the 'non-GPU memory tracking' errors after #2920. This one tries to make sure that our calculated size does not exceed the total buffer size in case of using buffer offsets.

@GHU7924
Copy link

GHU7924 commented May 17, 2025

[CUSA03173] Bloodborne

[Debug] <Critical> resource.h:88 operator(): Assertion Failed!

[CUSA00897] inFAMOUS™ First Light

The graphics in the menu are broken. (This game is not working stably for me yet and I can’t always get into the game, but the menu always works for me.)

Main:
Снимок экрана 2025-05-17 213044

PR:
Снимок экрана 2025-05-17 175900

[CUSA03745] The Last Guardian™

Can we say that this PR is fixing the lighting? Looks like something close to the original))

shadPS4 v0 8 1 WIP pr-2950-more_precise_buffer_size_calculation 62c21af _ CUSA03745 - The Last Guardian™ 01 03 Сб, 17 мая 2025 г  18_09_01

@gandalfthewhite19890404

Diablo III 1.26 - still
image

@mikusp mikusp force-pushed the more_precise_buffer_size_calculation branch from d5ec8a0 to df07ed4 Compare May 17, 2025 15:50
@mikusp mikusp force-pushed the more_precise_buffer_size_calculation branch from df07ed4 to 84667cf Compare May 17, 2025 15:52
@mikusp
Copy link
Contributor Author

mikusp commented May 17, 2025

@gandalfthewhite19890404 those errors can originate from buffers or images, unfortunately the latter are still a problem

@GHU7924
Copy link

GHU7924 commented May 17, 2025

@mikusp

[CUSA03173] Bloodborne - fixed

[CUSA00897] inFAMOUS™ First Light - the menu graphics are still broken

@StevenMiller123
Copy link
Contributor

Causes vertex explosions for Intel GPUs in Persona 5 Royal.

Main:
image

PR:
image

@GHU7924
Copy link

GHU7924 commented May 17, 2025

Now I've tested [CUSA03745] The Last Guardian more, as soon as I got into the game, and then I pressed "Return to Title Screen", I also got vertex explosions in the menu.

Main:
Снимок экрана 2025-05-17 205036

PR:
Снимок экрана 2025-05-17 204030

@raphaelthegreat
Copy link
Contributor

raphaelthegreat commented May 18, 2025

Could you explain the logic behind this change? Because I don't see how it's correct (and the regressions also confirm this). The AMD isa docs mention about clamping

The address is clamped if:
• Stride is zero: clamp if (offset >= num_records)
• Stride is non-zero: clamp if (offset > (stride * num_records))

Which is how the buffer size calculation on main branch is derived. Your change shaves stride bytes from this size and adds bpp bytes. This will have no effect in most cases where the stride is equal to the bpp, but stride can be larger or even smaller than bpp which will result in different size calculations.

Also the data format in the V# doesn't even matter in most cases. It only matters for BUFFER_LOAD_FORMAT/BUFFER_STORE_FORMAT instructions. TBUFFER* instructions hardcode the format in the shader and S_BUFFER* ones typically used for UBOs ignore format and load raw data. You could have a UBO with stride = 4 and dfmt = Format8, which with this change will shave off 3 bytes from the end of the buffer, probably breaking some matrix and causing these glitches.

I still think you should try the clamping thing, it's not uncommon for V# to have way larger sizes than what is actually mapped and as long as the shader accesses in bounds it doesn't matter on real HW.

@mikusp
Copy link
Contributor Author

mikusp commented May 19, 2025

I've put some more detailed explanations in #2920, basically it was an attempt to stop buffers from extending over unallocated memory in some specific cases, namely the buffer used has stride larger than its dfmt size, it has a non-zero offset from the buffer start and the buffer spans the whole VMA. Then the memory manager would try to map a page past the allocated memory and fail. I've checked in RE2 and clamping all buffers seems to give me the same result, but I don't know how it would behave in the games mentioned here

@raphaelthegreat
Copy link
Contributor

raphaelthegreat commented May 19, 2025

By offset you mean an offset inside the shader, or the adjustment offset in push constants? I don't think you need to view this as a specific case by associating the stride size and bpp with this. There are plenty of cases where stride is a lot larger than bpp (texel buffers used to be implemented under the assumption stride == bpp, which triggered asserts in a bunch of games). There are also a lot of other games that exhibit similar behavior of the size covering unmapped areas. Lowering the clamping threshold to 2MB should be a decent comprise that wont affect most buffers perf wise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants