New GLTF API Sample Base Class & GLTF Sample#1473
New GLTF API Sample Base Class & GLTF Sample#1473HaoranXu-ARM wants to merge 7 commits intoKhronosGroup:mainfrom
Conversation
asuessenbach
left a comment
There was a problem hiding this comment.
I think, having some GLTFApiVulkanSample base class would be a good extension to the class hierarchy.
But the responsibilities between GLTFApiVulkanSample and GLTF seems to be a bit fuzzy:
- empty virtual functions introduced with
GLTFApiVulkanSample, but only called inGLTF? - elements (like
present.pipeline.pipeline) that are members inGLTFApiVulkanSample, but created and used inGLTF; and destroyed inGLTFApiVulkanSample.
Maybe more, stopped reviewing at that point.
If we want to have something like GLTFApiVulkanSample, the tasks and responsibilities of that class should be a bit cleaner.
Or maybe we could introduce some GLTF class that provides all those capabilities and could be used by a sample, without introducing another layer above ApiVulkanSample? Would probably make such a class more widely usable.
Fixed the mismatch in site of creation and destroy.
I think the goal of The expected use case will be that developers use the |
Description
Add a new GLTF API Sample base class to the framework, as well as a new GLTF sample instance to demonstrate its usage.
Note
We made the following changes to the framework:
Fixes #848
The sample builds and runs successfully on Windows (NVIDIA) and Android (Mali).
General Checklist:
Please ensure the following points are checked:
Note: The Samples CI runs a number of checks including:
If this PR contains framework changes:
batchcommand line argument to make sure all samples still work properlySample Checklist
If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist: