Skip to content
This repository has been archived by the owner on Jul 19, 2018. It is now read-only.

WIP: layers: Use CB invalidation mechanism for VU135. #1789

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chrisforbes
Copy link
Collaborator

Makes more sense to use command buffer invalidation to drive this than
maintaining parallel logic and only picking this up at draw time.

Drops some additional TODOs in around areas that need some more work to
make this nice.

Note: Looking for feedback here, this isn't ready -- we make the validation message in this case much less clear by this change.

@chrisforbes chrisforbes requested a review from tobine May 19, 2017 16:59
@@ -9550,6 +9540,12 @@ VKAPI_ATTR void VKAPI_CALL CmdExecuteCommands(VkCommandBuffer commandBuffer, uin
pCommandBuffers[i], pCB->commandBuffer);
pCB->beginInfo.flags &= ~VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT;
}
if (pSubCB->linkedCommandBuffers.size()) {
// TODO: adjust message support so we can report an accurate error message for this.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, change looks good and fulfills the old TODO but currently killing VAL_ERR_00135 which is in Command Buffer Submission portion of spec. Need to preserve that check. It's similar to 133, which is handled in validateCommandBufferSimultaneousUse() just before the killed 135 check. Not immediately obvious to me the best way to preserve 135.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From private conversation with @tobine: may be better to remove this VU from the spec as it's redundant with the 'causes the command buffer to become invalid' language.

@mark-lunarg
Copy link
Collaborator

This looks fine, though I have nothing constructive to add just now.

Makes more sense to use command buffer invalidation to drive this than
maintaining parallel logic and only picking this up at draw time.

Drops some additional TODOs in around areas that need some more work to
make this nice.
@chrisforbes
Copy link
Collaborator Author

Need to rework this a bit for the cleaned up spec language.

@mark-lunarg
Copy link
Collaborator

What needs to be done here -- are the spec concerns still valid?

@mark-lunarg
Copy link
Collaborator

@chrisforbes, this repository will close for write access on Sunday, 5/13/2018. If it is pushed before that time it will be present in the follow-on Vulkan-ValidationLayers repository on Monday, otherwise a new PR will be required in the new repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants