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

layers: Move state tracker methods to CMD_BUFFER_STATE #3149

Merged

Conversation

jeremyg-lunarg
Copy link
Contributor

Moving these methods requires a back pointer to the state
tracker in the command buffer objects for doing handle lookups.

Moving these methods requires a back pointer to the state
tracker in the command buffer objects for doing handle lookups.
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 17169.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 4341 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 4341 passed.

@jeremyg-lunarg
Copy link
Contributor Author

FYI @nadavgevaAMD

Copy link
Contributor

@ncesario-lunarg ncesario-lunarg left a comment

Choose a reason for hiding this comment

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

Didn't look terribly close, but LGTM!

}

void CMD_BUFFER_STATE::Destroy() {
// Allow any derived class to clean up command buffer state
if (dev_data->command_buffer_reset_callback) {
(*dev_data->command_buffer_reset_callback)(commandBuffer());
Copy link
Contributor

Choose a reason for hiding this comment

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

Any luck with removing these callbacks from state tracker? Or perhaps it doesn't make sense to do so?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is coming in a follow on PR. I have it working in a local build.

@jzulauf-lunarg
Copy link
Contributor

LGTM -- though with code movement changes, hard to assess. Recommend additional testing (CTS/perf traces).

@jeremyg-lunarg
Copy link
Contributor Author

This went through perf and error count vvltraces runs last night without regressions.

@jeremyg-lunarg jeremyg-lunarg merged commit 1ec8933 into KhronosGroup:master Aug 13, 2021
@jeremyg-lunarg jeremyg-lunarg deleted the jeremyg-cmd-state branch August 13, 2021 17:54
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.

None yet

4 participants