Syncing with changes for up comming VulkanSceneGraph-1.0

[robertosfield]

I've now ticked off the feature complete milestone required for VulaknSceneGraph-1.0 and am now focused on shaking down the interfaces and codebase. I'd like to discuss all the recent changes to the VSG and up coming refinement work to the ray tracing code as these will likely break VulkanPBRT.

[Lachei]

Sure, do you have a list of your ideas/things you already implemented where i can comment on and inform myself about the major changes? Unfortunately I was quite busy and thus not able to keep track of all changes to the vsg. If you want to chat sometime that of course would also be fine with me. I think adopting VulkanPBRT should not be too hard. Especially as we might simply change the source of vsg a bit to fit our needs. Cheers, Josef

[robertosfield]

On Mon, 23 May 2022 at 18:09, Lachei @.***> wrote:

Sure, do you have a list of your ideas/things you already implemented where i can comment on and inform myself about the major changes?

The changes around vsg::ShaderSet are probably the ones that overlap most with modifications that were made locally in your version of the VSG. I am away from my dev system right now, but tomorrow I'll track down the specific classes on the VulkanPBRT side and the vsg::ShaderSet side.

I've just started a review of the existing Ray Tracing code in the VSG. I'd like to modernize it a bit so it's most consistent with the rest of the VSG, and also look at getting it compile with older versions of VulaknSDK before the Khronos extensions were introduced. Android NDK doesn't support a modern enough version to include these extensions.

If you have improvements to the Ray Tracing classes then I would like to be able to roll those in as well so for 1.0 you'll be able to use the VSG 1.0 as is rather than need local mods. Message ID: @.***>

[robertosfield]

@Lachei, my plan for today is focus on the VSG's Khronos Ray Tracing with a goal of getting the VSG to compile against VulkanSDK versions older than 1.2.162 which added the Khronos Ray Tracing functions. Android's latest NDK only supports 1.2.158 so I'll use this as a base and see where I get to.

It'll be useful to look the changes you've made to the VSG's Ray Tracing classes in VulkanPBRT so if there are particular commits that would be a good starting place then this would be helpful.

The other area of interest is the ShaderSet work I did on the VSG as this overlaps with some of the SPIR-V reflection functionality that you previous implemented. It not a 1:1 mapping in any way so it's not a drop in replacement but a while higher level shader composition scheme that has a small part that will be of interest to you. The key header is ShaderSet.h.

The part that you may be able to adopt, rather have your own reflection code is the vsg::AttributeBinding, UniformBinding, PushConstantRange and DefinesArrayState struct used to describe how various features in the shaders map to data structures they need to be bound with at runtime. These are simple structs:

struct VSG_DECLSPEC AttributeBinding
{
    std::string name;
    std::string define;
    uint32_t location = 0;
    VkFormat format = VK_FORMAT_UNDEFINED;
    ref_ptr<Data> data;

    int compare(const AttributeBinding& rhs) const;

    explicit operator bool() const noexcept { return !name.empty(); }
};

struct VSG_DECLSPEC UniformBinding
{
    std::string name;
    std::string define;
    uint32_t set = 0;
    uint32_t binding = 0;
    VkDescriptorType descriptorType = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER;
    uint32_t descriptorCount = 0;
    VkShaderStageFlags stageFlags = 0;
    ref_ptr<Data> data;

    int compare(const UniformBinding& rhs) const;

    explicit operator bool() const noexcept { return !name.empty(); }
};

struct VSG_DECLSPEC PushConstantRange
{
    std::string name;
    std::string define;
    VkPushConstantRange range;

    int compare(const PushConstantRange& rhs) const;
};

struct VSG_DECLSPEC DefinesArrayState
{
    std::vector<std::string> defines;
    ref_ptr<ArrayState> arrayState;

    int compare(const DefinesArrayState& rhs) const;
};

My thought is that you may be able to just adopt these for the shader reflection, and the reading of SPIR-V bindings could be moved out of the VSG into a utility that reads a shader and generates the appropriate vsg::ShaderSet for it.

[robertosfield]

To get a better idea of the divergance of VulaknPBRT's VSG components from the main VSG I've checked out VulkanPBRT and tried a build. I got a build error with vsgXchange due to a stray :, the below patch fixes this. I see this error because the openexr dependency isn't installed on my system yet.

