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

Add new DeferredStagingBuffer type #5838

Merged
merged 3 commits into from
Jun 13, 2023
Merged

Conversation

smoogipoo
Copy link
Contributor

On occasion, I have noticed the new PersistentStagingBuffer causing graphics corruption. At least, I think it's because of this type and its undefined nature (part of the reason why we now have an Unmap() inside the copy function).

As a result, I'm going back to buffers to figure out the most efficient update strategy. This implements a new DeferredStagingBuffer, which uses a managed CPU buffer and an intermediate driver-side Staging buffer, to do updates.
The new type is off by default, and is only accessible with an environment variable:

Name Values

OSU_GRAPHICS_STAGING_BUFFER_TYPE

0 = ManagedStagingBuffer
1 = PersistentStagingBuffer
2 = DeferredStagingBuffer

I'd like to test this new type over time, with the intent of it replacing PersistentStagingBuffer. I'm currently unable to reproduce the graphics corruption I was seeing previously, so I intend to be able to reproduce that once more first to see whether it was a fluke.

Type Fullscreen? Performance
ManagedStagingBuffer No image
ManagedStagingBuffer Yes image
PersistentStagingBuffer No image
PersistentStagingBuffer Yes image
DeferredStagingBuffer No image
DeferredStagingBuffer Yes image

@peppy peppy self-requested a review June 13, 2023 05:28
{
case GraphicsBackend.Direct3D11:
case GraphicsBackend.Vulkan:
return new PersistentStagingBuffer<T>(this, count);
Copy link
Member

Choose a reason for hiding this comment

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

To confirm, this is changing d3d back to persistent in a hope that the corruption was a fluke / is fixed? And the new deferred buffer is a fallback for users to potentially test if corruption does come back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not changing D3D/Vulkan.

And the new deferred buffer is a fallback for users to potentially test if corruption does come back?

Correct.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I misread.

@peppy peppy merged commit 4253ec8 into ppy:master Jun 13, 2023
@smoogipoo smoogipoo deleted the deferred-staging-buffer branch September 11, 2023 02:23
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.

2 participants