vsg-dev / VulkanSceneGraph

Vulkan & C++17 based Scene Graph Project
http://www.vulkanscenegraph.org
MIT License
1.32k stars 212 forks source link

vsg::Object cast to vsg::Visitor doesn't work when instance is vsg::Trackball #1089

Closed Mikalai closed 8 months ago

Mikalai commented 8 months ago

Describe the bug Hello, I've encountered a strange behaviour of the cast method on Visual Studio 2022 with vsg::Trackball. vsg::Trackball is inherited from vsg::Visitor, thus I expected that if trackball is cast to vsg::Object and that object is cast to vsg::Visitor then final result should be some valid reference. But it is null. Reviewing the code revealed that vsg::Visitor doesn't have required methods to perform correct cast. Not sure if this is expected that vsg::Visitor doesn't have is_compatible method, but without it cast doesn't work. When added to vsg::Visitor method

bool is_compatible(const std::type_info& type) const noexcept override { return typeid(Visitor) == type || Object::is_compatible(type); }

the cast worked fine.

To Reproduce Steps to reproduce the behavior:

#include <cassert>
#include <vsg/all.h>

int main() {
    auto proj = vsg::Perspective::create(30, 1, 0.1, 100.0);
    auto view = vsg::LookAt::create(vsg::dvec3(10, 10, 10), vsg::dvec3(0, 0, 0), vsg::dvec3(0, 0, 1));
    auto camera = vsg::Camera::create(proj, view);
    auto trackball = vsg::Trackball::create(camera);

    auto obj = trackball.cast<vsg::Object>();
    auto vis = obj.cast<vsg::Visitor>();

    assert(vis);
}

Expected behavior vis should not be nullptr

Desktop (please complete the following information):

robertosfield commented 8 months ago

This looks like a bug, I'm in the middle of some major VSG work right now that I plan to merge with VSG today so can't dive off on other topics right away so will have to come back looking at the cause.

What version of the VSG are you using?

Is there are particular tasks that this bug is causing problems with? I ask this as this combination of casting is not what I would expect users to do.

Mikalai commented 8 months ago

I'm using master branch. I was experimenting with C#/WPF application with integrated vsg window over HwndHost. And was exposing some vsg objects (including vsg::Trackball) to C# casting them to vsg::Object before returning the pointer. Thus I was able to cast Trackball to object and pass this pointer to C#, but later when this pointer was received from C# to native code the cast to Visitor failed. These all just experiments and can be workaround in multiple ways (for example dynamic_cast), but thought probably this cast still should work to be consistent with other vsg parts.

robertosfield commented 8 months ago

Have you tried casting to vsg::Trackball instead of vsg::Visitor?

This is just another datapoint that might help shine a light on what might going wrong & where.

Mikalai commented 8 months ago

Yes, it works fine. I guess because Trackball is inherited from vsg::Inherit and vsg::Inherit has

bool is_compatible(const std::type_info& type) const noexcept override { return typeid(Subclass) == type || ParentClass::is_compatible(type); }

Which can forward request to ParentClass. But vsg::Visitor is inherited directly from vsg::Object thus there is no implementation of is_compatible and vsg::Visitor is skipped in that recursive chain.

robertosfield commented 8 months ago

Ahhh, that's almost certainly it, I'll need to add is_compatible into Visitor and ConstVisitor. If you fancy try this and generating a PR then go for it :-)

Mikalai commented 8 months ago

No problem, currently a bit busy will be able to do that in the evening.