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

Change Depth Store Default to Don't Care #1135

Closed

Conversation

ahcox
Copy link
Contributor

@ahcox ahcox commented May 27, 2024

Using the Gears example app to experiment with ARM's new Frame Advisor performance tool for their Mali GPUs I noticed that the depth buffer was being written out:

frameadvisor_gears_storing_depth

With this one-line change to the base class, the depth isn't being written:
frameadvisor_gears_not_storing_depth png

If this base class change will have negative impacts on other examples, I could make a slightly more complex PR which allows derived examples to specify the depth buffer storage operation to the base class before it sets up its main renderpass in VulkanExample::setupRenderPass().

@SaschaWillems
Copy link
Owner

Can you elaborate a bit? Is this a vendor specific optimization?

As this touches the base class I would have to check with all samples to make sure this doesn't break anything. Will probably take some time to review this.

@ahcox
Copy link
Contributor Author

ahcox commented Jun 1, 2024

Can you elaborate a bit? Is this a vendor specific optimization?

In the tile-based GPUs typical of mobile devices like phones, tablets, and watches, the depth buffer exists in a small area of static RAM alongside colour buffers. This normally represents a rectangle of up to ~64x64 pixels depending on pixel formats and levels of MSAA. In rendering the main view of a game, while a colour buffer does need to be written from tile memory to external RAM for post processing and scan out to a display, the depth buffer does not need to be written to memory at all, and not doing so is a large bandwidth saving. By setting the flag the way it is before this PR, the code explicitly instructs the Vulkan implementation to write the depth buffer to external memory. By setting the flag used in this PR the code is telling the implementation that the depth buffer may be safely discarded without being written to RAM. This is a cross-vendor feature in Vulkan that definitely helps Imagination PowerVR and ARM Mali tile based mobile GPUs, and probably Adrenos, Apple Silicon via MoltenVK, and less well-known ones too. In addition, a desktop GPU vendor should at least be able to use it to invalidate cachelines covering the depth buffer rather than letting them write back over time uselessly.

As this touches the base class I would have to check with all samples to make sure this doesn't break anything. Will probably take some time to review this.

I was putting this in speculatively in hopes you'd know it was safe off the top of your head. I don't want to make work for you more than reviewing the PR already did so I will close it.

@ahcox ahcox closed this Jun 1, 2024
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.

2 participants