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.

Sign up for GitHub

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

Jump to bottom

layers: Cleanup vkCmdBeginRendering #9138

Merged
merged 1 commit into from
Jan 6, 2025
Merged

layers: Cleanup vkCmdBeginRendering #9138

merged 1 commit into from
Jan 6, 2025

Conversation

spencer-lunarg
Copy link
Contributor

The logic for vkCmdBeginRendering was all over the place between Stateless, CoreCheck, and Best Practice

Tried to unify things (naming, how things are broken up, etc) and group things as there is a lot going on and hard to follow it at first read

@spencer-lunarg spencer-lunarg requested a review from a team as a code owner January 5, 2025 23:31
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 337789.

@spencer-lunarg spencer-lunarg force-pushed the spencer-lunarg-begin-rendering-fix branch from de90be0 to d1674d0 Compare January 5, 2025 23:32
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 337804.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18527 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18527 passed.

arno-lunarg
arno-lunarg approved these changes Jan 6, 2025
View reviewed changes
Copy link
Contributor

@arno-lunarg arno-lunarg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There is a mixture of plain continue; and ASSERT_AND_CONTINUE, everything ok? Easy to conflate them

layers/core_checks/cc_render_pass.cpp Outdated Show resolved Hide resolved
layers/core_checks/cc_render_pass.cpp Outdated Show resolved Hide resolved
layers/core_checks/cc_render_pass.cpp Outdated Show resolved Hide resolved
@spencer-lunarg
Copy link
Contributor Author

spencer-lunarg commented Jan 6, 2025 •
edited
Loading

There is a mixture of plain continue; and ASSERT_AND_CONTINUE, everything ok? Easy to conflate them

the goal was to clarify it... it is valid to have imageView be VK_NULL_HANDLE, but it is NOT valid to have a garbage imageView handle

going

if (color_attachment.imageView == VK_NULL_HANDLE) continue;
auto image_view_state = Get<vvl::ImageView>(color_attachment.imageView);
ASSERT_AND_CONTINUE(image_view_state);

should make it clear if things are null, skip, else we can rely the Get call will be valid

@spencer-lunarg spencer-lunarg reopened this Jan 6, 2025
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 338400.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18541 running.

@spencer-lunarg spencer-lunarg force-pushed the spencer-lunarg-begin-rendering-fix branch from d1674d0 to 40bd589 Compare January 6, 2025 19:08
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 338471.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18544 running.

@spencer-lunarg spencer-lunarg force-pushed the spencer-lunarg-begin-rendering-fix branch from 40bd589 to 9a21fe1 Compare January 6, 2025 19:10
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 338502.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18546 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18546 passed.

@spencer-lunarg spencer-lunarg merged commit e6921b3 into KhronosGroup:main Jan 6, 2025
21 checks passed
@spencer-lunarg spencer-lunarg deleted the spencer-lunarg-begin-rendering-fix branch January 6, 2025 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arno-lunarg arno-lunarg arno-lunarg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spencer-lunarg @ci-tester-lunarg @arno-lunarg