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

Flexible buffer bindings #67

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Flexible buffer bindings #67

wants to merge 10 commits into from

Conversation

lshoek
Copy link
Contributor

@lshoek lshoek commented Nov 2, 2024

Removes restriction that would assert that all buffers bound to bindings would have the same size as specified by the shader declaration. Buffer declarations are also permitted to have an element size of zero. This makes it possible to define buffer bindings without specifying the size in the shader:

layout(std430) restrict readonly buffer InPositions
{
	vec4 positions[];
};

We only check that the buffer stride equals the stride in the declaration.

Also:

  • Solve some validation errors due to insufficient render target clear colors (these must be greater than or equal to the number of attachments)
  • Added a texture copy function that wraps vkCmdCopyImage

@lshoek lshoek requested a review from cklosters November 2, 2024 01:47
Copy link
Member

@cklosters cklosters left a comment

Choose a reason for hiding this comment

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

I support the changes, see comments

system_modules/naprender/src/bufferbindinginstance.cpp Outdated Show resolved Hide resolved
system_modules/naprender/src/rendertarget.cpp Show resolved Hide resolved
system_modules/naprender/src/shader.cpp Outdated Show resolved Hide resolved
system_modules/naprender/src/shader.cpp Show resolved Hide resolved
system_modules/naprender/src/textureutils.cpp Outdated Show resolved Hide resolved
system_modules/naprender/src/textureutils.cpp Outdated Show resolved Hide resolved
@lshoek
Copy link
Contributor Author

lshoek commented Jan 6, 2025

Made changes to naprender such that image layouts are properly tracked. NAP always assumed image layout SHADER_READ after any render pass, and would not update nap::ImageData. This can no longer fly as we want to perform other operations on textures such as clear/blit/copy without having to guess the image layout.

I ran into some of there problems on a project where I implemented render target content conservation. Now merging this back into NAP, so nap::RenderTarget now has the property Clear (true by default) that lets you avoid clearing the render target! Very useful in some cases like feedback!

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