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

Support multi-layered render target #8108

Merged
merged 8 commits into from
Sep 9, 2024
Merged

Support multi-layered render target #8108

merged 8 commits into from
Sep 9, 2024

Conversation

z3moon
Copy link
Contributor

@z3moon z3moon commented Sep 6, 2024

Clients can create a multi-layered render target that consists of array textures, and use it as a custom render target.

A new sample app "hellostereo" demonstrates how to use this feature.

Clients can create a multi-layered render target that consists of array
textures, and use it as a custom render target.

A new sample app "hellostereo" demonstrates how to use this feature.
samples/hellostereo.cpp Outdated Show resolved Hide resolved
@@ -34,7 +34,7 @@ struct RenderTarget::BuilderDetails {
uint32_t mWidth{};
uint32_t mHeight{};
uint8_t mSamples = 1; // currently not settable in the public facing API
uint8_t mLayerCount = 0;// currently not settable in the public facing API
uint8_t mLayerCount = 0;
Copy link
Contributor

@poweifeng poweifeng Sep 6, 2024

Choose a reason for hiding this comment

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

I think this needs to be 1. This value gets directly assigned to a range struct in vk for doing barriers, and it needs to be > 0, which makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though I can change it to 1 since layer==1 still means a regular RT, I'm not sure what you meant. Could you elaborate on where it could cause an issue? I tested it with both Vulkan and OpenGL backends.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry not about barriers, but when the image is created.

Here's the validation error it tripped:
https://registry.khronos.org/vulkan/specs/1.3-extensions/html/vkspec.html#VUID-VkImageViewCreateInfo-imageViewType-04973

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Makes sense. Let me update it.

@@ -107,6 +112,7 @@ RenderTarget* RenderTarget::Builder::build(Engine& engine) {

mImpl->mWidth = minWidth;
mImpl->mHeight = minHeight;
mImpl->mLayerCount = minDepth;
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, can minDepth be 0? In which case, it needs to be 1 IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It cannot be 0. The default for depth is 1.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose the user could pass 0, but that would be an precondition violation.

Copy link
Contributor Author

@z3moon z3moon Sep 9, 2024

Choose a reason for hiding this comment

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

What I meant by "it cannot be 0" is that even if a user passes 0 explicitly, FTexture::getDepth returns 1 as minimum. But yeah, they should know what went wrong in the case. Let me add one more check just in case.


int main(int argc, char** argv) {
App app{};
app.config.title = "stereoscopic rendering";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the default Config.stereoscopicType is NONE, then is there a bug if this renders in stereo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! this sample requires the multiview build flag. Let me update it.

@@ -107,6 +112,7 @@ RenderTarget* RenderTarget::Builder::build(Engine& engine) {

mImpl->mWidth = minWidth;
mImpl->mHeight = minHeight;
mImpl->mLayerCount = minDepth;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose the user could pass 0, but that would be an precondition violation.

@z3moon z3moon enabled auto-merge (squash) September 9, 2024 17:06
@z3moon z3moon disabled auto-merge September 9, 2024 20:04
@z3moon z3moon enabled auto-merge (squash) September 9, 2024 20:04
@z3moon z3moon disabled auto-merge September 9, 2024 20:04
@z3moon z3moon enabled auto-merge (squash) September 9, 2024 20:05
@z3moon z3moon merged commit 37c615e into main Sep 9, 2024
11 checks passed
@z3moon z3moon deleted the zm/layered-rt branch September 9, 2024 20:27
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.

4 participants