Improvements to dynamic vsg::Data support

[Robert Osfield]

Hi All,

In early August a pull request arrived that provided a novel extensions to the VSG's DescriptorBuffer support to provide multi-buffering to avoid updates overwriting data that is being rendered by a previously submitted frames rendering:

   https://github.com/vsg-dev/VulkanSceneGraph/pull/497

I've sat on this PR as while it solved a problem it adds a bit of complexity (two extra classes) and require extra knowledge from user, and decided to see if I could find an alternative approach that was simpler and wider applicability.

The first branch of this work was pretty straight forward - to add Vulkan's dynamic descriptor buffer support directly into vsg::DescriptorBuffer/BindDescriptorSet(s), this is a useful Vulkan feature that was missing, but ironcally for this thread it's not really for dynamic data, but just a way of reusing vkDescritors/Sets so you don't need to use so many.  This block of work was checked in 18 days ago:

    https://github.com/vsg-dev/VulkanSceneGraph/pull/510

Normally I'd put together an example to test and illustrate it but as I have broken arm have had keep work pretty minimal, so you'll have to make do with Sasha Williams Vulkan example using this vk feature:

    https://github.com/SaschaWillems/Vulkan/tree/master/examples/dynamicuniformbuffer

If folks fancy porting this to the VSG then please do :-)

The next branch of work was to look the essence of the problem of handling data that is updated on the CPU and needs to be copied to the GPU in a way that avoids writing and reading data across frames, i.e updates from frame i don't overwrite the data being rendered by frame i-1.

One way of tackling this problem is to put a wait on fence in the main frame loop so the CPU thread doesn't get too far ahead of the rendering on the GPU.  The vsgclip example used this approach, but this adds a little complexity and slows the frame rate as you are explicitly stopping the multi-threaded capability of having separate CPU and GPU cores.

The other approach is do multi-buffering, this is typically more complicated to implement, the PR that kicked off this work takes this approach. For performance it's better, but I felt there was a "more general approach" that could be done by the core VSG to provide this functionality.

Exactly what this "more general approach" wasn't obvious so it took me several goes at designing different approaches, I tried implements bits but for several weeks nothing gelled so I'd get a few days in then decide it wasn't good enough.  

Only have full use of my left hand is part of why I haven't converged to a solution more quickly.  Even brain storming with pen and paper was slow going as writing with the wrong hand takes such an effort it rather blocks free flow of ideas.

About a week an approach that might work well started to gel and the implementation while very slow still did steadily build something concrete and coherent enough to the fulfil the intangible sense of a design and implementation being "right". 

Late yesterday I got it working with a modified vsgclip and vsgcomputevertex example, and today I added vsgdynamictexture_cs to use the new approach.  I have checked the required changes into the VulkanSceneGraph and vsgExample DynamicData branches:

    https://github.com/vsg-dev/VulkanSceneGraph/tree/DynamicData

    https://github.com/vsg-dev/vsgExamples/tree/DynamicData

Github compare against vsgExamples master reports:

    Showing with 13 additions and 84 deletions.

So lots of code removed and a small addition, this means it's much simpler for end users to now implement dynamically update data associated with vsg::DescriptorBuffer and vertex arrays associated with vsg::Geoemetry/VertexIndexDraw/BinVertexBuffers/BindIndexBuffers

If you want to update vsg::Data dynamically you now have to add a data->getLayout().dynamic flag to true, prior to call viewer::compile() so that when the rendering backend (RecordAndSubmitTask) is configured it can record all the vsg::BufferInfo that have dynamic vsg::Data.

Then during the Viewer::recordAndSubmit() call all the dynamic BufferInfo vsg::Data that has it's modifiedCount updated since last copy is copied to a staging buffer and then a set of vkCmdCopyBuffer are then recorded to dedicated vkCommandBuffer and submitted prior to the main rendering submission with the later waiting on a semaphore singled by the transfer submission.  The staging buffer and associated resources are mulit-buffered to address the issue of write and read data collisions.  All this complexity is handled for you so no application level work required.

