Nabla icon indicating copy to clipboard operation
Nabla copied to clipboard

Command pool

Open deprilula28 opened this issue 3 years ago • 1 comments

Description

Adds a command pool reset function (#373), i.e. ECF_RESET_COMMAND_BUFFER_BIT is not set when creating the command pool.

Implemented by using a reset counter that increases on each reset, then when checkForCommandPoolReset is called on command buffer (currently done within begin), that counter is checked against the counter of the command buffer, which will cause an "internal reset" if it is higher.

"Internal reset" consists of releasing references captured in VK & GL, and clearing commands on GL.

Testing

Tested using a ECF_NONE command pool for leaking references & commands on GL & VK.

  • [ ] Test reset command buffer just after resetting the parent command pool (and do vice versa)

TODO list:

  • [ ] Reviews

deprilula28 avatar Aug 04 '22 13:08 deprilula28

A couple of issues.

Fix my sphagetti code

The pattern of having to remember to call IGPUCommandPool::something from the override of something in the base class, is incredibly dumb, I apologize.

We should stick to

inline bool something()
{
   stuff;
   
   something_impl();
   
   return more stuff;
}

protected:
virtual void something_impl() = 0;

Turn all methods of ICommandBuffer into pure virtuals

Push the logic into IGPUCommandBuffer, our future CPU commandbuffer will be much more lax.

Hoist all State Transition and flag checking logic to IGPUCommandBuffer

Have the _impl just be void functions.

Hoist releaseResourcesBackToPool to IGPUCommandBuffer and make it a protected inline + a releaseResourcesBackToPool_impl

ALWAYS make it drop reference count stuff (do through the destructors of the tape entries) regardless of the value of releaseResources

Have releaseResources only control whether to free the memory for the entries and whether to call vkResetCommandBuffer with the flag or not.

Do not call releaseReourcesBackToPool if commandpool has been reset already

This will cause a double free, the entries pushed into tapes are already gone (also the head/tail pointers to tape slabs/blocks).

But do m_commands.clear() the OpenGL Commandbuffer

@achalpandeyy will remove m_commands soon anyway, it will be replaced by tail/head pointers to a tape allocated from the command pool's memory pool so these will need to be nullified in the future instead.

Check for commandpool reset BEFORE EVERY method!

Directly related to the preceeding issue.

Set the Last-Reset-Stamp in the begin() not during the resetCommon

What is resetCommon now I'd want to fold into releaseResources, the setting of the stamp should be upon a begin, not upon a reset.

The stamp lets you "catch" pool-resets which have occured since the last time you set the stamp, you only care about new pool resets _since the last time you begun the commandbuffer.