From e07a87fd67b5f6e9990aa105fa625bba6c45358c Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Wed, 26 Nov 2025 18:53:18 +0000 Subject: [PATCH 1/2] Allow specialisation of vsg::Inherit for sublasses This allows default implementations of members to be provided, prevents copying and pasting of lots of trivial functions, and makes it possible to generate a compiler error if a subclass fails to provide a particular member without making it a pure virtual function. This is a pretty close equivalent to how in the OSG, there was a META_Object macro that did roughly what the VSG's Inherit template does, but also there were specialised macros like META_Node and META_StateAttribute to do extra things for particular kinds of object. I've only used this for vsg::ArrayState so far as that was the place where the problem came up in my work (I needed to add extra cloneArrayState overloads and ensure subclasses overrode them). I think it's a fairly safe bet that there are other places where using this would be helpful, as I know I've been annoyed by the lack of such a feature in the past, although I can't remember when or why. This isn't the prettiest code in the world, and has a bonus extra annoying feature that it makes the Visual Studio debugger add an extra level of indentation to loads of things, but it's the best that can be done until P3469 https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3469r0.pdf is ratified and becomes part of C++ (it's too late to get into C++26) and finally makes CRTP fully obsolete. --- include/vsg/core/Inherit.h | 11 +++++- include/vsg/state/ArrayState.h | 35 ++++++++--------- src/vsg/state/ArrayState.cpp | 61 ----------------------------- src/vsg/text/CpuLayoutTechnique.cpp | 5 +-- src/vsg/text/GpuLayoutTechnique.cpp | 5 +-- 5 files changed, 28 insertions(+), 89 deletions(-) diff --git a/include/vsg/core/Inherit.h b/include/vsg/core/Inherit.h index 1278a5f17e..56f2ec66a8 100644 --- a/include/vsg/core/Inherit.h +++ b/include/vsg/core/Inherit.h @@ -23,12 +23,13 @@ namespace vsg /// Inherit<> uses the Curiously Recurring Template Pattern /// to provide the classes versions of create, accept(..), RTTI and sizeofObject() + /// There's a layer of indirection to allow subclass-specific specialisation without copying and pasting everything template - class Inherit : public ParentClass + class InheritBase : public ParentClass { public: template - Inherit(Args&&... args) : + InheritBase(Args&&... args) : ParentClass(std::forward(args)...) {} template @@ -72,4 +73,10 @@ namespace vsg void accept(RecordTraversal& visitor) const override { visitor.apply(static_cast(*this)); } }; + template + class Inherit : public InheritBase + { + using InheritBase::InheritBase; + }; + } // namespace vsg diff --git a/include/vsg/state/ArrayState.h b/include/vsg/state/ArrayState.h index 4c4897495c..bba0756066 100644 --- a/include/vsg/state/ArrayState.h +++ b/include/vsg/state/ArrayState.h @@ -96,6 +96,23 @@ namespace vsg }; VSG_type_name(vsg::ArrayState); + template + class Inherit>> : public InheritBase + { + public: + using InheritBase::InheritBase; + + ref_ptr cloneArrayState() override + { + return Subclass::create(static_cast(*this)); + } + + ref_ptr cloneArrayState(ref_ptr arrayState) override + { + return Subclass::create(static_cast(*arrayState)); + } + }; + /// NullArrayState provides a mechanism for geometry in a subgraph to be ignored by traversals that use ArrayState such as ComputeBounds/Intersector/LineSegmentIntersector /// this is useful for subgraphs that have custom shaders that move the final rendered geometry to a different place that would be naively interpreted by a straight forward vec3Array vertex array in local coordinates. /// To disable the handling of geometry in a subgraph simply assign a NullArrayState to the StateGroup::prototypeArrayState, i.e. @@ -106,9 +123,6 @@ namespace vsg NullArrayState(); explicit NullArrayState(const ArrayState& as); - ref_ptr cloneArrayState() override; - ref_ptr cloneArrayState(ref_ptr arrayState) override; - using ArrayState::apply; void apply(const vec3Array&) override; @@ -124,9 +138,6 @@ namespace vsg TranslationArrayState(const TranslationArrayState& rhs); explicit TranslationArrayState(const ArrayState& rhs); - ref_ptr cloneArrayState() override; - ref_ptr cloneArrayState(ref_ptr arrayState) override; - uint32_t translation_attribute_location = 7; AttributeDetails translationAttribute; @@ -145,9 +156,6 @@ namespace vsg TranslationRotationScaleArrayState(const TranslationRotationScaleArrayState& rhs); explicit TranslationRotationScaleArrayState(const ArrayState& rhs); - ref_ptr cloneArrayState() override; - ref_ptr cloneArrayState(ref_ptr arrayState) override; - uint32_t translation_attribute_location = 7; uint32_t rotation_attribute_location = 8; uint32_t scale_attribute_location = 9; @@ -170,9 +178,6 @@ namespace vsg DisplacementMapArrayState(const DisplacementMapArrayState& rhs); explicit DisplacementMapArrayState(const ArrayState& rhs); - ref_ptr cloneArrayState() override; - ref_ptr cloneArrayState(ref_ptr arrayState) override; - // binding of displacement map uint32_t normal_attribute_location = 1; uint32_t texcoord_attribute_location = 2; @@ -207,9 +212,6 @@ namespace vsg uint32_t translation_attribute_location = 7; AttributeDetails translationAttribute; - ref_ptr cloneArrayState() override; - ref_ptr cloneArrayState(ref_ptr arrayState) override; - void apply(const VertexInputState& vas) override; ref_ptr vertexArray(uint32_t instanceIndex) override; }; @@ -223,9 +225,6 @@ namespace vsg BillboardArrayState(const BillboardArrayState& rhs); explicit BillboardArrayState(const ArrayState& rhs); - ref_ptr cloneArrayState() override; - ref_ptr cloneArrayState(ref_ptr arrayState) override; - uint32_t translation_attribute_location = 7; AttributeDetails translationAttribute; diff --git a/src/vsg/state/ArrayState.cpp b/src/vsg/state/ArrayState.cpp index 1a08fe9dba..0c37cf3fd3 100644 --- a/src/vsg/state/ArrayState.cpp +++ b/src/vsg/state/ArrayState.cpp @@ -219,17 +219,6 @@ NullArrayState::NullArrayState(const ArrayState& as) : vertices = {}; } -ref_ptr NullArrayState::cloneArrayState() -{ - return NullArrayState::create(*this); -} - -// clone the specified ArrayState -ref_ptr NullArrayState::cloneArrayState(ref_ptr arrayState) -{ - return NullArrayState::create(*arrayState); -} - void NullArrayState::apply(const vsg::vec3Array&) { vertices = {}; @@ -260,16 +249,6 @@ TranslationArrayState::TranslationArrayState(const ArrayState& rhs) : { } -ref_ptr TranslationArrayState::cloneArrayState() -{ - return TranslationArrayState::create(*this); -} - -ref_ptr TranslationArrayState::cloneArrayState(ref_ptr arrayState) -{ - return TranslationArrayState::create(*arrayState); -} - void TranslationArrayState::apply(const VertexInputState& vas) { getAttributeDetails(vas, vertex_attribute_location, vertexAttribute); @@ -315,16 +294,6 @@ TranslationRotationScaleArrayState::TranslationRotationScaleArrayState(const Arr { } -ref_ptr TranslationRotationScaleArrayState::cloneArrayState() -{ - return TranslationRotationScaleArrayState::create(*this); -} - -ref_ptr TranslationRotationScaleArrayState::cloneArrayState(ref_ptr arrayState) -{ - return TranslationRotationScaleArrayState::create(*arrayState); -} - void TranslationRotationScaleArrayState::apply(const VertexInputState& vas) { getAttributeDetails(vas, vertex_attribute_location, vertexAttribute); @@ -378,16 +347,6 @@ DisplacementMapArrayState::DisplacementMapArrayState(const ArrayState& rhs) : { } -ref_ptr DisplacementMapArrayState::cloneArrayState() -{ - return DisplacementMapArrayState::create(*this); -} - -ref_ptr DisplacementMapArrayState::cloneArrayState(ref_ptr arrayState) -{ - return DisplacementMapArrayState::create(*arrayState); -} - void DisplacementMapArrayState::apply(const DescriptorImage& di) { if (!di.imageInfoList.empty()) @@ -485,16 +444,6 @@ TranslationAndDisplacementMapArrayState::TranslationAndDisplacementMapArrayState { } -ref_ptr TranslationAndDisplacementMapArrayState::cloneArrayState() -{ - return TranslationAndDisplacementMapArrayState::create(*this); -} - -ref_ptr TranslationAndDisplacementMapArrayState::cloneArrayState(ref_ptr arrayState) -{ - return TranslationAndDisplacementMapArrayState::create(*arrayState); -} - void TranslationAndDisplacementMapArrayState::apply(const VertexInputState& vas) { getAttributeDetails(vas, vertex_attribute_location, vertexAttribute); @@ -562,16 +511,6 @@ BillboardArrayState::BillboardArrayState(const ArrayState& rhs) : { } -ref_ptr BillboardArrayState::cloneArrayState() -{ - return BillboardArrayState::create(*this); -} - -ref_ptr BillboardArrayState::cloneArrayState(ref_ptr arrayState) -{ - return BillboardArrayState::create(*arrayState); -} - void BillboardArrayState::apply(const VertexInputState& vas) { getAttributeDetails(vas, vertex_attribute_location, vertexAttribute); diff --git a/src/vsg/text/CpuLayoutTechnique.cpp b/src/vsg/text/CpuLayoutTechnique.cpp index 09db212c5a..975b4f4fa3 100644 --- a/src/vsg/text/CpuLayoutTechnique.cpp +++ b/src/vsg/text/CpuLayoutTechnique.cpp @@ -47,10 +47,7 @@ class VSG_DECLSPEC CpuLayoutTechniqueArrayState : public Inherit cloneArrayState() override - { - return CpuLayoutTechniqueArrayState::create(*this); - } + using Inherit::cloneArrayState; ref_ptr cloneArrayState(ref_ptr arrayState) override { diff --git a/src/vsg/text/GpuLayoutTechnique.cpp b/src/vsg/text/GpuLayoutTechnique.cpp index 387ac72de2..378b5a4a66 100644 --- a/src/vsg/text/GpuLayoutTechnique.cpp +++ b/src/vsg/text/GpuLayoutTechnique.cpp @@ -51,10 +51,7 @@ class VSG_DECLSPEC GpuLayoutTechniqueArrayState : public Inherit cloneArrayState() override - { - return GpuLayoutTechniqueArrayState::create(*this); - } + using Inherit::cloneArrayState; ref_ptr cloneArrayState(ref_ptr arrayState) override { From 6551c52c50660826f3b90e4623c0360c78ab193b Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Thu, 27 Nov 2025 00:07:36 +0000 Subject: [PATCH 2/2] Fix compilation as DLL with MSVC MSVC apparently got confused by the mixed approaches for inheriting constructors, so switched InheritBase to a using declaration as that's the simpler syntax. This then upset GCC because 'an inherited constructor is not a candidate for initialization from an expression of the same or derived type', so the explicit from-ArrayState constructors required for cloneArrayState stopped being available until they were explicitly reimplemented, and then that meant that DisplacementMapArrayState had to expose it with the copyop. --- include/vsg/core/Inherit.h | 4 +--- include/vsg/state/ArrayState.h | 6 +++++- src/vsg/state/ArrayState.cpp | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/vsg/core/Inherit.h b/include/vsg/core/Inherit.h index 56f2ec66a8..7008c88f8f 100644 --- a/include/vsg/core/Inherit.h +++ b/include/vsg/core/Inherit.h @@ -28,9 +28,7 @@ namespace vsg class InheritBase : public ParentClass { public: - template - InheritBase(Args&&... args) : - ParentClass(std::forward(args)...) {} + using ParentClass::ParentClass; template static ref_ptr create(Args&&... args) diff --git a/include/vsg/state/ArrayState.h b/include/vsg/state/ArrayState.h index bba0756066..86daa4d4b6 100644 --- a/include/vsg/state/ArrayState.h +++ b/include/vsg/state/ArrayState.h @@ -102,6 +102,10 @@ namespace vsg public: using InheritBase::InheritBase; + explicit Inherit(const ArrayState& rhs, const CopyOp& copyop = {}) : + InheritBase(rhs, copyop) + {} + ref_ptr cloneArrayState() override { return Subclass::create(static_cast(*this)); @@ -176,7 +180,7 @@ namespace vsg public: DisplacementMapArrayState(); DisplacementMapArrayState(const DisplacementMapArrayState& rhs); - explicit DisplacementMapArrayState(const ArrayState& rhs); + explicit DisplacementMapArrayState(const ArrayState& rhs, const CopyOp& copyop = {}); // binding of displacement map uint32_t normal_attribute_location = 1; diff --git a/src/vsg/state/ArrayState.cpp b/src/vsg/state/ArrayState.cpp index 0c37cf3fd3..c9dc405b2d 100644 --- a/src/vsg/state/ArrayState.cpp +++ b/src/vsg/state/ArrayState.cpp @@ -342,8 +342,8 @@ DisplacementMapArrayState::DisplacementMapArrayState(const DisplacementMapArrayS { } -DisplacementMapArrayState::DisplacementMapArrayState(const ArrayState& rhs) : - Inherit(rhs) +DisplacementMapArrayState::DisplacementMapArrayState(const ArrayState& rhs, const CopyOp& copyop ) : + Inherit(rhs, copyop) { }