All you need to is update your vsg::Data and call data->dirty() to update the modificationCount.  Have a look at the changes to vsgExamples linked above to see it in action.  For example vsgcomputevertex in scene graph set up does:

   // setting we pass to the compute shader each frame to provide animation of vertex data.
    auto computeScale = vsg::vec4Array::create(1);
    computeScale->getLayout().dynamic = true;
    computeScale->set(0, vsg::vec4(1.0, 1.0, 1.0, 0.0));

And then in main frame loop:

       double frameTime = std::chrono::duration<float, std::chrono::seconds::period>(viewer->getFrameStamp()->time - viewer->start_point()).count();
        computeScale->at(0).z = sin(frameTime);
        computeScale->dirty();

The computeScale->getLayout().dynamic = true; and computeScale->dirty(); are the only additional code you need to make sure that data will get synced.

This new mechanism means you want nee to explicitly copy the data yourself - so no need for a vsg::CopyAndReleaseBuffer node and all the extras that surround it's use, which is where all the code deletions were achieved with the vsgExamples changes.

I need to stop now as my wrist is protesting.

I still more refinement work to do, and some variable/class renaming such vsg::Data::Layout and it's new bool dynamic.  The overall approach I'm am happy with so worth wider community testing before I consider merging with master.

So please dive in with testing.

Cheers,

Robert.

[bret curtis]

Thanks for the right up, this really helps certain fledgling projects that I have an eye on.

Taken care of that wrist! 

Cheers,

Bret

--
You received this message because you are subscribed to the Google Groups "vsg-users : VulkanSceneGraph Developer Discussion Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to vsg-users+...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/vsg-users/124fa293-4b23-4f72-855d-09f6a7b18421n%40googlegroups.com.

[Robert Osfield]

Hi All,,

While I've been quiet on this group I've kept chipping away at the dynamic data support, far off my usual rate, but still productive enough to make real progress. 

The DynamicData branch of the VSG of now efficiently batch transfers of dynamic vsg::Data and passes them automatically each from to their corresponding vsg/vkBuffer on the GPU , and automatically prunes dynamic data list when subgraphs that reference those BufferInfo are no longer required.  The vsg::Data also only transfers data when it needs to when the data is dirtied by the data->dirty() call, so if you only occasionally update the data you will not pay the cost of transfer.  The scheme works for vsg::Data used in conjunction with vsg/vkDescriptorBuffer as well as vertex and index arrays associated with render data.

To help with testing I have added a vsgdynamicvertex example, to vsgExample DynamicData branch, that is loads a user specified model then optionally finds all vertex arrays in the loaded model, sets the vsg::Data::Layout.dirty flag to true, then calls dirty on each frame, and modifies the vertex data with a simple sin function to animate the vertices.  This example also provide some scene graph and frame rate stats to help with performance testing as I make changes to the backend implementation. i.e.

$ vsgdynamicvertex bigcity.vsgb --dirty --st -f 1000       
info: number of dynamic vertex arrays : 6635
info: number of dynamic vertices : 2857067
info: size of dynamic vertices : 34284804
Average fps = 154.241
Average transfer speed 5043.13 Mb/sec

The average transfer speed is overly approximate as it's based on the average frame time rather than just the cost of transfer time in each frame.  At this point I'm not trying to provide full example with all the figures exact, but give me some thing I can get some basic results to help guide the development of the backend.  However, I think it's still useful enough at this point for others to review the code to see how straight forward it is now to update vertex/normal/texcoord/index and uniform data.

The other useful tests with vsgdynamicvertex example are:

$ vsgdynamicvertex models/lz.vsgt                   # just load and render model, no vertex data is made dynamic, so gives you a base line

$ vsgdynamicvertex models/lz.vsgt --dynamic # load and render model, all vertex data is set to dynamic, but nothing is change per frame

$ vsgdynamicvertex models/lz.vsgt --dirty        # as --dynamic but adds a data->dirty() call to all scene graph vertex data every frame. so it transfers all vertex data on every frame

$ vsgdynamicvertex models/lz.vsgt --modify     # as --dirty but modification of all vertex.z values using a simple simple (but slow) sin function, just done to illustrate copying of data has worked.

I am now comfortable enough with the public API and the implementation to commit to this body of work is no longer experimental and will become part of VSG master and then subsequently VSG-1.0.  The work isn't complete yet, so I still have features and refinements I am planning to make over the coming week.  This means there is plenty of time for members of the community to dive in a review/test the code and help chip in ideas.

Items I have pencilled in are:

1. multi-threading
   
2. more flexible high level management to allow record and submission to be more finely controlled by app devs
3. how to handle one frame updates
4. integration of image updates to make vkImageView/vkImage udaptes as straight forward as it is now for vsg::Data
5. refining the vsg::Data;;Layout usage

If you have any thoughts/suggestions please chip in.

Cheers,

Robert.

[Robert Osfield]

Hi All,

I've been 3 weeks since I did serious work on the DynamicData branch,  during this period I've been tackling client work (all open sourced already, or will be :-), this is all wrapped up now so I've jumped back into DynamicData work. 

3 weeks ago I had vsg::Data associated with vertex arrays and uniforms (vsg::DecirptorBuffer) working well, and decide that the new API front and back end implementation were so much neater and more efficient that I had to add support for automatically updating vsg::Data associated with textures ( (vsg::DesriptorImage/ImageView/Image) using the same public API and back end implementation.  I've now made good progress with this and have been able to get on testing.

To aid with testing I've modified the vsgExamples vsgdynamictexture application to support a --dynamic command line option that selects the new API/backend in place of the original vsg::CopyAndReleaseImage, so can now report some performance tests, with RGBA that can be copied directly, and RGB data that has to be expanded to RGBA before copying to GPU memory.  First up let's compare RGBA with old and new dynamic:

$vsgdynamictexture -t -f 100000          
Average frame rate = 4853.57

$ vsgdynamictexture --dynamic -t -f 100000
Average frame rate = 6608.68

That 36% speed improvement.  All with a public API usage that is far simpler to use :-)

Comparing the on the fly RGB -> RGBA conversion:

$ vsgdynamictexture --rgb -t -f 10000          
Average frame rate = 2008.33

$ vsgdynamictexture --dynamic --rgb -t -f 10000
Average frame rate = 2505.3

Which equates to a 25% speed improvement, less because the bottleneck is more on the CPU based RGB->RGBA conversion code.

This work is yet complete, the approach has the potential for breaking th adding threading of the TransferTask(s) to the Viewer so that transfer of data and recording of the Vulkan copy commands could be done in a parallel to the recording of the main scene graphs RecordAndSubmitTask, so there is potentially for further performance improvements.

Given these performance and usability improvements this work makes the original vsg::CopyAndReleaseBuffer and vsg::CopyAndReleaseImage commands obsolete.  Potentially we could just officially deprecate them and keep them around in 1.0 as baggage for backwards compatibility, but my current inclination is not to include them in 1.0 as they will just lead to substandard applications that still use them. 

For those using vsg::CopyAndReleaseBuffer and vsg::CopyAndReleaseImage a small refactor to use the new API will be required, but it'll simplify code and make it easier to manage so it'll be a win for long term maintenance and performance.  For instance here's the changes to the vsgdynamcitextgure example - 4 new lines of code and 68 removed:

     Changes to vsgdynamictexture

What I'll do is merge DynamicData branch first with VSG master, then I'll create a new branch that removes vsg::CopyAndReleaseBuffer and vsg::CopyAndReleaseImage and then merge that a few days later to give folks some time to adjust their code.

Cheers,

Robert

[Robert Osfield]

Hi All,

I've just checked in a small change to how applications specify that a vsg::Data object is dynamic, instead of a bool vsg::Data::Layout.dynamic member there is now vsg::Data::Layout::dataVariance member, with a vsg::DataVariance enum defined as:

   enum DataVariance : uint8_t
    {
        STATIC_DATA = 0,
        DYNAMIC_DATA = 1
    };

Usage is of the style:

    textureData->getLayout().dataVariance = vsg::DYNAMIC_DATA;

The changes to the VSG and vsgExample DynamicData branches are:

    VulkanSceneGraph DynamicData changes

    vsgExamples DynamicData changes

I've made this change as a stepping stone to adding other DataVariance enums so we can more finely control how the vsg::Data is treated.  For instance I'm going to experiment with having two placements of a vsg::TransferTask, and early one that transfers data that is modified prior to the main recordTraversals of the RecordAndSubmitTask, and a transfer data that is modified ruing the recordTraversals - such as view dependent state.

The motivation for split up the transfer this way is it's likely that most applications will update their data during the update phase of the application prior to the recordAndSubmit, so that transfer of that updated data can happen prior or in parallel to the recordAndSubmit record traversals enabling better parallelization of the work. 

My expectation is that will also be a more modest amount of dynamic data that is computed during the record traversal - such as lighting data, and this data will likely be smaller in size and quicker to transfer so the penalty in running the transfer after the main record will be lower than it would be if this transfer had to including all the other dynamic data that isn't view/record dependent.

Right now the TransferTask does all the dynamic data transfers and is run after the record traversal happens, which is the worst case, but even this worst case is looking pretty efficient - not the big performance improvements I've reported in this thread so far, this improvements are happening even with this potential further optimization.

--

Another change I'm considering is renaming the Data::Layout struct to something like Data::Properties as this struct which tightly packs various Data settings for layout now has a setting for the dataVariance of the data, but this doesn't mean that Layout is varying, but the data itself.

A "using Layout = Property;" within the vsg::Data class declaration should enable old code to keep compiling.

Thoughts on naming of Data::Layout/Properties?

--

It may make sense to merge DynamicData quite soon before I lay on too many other changes as it's already been quite a few weeks sitting out in a branch and it's so much better an approach than what I implemented previously so be good to get everyone using it.

Cheers,

Robert.

[Robert Osfield]

Hi All,

Today I finished up my TODO list for the DynamicData work, implementing the early/late vsg::TransferTask functionality, dirty counts for vsg::ImageInfo that mirrors the one that as already in vsg::BufferInfo and implementing threading support that enables the early TransferTask to run in parallel to the record traversals.

To enable control of the early/late TransferTask functionality I introduced a new DYNAMIC_DATA_TRANSFER_AFTER_RECORD enum entry for vsg::DataVariance that now looks like:

    enum DataVariance : uint8_t
    {
        STATIC_DATA = 0,                       /** treat data as if doesn't not change .*/
        STATIC_DATA_UNREF_AFTER_TRANSFER = 1,  /** unref this vsg::Data after the data has been transferred to the GPU memory .*/
        DYNAMIC_DATA = 2,                      /** data is updated prior to the record traversal and will need transferring to GPU memory.*/
        DYNAMIC_DATA_TRANSFER_AFTER_RECORD = 3 /** data is updated during the record traversal and will need transferring to GPU memory.*/
    };

For now ignore the STATIC_DATA_UNREF_AFTER_TRANSFER enum as it's a thought for future functionality, that will be either be follow up work before 1.0 , or I might just remove it for 1.0 if I don't get around to it.  The general idea is to unref vsg::Data once the associated data is copied to GPU memory in order to save main memory.