I think it would be useful to add VulkanPBRT openexr ReaderWriter to the main vsgXchange. There isn't any copyright notice on the files though. Could you add Copyright notices?

diff --git a/external/vsgXchange/src/openexr/openexr_fallback.cpp b/external/vsgXchange/src/openexr/openexr_fallback.cpp index d3b9de2..71f5415 100644 --- a/external/vsgXchange/src/openexr/openexr_fallback.cpp +++ b/external/vsgXchange/src/openexr/openexr_fallback.cpp @@ -6,7 +6,7 @@ using namespace vsgXchange; // // openEXR ReaderWriter fallback // -openexr::openexr() : +openexr::openexr() { }

[robertosfield]

I have had bas at adding The VulkanPBR/vsgXchange/openexr ReaderWriter to the main vsgXchange repo, placing it in an OpeEXR branch.

When I build I see lots of warnings which I will attempt to fix. The use of dyanmic_cast<> is also a bit crude so converting the code to use a local ConstVisitor would be appropriate. I'll do this later. I have also added the openexr ReaderWriter to the list of ReaderWriter assigned to the vsgXhange::all ReaderWriter so that when available this will be supported by default.

[robertosfield]

When building openexr ReaderWriter as part of vsgXchange I'm seeing lots of warnings, I will have a bash at resolving these.

/home/robert/Dev/vsgXchange/src/openexr/openexr.cpp: In constructor ‘Array_IStream::Array_IStream(const uint8_t, size_t, const char)’: /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:74:52: warning: declaration of ‘size’ shadows a member of ‘Array_IStream’ [-Wshadow] 74 | Array_IStream (const uint8_t data, size_t size, const char fileName[]): | ~^~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:93:16: note: shadowed declaration is here 93 | size_t size; | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:74:39: warning: declaration of ‘data’ shadows a member of ‘Array_IStream’ [-Wshadow] 74 | Array_IStream (const uint8_t data, size_t size, const char fileName[]): | ~~~^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:92:24: note: shadowed declaration is here 92 | const uint8_t data; | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp: In constructor ‘Array_IStream::Array_IStream(const uint8_t, size_t, const char)’: /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:74:52: warning: declaration of ‘size’ shadows a member of ‘Array_IStream’ [-Wshadow] 74 | Array_IStream (const uint8_t data, size_t size, const char fileName[]): | ~^~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:93:16: note: shadowed declaration is here 93 | size_t size; | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:74:39: warning: declaration of ‘data’ shadows a member of ‘Array_IStream’ [-Wshadow] 74 | Array_IStream (const uint8_t data, size_t size, const char fileName[]): | ~~~^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:92:24: note: shadowed declaration is here 92 | const uint8_t data; | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp: In constructor ‘Array_IStream::Array_IStream(const uint8_t, size_t, const char)’: /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:74:52: warning: declaration of ‘size’ shadows a member of ‘Array_IStream’ [-Wshadow] 74 | Array_IStream (const uint8_t data, size_t size, const char fileName[]): | ~^~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:93:16: note: shadowed declaration is here 93 | size_t size; | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:74:39: warning: declaration of ‘data’ shadows a member of ‘Array_IStream’ [-Wshadow] 74 | Array_IStream (const uint8_t data, size_t size, const char fileName[]): | ~~~^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:92:24: note: shadowed declaration is here 92 | const uint8_t data; | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp: In member function ‘virtual void Array_IStream::seekg(Imath_2_5::Int64)’: /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:86:43: warning: unused parameter ‘pos’ [-Wunused-parameter] 86 | virtual void seekg (Imf::Int64 pos){ | ~~~^~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp: In member function ‘virtual vsg::ref_ptr vsgXchange::openexr::read(const vsg::Path&, vsg::ref_ptr) const’: /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:238:9: warning: unused variable ‘width’ [-Wunused-variable] 238 | int width, height, channels; | ^~~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:238:16: warning: unused variable ‘height’ [-Wunused-variable] 238 | int width, height, channels; | ^~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:238:24: warning: unused variable ‘channels’ [-Wunused-variable] 238 | int width, height, channels; | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp: In member function ‘virtual bool vsgXchange::openexr::write(const vsg::Object, const vsg::Path&, vsg::ref_ptr) const’: /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:287:13: warning: unused variable ‘dataSize’ [-Wunused-variable] 287 | int dataSize = obj->dataSize(); | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:288:13: warning: unused variable ‘space’ [-Wunused-variable] 288 | int space = width height sizeof(float); | ^~~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:289:13: warning: unused variable ‘size’ [-Wunused-variable] 289 | int size = sizeof(obj->data()); | ^~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:290:15: warning: unused variable ‘dat’ [-Wunused-variable] 290 | float dat = obj->data()[0]; | ^~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:265:108: warning: unused parameter ‘options’ [-Wunused-parameter] 265 | bool openexr::write(const vsg::Object object, const vsg::Path& filename, vsg::ref_ptr options) const | ~~~~~~~^~~~~ /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp: In member function ‘virtual bool vsgXchange::openexr::write(const vsg::Object, std::ostream&, vsg::ref_ptr) const’: /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:409:101: warning: unused parameter ‘options’ [-Wunused-parameter] 409 | bool openexr::write(const vsg::Object object, std::ostream& fout, vsg::ref_ptr options) const | ~~~~~~~^~~~~

[robertosfield]

I'm reviewing the vsgXchange/src/openexr/openexr.cpp with a view of refactoring it make better use of the VSG and to unify the code a bit more is quite a bit of duplicated code for the write to filename and stream code paths. An inconsistency I noticed was the method _bool openexr::write(const vsg::Object* object, const vsg::Path& filename, vsg::refptr) const has a line: header.channels().insert("Y", Imf::Channel(Imf::HALF));

While the equivalent line the the bool openexr::write(const vsg::Object* object, std::ostream& fout, vsg::ref_ptr options) const method is:

  header.channels().insert("Z", Imf::Channel(Imf::HALF));

I can't see a reason for a difference, but also don't know what is the correct value - Y or Z? Or neither?

[Lachei]

If you have improvements to the Ray Tracing classes then I would like to be able to roll those in as well so for 1.0

The improvements that i mad to the ray tracing is mainly the implementation of a ray tracing shader binding table to make the binding of the shader stages better. The commit for these can be seen here: https://github.com/Lachei/VulkanPBRT/commits/master/external/vsg/src/vsg/raytracing/RayTracingShaderBindingTable.cpp .

My thought is that you may be able to just adopt these for the shader reflection, and the reading of SPIR-V bindings could be moved out of the VSG into a utility that reads a shader and generates the appropriate vsg::ShaderSet for it.

This looks not too hard to adopt. I think your approach should work.

Could you add Copyright notices?

Will do later today when i am finished with my work (in about 4 hours).

I can't see a reason for a difference, but also don't know what is the correct value - Y or Z? Or neither?

Actually that does matter on the application that should read the images again. In the documentation the example code uses Z i think for single channel, while Blender safes it in Y. Further RenderDoc can also load single values stored in Y but i think had problems with single values in Z. So i switched to Y and forgot the second write method. Also i maybe should have done the writing in an extra method to have only to change one place, but then again time restraints got me to simply copy paste...

Cheers, Josef

[robertosfield]

I have refactored vsgXchange::openexr so it now uses an InitializeHeader and InitialFramebuffer visitors to handle the different vsg::Array2D types in a type safe way. Both the write to file, and write to ostream implementations can use these visitors avoid the code duplication that existed previously, this cuts the openexr.cpp code size by 111. The changes are: vsgXchange/src/openexr/openexr.cpp.

In testing I found that writing RGBA files work fine, but writing RGB didn't succeed, I don't know if this means RGB isn't supported by OpenEXR so I'll look this up. The write to ostream doesn't presently check the options extension hint, I'll review this and likely add a check.

The changes to the OpenEXR ReaderWriter also take into account the changes to vsg::Path, as it's now a class in it's own right rather than a std::string. This does mean that it'll the changes will need a little tweak to work with older versions of the VSG.

Thanks for the clarification on Y vs Z. I chose the Y as the default for the single channel. We could easily pass in a preferred channel name via vsg::Options meta value that could be checked against to decide what to override with.

[Lachei]

Your implementation looks good.

In testing I found that writing RGBA files work fine, but writing RGB didn't succeed, I don't know if this means RGB isn't supported by OpenEXR so I'll look this up

OpenEXR does not really care about the channel names and such. However currently only RGBA and single image loading is really tested. It might be that something on reading out an RGB image goes wrong. Exporting such a file is defenitly not possible, as only 4 compent arrays are supported currently...

I chose the Y as the default for the single channel. We could easily pass in a preferred channel name via vsg::Options meta value that could be checked against to decide what to override with.

Yes good idea. I also already thought about using the vsg::Options but didnt bother in the end as the current version suited my needs already... For the Y: According to the OpenEXR documentation (https://openexr.readthedocs.io/en/latest/ReadingAndWritingImageFiles.html#luminance-chroma-and-gray-scale-images), Y stands for luminance only so it is perfect for greyscale images (single channel images). Z was only used for extra informations in the tutorials...

[robertosfield]

Looking at the OpenEXR API it looks like 1 to 4 channel data should be possible, The current reading code is hardwired to single channel or RGBA I can't see any reason 1,2,3,4 channel images can't be mapped to R, RG, RGB and RGBA respectively so I will have a bash at generalizing the code further.

[robertosfield]

I have refactored the vsgXchange::openexr::read(..) method to avoid code duplication and then added support for reading 2 and 3 channel images.

vsgXchange::openexr changes

When testing the OpnEXR test dataset I'm getting a new of unhandled exceptions being thrown. Tomorrow I'll add try/catch around the code to avoid these bringing down the whole application.

I also need to add support for writing 2 and 3 channel images to complete vsgXchange::openexr, after that it should be good to merge with vsgXchange master.

[Lachei]

Looking already quite good. If you want to use an even more general solution i would suggest reading out all channels and storing them in a map with the channel name as key. Then create a 1/2/3/4 component array according to the cardinality of the map and the type of the channels and then fill the xyzw compontents of the vsg data array in reverse order (so alphabetically decreasing). Like this you atomatically guarantee that for rgba images the r channel comes first, b channel second ..., for yca images you guarantee that the y channel comes first, then c... Also then we would not be bound to the names of the channels at all.

[Lachei]

I just added copyright information to the openexr header. Another thing that might be a useful feature for some is our vsg implementation for VkQueryPools: https://github.com/Lachei/VulkanPBRT/blob/master/external/vsg/src/vsg/state/QueryPool.cpp and https://github.com/Lachei/VulkanPBRT/blob/master/external/vsg/include/vsg/state/QueryPool.h, https://github.com/Lachei/VulkanPBRT/blob/master/external/vsg/include/vsg/commands/ResetQueryPool.h, https://github.com/Lachei/VulkanPBRT/blob/master/external/vsg/include/vsg/commands, https://github.com/Lachei/VulkanPBRT/blob/master/external/vsg/src/vsg/commands/ResetQueryPool.cpp/WriteTimestamp.h, https://github.com/Lachei/VulkanPBRT/blob/master/external/vsg/src/vsg/commands/WriteTimestamp.cpp. Usage of these is then:

            auto commands = vsg::Commands::create();
            query_pool = vsg::QueryPool::create();  // standard init has 1 timestamp place
            query_pool->queryCount = 2;
            auto reset_query = vsg::ResetQueryPool::create(query_pool);
            auto write1 = vsg::WriteTimestamp::create(query_pool, 0, VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR);
            auto write2 = vsg::WriteTimestamp::create(query_pool, 1, VK_PIPELINE_STAGE_RAY_TRACING_SHADER_BIT_KHR);
            commands->addChild(reset_query);
            commands->addChild(write1);
            pbrt_pipeline->add_trace_rays_to_command_graph(commands, push_constants);
            commands->addChild(write2);
            // do stuff and submit commands
            std::vector<uint32_t> timestamps = query_pool->getResults();

They would provide accurate gpu timing which is especially useful when having multiple pipelines being executed one after another withut cpu sync.

[robertosfield]

On Thu, 26 May 2022 at 07:26, Lachei @.***> wrote:

Looking already quite good. If you want to use an even more general solution i would suggest reading out all channels and storing them in a map with the channel name as key. Then create a 1/2/3/4 component array according to the cardinality of the map and the type of the channels and then fill the xyzw compontents of the vsg data array in reverse order (so alphabetically decreasing). Like this you atomatically guarantee that for rgba images the r channel comes first, b channel second ..., for yca images you guarantee that the y channel comes first, then c... Also then we would not be bound to the names of the channels at all.

Interesting idea.

The vsg::Object meta value system uses an optional vsg::Auxilary object to hold a map of string to vsg::Object, so you could encode the channels as separate images that way. However, it's not something that could be used for rendering without repacking within the application. I can see some app might prefer this, but as a default I think package images would be better.

By default I packing the data into usable R, RG, RGB or RGBA would be my preference. Your suggestion of packing in reverse alphabetical order would pack XYZ as BGR which would be a bit counterintuitive. Having a set of mappings of channel combos might make sense.

Do you have examples .exr files that have the various combinations of channel names?

I do however lots of other work to get on with, I've already spent a day on OpenEXR that hadn't planned for.

Message ID: @.***>

[Lachei]

I also just realized the problem with invers alphabetical order for rgb... In openEXR the main formats of images stored are given as:

WRITE_RGBA	red, green, blue, alpha
WRITE_YC	luminance, chroma
WRITE_YCA	luminance, chroma, alpha
WRITE_Y	luminance only
WRITE_YA	luminance, alpha

So supporting these channel names might also be interesting for a general adoption.

[Lachei]

but as a default I think package images would be better.

I would also say that this is definitely the way to go. I also had normal packing in mind and not separate images for different channels. My idea was to first read out the channels into a map with separate vectors for each channel, and then in an composing step interleave these channels. The idea for the inverted alphabetic order was just a first idea to automatically interleave the channels correctly. The returned image then would always be an interleaved image. By doing this you are not bound to the channel names and would get automatically a correctly interleaved image. The problem with XYZ would still stand, the solution with a set of predefined mappings which dictate a specific ordering would be an easy solution as you already suggested

[Lachei]

One thing that might be interesting for the raytracing update is the additional support for AABB's (so not just triangles) and custom intersection shader which were implemented in a fork of VulkanPBRT: https://github.com/ohomburg/VulkanPBRT . If i have time i will check all commits which are relevant to make this work and which also show how to use these things.

[robertosfield]

I have added try/catch blocks to vsgXchange::openexr which handles some cases, but am also getting a segfault inside openexr with a couple of the openexr tests image due toImf_2_5::ScanLineInputFile::readPixels not checking that that the Iml::FrameBuffer isn't correctly matched to a channel. I'll need to dig into why this is happening and address this.

I'll aim to wrap up the OpenEXR work this morning as I have lots of other tasks to get on with.

For the Ray Tracing related modifications that you mention, I'll need to return this these, hopefully next week as it looks like I won't have time this week - the OpenEXR detour has gobbled up the time that I had available.

[robertosfield]

I have completed the work adding handling of invalid files and exceptions so am ready with merge the openexr branch with vsgXchange master. One open issue I haven't tackled yet is a warning:

/home/robert/Dev/vsgXchange/src/openexr/openexr.cpp: In member function ‘virtual void {anonymous}::Array_IStream::seekg(Imath_2_5::Int64)’: /home/robert/Dev/vsgXchange/src/openexr/openexr.cpp:103:39: warning: unused parameter ‘pos’ [-Wunused-parameter] 103 | virtual void seekg(Imf::Int64 pos) | ~~~^~~

Relating to the [line 103 https://github.com/vsg-dev/vsgXchange/blob/OpenEXR/src/openexr/openexr.cpp#L103 ]:

   virtual void seekg(Imf::Int64 pos)
    {
        curPlace = 0;
    };

Is this code what is intended? If so putting a / pos / will be safe way to quieten the warning.

[robertosfield]

I've reviewed the code and feel that the below change is appropriate:

diff --git a/src/openexr/openexr.cpp b/src/openexr/openexr.cpp index 5bb9fee..06fc5ca 100644 --- a/src/openexr/openexr.cpp +++ b/src/openexr/openexr.cpp @@ -102,7 +102,7 @@ namespace }; virtual void seekg(Imf::Int64 pos) {

• curPlace = 0;
• curPlace = pos; }; virtual void clear(){};

[Lachei]

Yes, your changes are appropriate.

[robertosfield]

On Thu, 26 May 2022 at 14:36, Lachei @.***> wrote:

Yes, your changes are appropriate.

That's a relief as I've already merged with vsgXchange master :-)

There are still improvements that could be made, some which we've discussed here, but I'm happy enough with what has been merged as a base for future work, it can be extended when the need arises.

I'm now working on other parts of the VSG, once I'm able to jump back to reviewing the Ray Tracing functionality I'll look at your links and likely have more questions. Ideally I'd like us to get to the point where you don't need to modify the VSG locally for your purposes.

Message ID: @.***>

[Lachei]

Ideally I'd like us to get to the point where you don't need to modify the VSG locally for your purposes.

That would be of course perfect. I still will keep vsg in my project currently via git subtree to be able to get the newest versions and still be able to quickly try out some new implementations.

[robertosfield]

Touching base once more as the VSG is steadily converging towards 1.0.

I will spend some time this afternoon reviews the QueryPool functionality to see if it's appropriate for merging. Is there a commit I should look at that add this functionality?

If there is anything else you'd like to see make it to the VSG before 1.0 then let me know.

[Lachei]

For the Query pools i think copy pasting of the current files needed is the easiest. A list of all files that are needed are here: https://github.com/Lachei/VulkanPBRT/issues/39#issuecomment-1138214261.

Unfortunately i added the functionality while also doing a bunch of other updates, which caused the commit where this got added to be very large...

I will come back if anything jumps to my mind.

[robertosfield]

I've copied the files directly and created a branch from these changes, and assigned you as the original author:

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

I've also done various house keeping changes to the files to keep them more consent with the rest of the VSG.

It's broken the Android build due to vkResetQueryPool not being defined so I'll add an extension definition and check for this.

I also need to decide what do about what should happen when a QueryPool is used within a multidevice setup. It might be that we just need to document that timing info needs to be done above subgraphs that are shared between devices.

Do you have an example program that illustrates use of the vkQueryPool functionality?

[robertosfield]

I have created a vsgquerypool example that puts timing around the main RenderGraph:

examples/commands/vsgquerypool/vsgquerypool.cpp

I still need to background reading on this feature, and fix the build on Android, but as it's end of working day here in Scotland so I'll return to finishing up tomorrow.

[Lachei]

Your example has everything in it that i also have. I think there should be a description for the query_pool->getResults() method that informs the user that the query pool results are retrieved with the VK_QUERY_RESULT_WAIT_BIT set. This means that the getResults() method also acts as a kind of synchronization point.

For conversion of the timestamps to ms you can simply multiply them by 1e-6. This is however not the fixed time conversion factor. To do this platform independent you would have to read out some variable from the physical device properties (however on all devices i tested so far the conversion factor was always correct).

[robertosfield]

Thanks for the info. Tomorrow I'll read up and do more tinkering. I think getting the device properties in the main would be useful example as well.

[robertosfield]

I have replaced the QueryPool::getResults() with the methods that better map to the vkGetQueryPoolResults features:

Added more complete support for getting 32 and 64 bit results.

    VkResult getResults(std::vector<uint32_t>& results, uint32_t firstQuery  = 0, VkQueryResultFlags resultsFlags = VK_QUERY_RESULT_WAIT_BIT) const;
    VkResult getResults(std::vector<uint64_t>& results, uint32_t firstQuery  = 0, VkQueryResultFlags resultsFlags = VK_QUERY_RESULT_WAIT_BIT | VK_QUERY_RESULT_64_BIT) const;

The usage is now in form:

    std::vector<uint64_t> timestamps(2);
    if (query_pool->getResults(timestamps) == VK_SUCCESS)
    {
        auto delta = timestamps[1] - timestamps[0];
        std::cout<<"delta = "<<delta<<std::endl;
    }

I will now look up the device properties.

[robertosfield]

Touching base, VulkanSceneGraph-1.0 is very nearly ready, my plan is tag the 1.0.0 these at the end of October, possible this coming Sunday.

I noticed that there hasn't been recent commits of VulkanPBRT master. Does this mean the project is mothballed for now?

[Lachei]

Hello Robert, currently the work on VulkanPBRT is on hold, yes. I might come back to developing things further here, however I am currently working on other projects which demand my full attention and will take up a few months to finish. I will try to merge in all the changes (which look very promising), however am not sure when i will have time/will take the time to do so. Best, Josef