The DYNAMIC_DATA enum is used for normal data that is updated during the update phase of the frame, this is ready to copy as soon as the Viewer::recordAndSubmit() method is called so can be copied to the transfer buffer right away by the "early" TransferTask, so can be done either before, or in a parallel with the normal record traversals when you have switch on viewer threading with viewer.setupThreading().  In the later case a dedicated thread per RecordAndSubmitTask is used to run the transfer work and recording of the required Vulkan transfer command required to get the data to GPU memory. 

The DYNAMIC_DATA_TRANSFER_AFTER_RECORD enum is used for data that is computed within the record traversals, such as light source data provided by the vsg::ViewDependentState functionality.  This is then transferred to the GPU via a dedicate "late" TransferTask that is run by the RecordAndSubmitTask::finish() method - this has to be run by the tasks primary thread (if you are multi-threadnig.)

The vsg::Viewer/RecordAndSbmitTask backend does this thread management for you so there is no additional overhead you need to worry about when using dynamic data.

--

The addition of ModifiedCount support in vsg::ImageInfo enables the TransferTask to track when the vsg:::Data associated with it has been updated (you indicate this by calling vsg::Data.dirty() see examples like vsgdynamictexture) and to use this to make sure that data is only copied to the GPU when it's actually modified rather doing it every frame regardless.

--

I have added --mt command line options to the vsgdynamicvertex and vsgdynmamictexture examples to test out the new multi-threading of the TransferTask of for a large synthetic model scene vsgdynamicvertex is 10% faster.   You may see more or less improvement in your own applications - the higher your transfer loads and higher your record traversal cost the greater the value in multi-threading.

I have also modified the vsgtexturearray example to use the DynamicData functionality, which removes the final vsg::CopyAndReleaseImage usage from vsgExamples, again this reduces the amount of code required to setup the dynamic updates and improves performance.  The changes are:

    vsgtexturearray changes

--

From my perspective the API and implementation are now complete, so this would be a good time for members of the community to test out the DynamicData branch.  If there are no problems found I'll merge DynamicData with VSG master tomorrow.

Cheers,

Robert.

[Robert Osfield]

Hi All,

I've completed my reviews of the DynamicData branches, tested on integrated GPU and dedicated GPU graphics cards and everything is behaving well.  As I don't have any issues reported and seen none myself I have merged the DynamicData branches of the VSG and vsgExamples with the respective master.

Changes to the VSG:

    https://github.com/vsg-dev/VulkanSceneGraph/pull/521

Changes to vsgExamples - adding new vsgdynamicvertex example and replacing CopyAndReleaseBuffer/CopyAndreleaseImage usage:

    https://github.com/vsg-dev/vsgExamples/pull/174

This work has taken almost 2 months to wrap up, with a broken arm and COVID throwing in a few extra physical challenges to the intellectual one of how one earth do you provide a simple to use and coherent scheme for passing data from the CPU containers to associated GPU resources, doing so in an efficient and scalable way.

While it's taken some time to complete I am extremely pleased how well the work has turned out, both from how easy it is now to set up dynamic data and how efficiently it's managed by the backend - it's ended up be far easier to work with and performs significantly better than the CopyAndReleaseBuffer/CopyAndrReleaseImage classes that provided the a subset of this functionality before.

Cheers,

Robert.

[Robert Osfield]

I forgot to mention, I've bumped the VSG version to 0.5.7 for the DynamicData work so you can now use that as a package requirement when building your applications.

[Robert Osfield]

Heads up everyone, Rainer has reported a regression seen with examples like vsgdraw that crashes in the vsg::BindIndexBuffer::record(..) that looks to be due to a change in BuferInfo::requiresCopy(..).  I have started looking into this bug but haven't yet resolved it.  I'm wrapping up work for the day now so I will work to resolve it tomorrow morning.  The Issue report is:

    https://github.com/vsg-dev/VulkanSceneGraph/issues/522

[Robert Osfield]

I have got to the bottom of the regression and checked in a fix to VSG master and the DynamicData branch.

    BufferInfo fix