Partage
  • Partager sur Facebook
  • Partager sur Twitter

Problème de design : double call pour le visiteur

2 septembre 2018 à 20:39:16

Bonjour,

Toujours sur l'hypothèse où je travaille sur un scene graph.

Nous avons une classe Node, dont le seul but est d'être visité. Nous avons aussi une classe Group, qui contient plusieurs Node, qui dérive de Node.
Nous avons une classe ActivableNode qui est un Group dont le but est d'être activé afin d'afficher ou non les nodes sous jacentes.
Nous avons une classe TransformNode qui est aussi un Group qui permet d'effectuer des transformations (matricielle normalement) sur les Node sous jacente.
Nous avons également une classe GeometryNode dont le but est d'avoir de la géométrie en son sein.

Etant donné que le nombre de Node peut grossir encore et encore, je ne peux pas vraiment utiliser de template à cause des Group (qui devraient contenir un vector pour chaque type de Node si on partait sur des templates).

Seulement voilà.

A la création de mon visiteur DrawerVisitor, je me rends compte que pour le cas de la TransformNode, on a besoin de savoir que c'est un noeud de transformation, ainsi que c'est un noeud Group. (sinon cela ne marchera pas pour les autres noeuds de type groupe).

Et je trouve ça un peu moche, mais c'est peut être uniquement mon avis.

Une solution possible serait de renvoyer un boolean pour savoir si ou non on a réussi le dynamic_cast, et d'utiliser traverse directement dans la fonction qui attend le noeud Transform, mais je trouve pas ça terrible non plus.

Peut être quelqu'un aura une idée plus propre pour faire les choses?

Voici mon code

#include <iostream>
#include <memory>
#include <vector>
#include <string>

class INode;
class INodeVisitor {
public:
    virtual void visit(INode&) = 0;
    virtual ~INodeVisitor() = default;
};

template<typename ...Ts>
class TypedNodeVisitor;

template<typename T1, typename ...Ts>
class TypedNodeVisitor<T1, Ts...> : public TypedNodeVisitor<Ts...> {
public:
    virtual void visit(INode &v) override {
        if(auto p = dynamic_cast<T1*>(std::addressof(v))) {
            apply(*p);
        }
        TypedNodeVisitor<Ts...>::visit(v);
    }

    using TypedNodeVisitor<Ts...>::apply;

    virtual void apply(T1 &) = 0;
};

template<>
class TypedNodeVisitor<> : public INodeVisitor {
public:
    void visit(INode&) override{}
    void apply(){} // use for the using apply
};

class INode {
public:
    void accept(INodeVisitor &nv) {
        nv.visit(*this);
    }

    virtual ~INode() = default;
};

class Group : public INode {
    std::vector<std::shared_ptr<INode>> m_children;

public:
    void addNode(std::shared_ptr<INode> node) {
        m_children.push_back(std::move(node));
    }

    virtual void traverse(INodeVisitor &nv) {
        for(auto &c : m_children) c->accept(nv);
    }
};

class ActivableNode : public Group {
    bool m_activate{true};
public:
    void traverse(INodeVisitor &nv) override {
        if(m_activate)
            Group::traverse(nv);
    }

    void disable() noexcept {
        m_activate = false;
    }
};

class GeometryNode : public INode {
public:
    std::string getGeometry() const noexcept {
        return "Geometry";
    }
};

class TransformNode : public Group {
public:
    int m_transform = 2;
    int getTransform() const noexcept {
        return m_transform;
    }
};

class DrawerVisitor : public TypedNodeVisitor<GeometryNode, TransformNode, Group> {
    int m_transform = 1;
public:
    void apply(GeometryNode &node) override {
        std::cout << node.getGeometry() << " with transform : " << m_transform << std::endl;
    }
    
    void apply(TransformNode &node) override {
        m_transform *= node.getTransform();
    }

    void apply(Group &node) override {
        node.traverse(*this);
    }
};

int main()
{
    auto root = std::make_shared<TransformNode>();
    auto activable1 = std::make_shared<ActivableNode>();
    auto geometry = std::make_shared<GeometryNode>();
    auto activable2 = std::make_shared<ActivableNode>();
    auto transform2 = std::make_shared<TransformNode>();

    root->addNode(activable1);
    root->addNode(activable2);
    activable1->disable();
    activable1->addNode(geometry); // this one will not be displayed
    activable2->addNode(geometry); // this one will
    root->addNode(transform2);
    transform2->addNode(geometry); // this one will

    DrawerVisitor visitor;
    root->accept(visitor);

    return 0;
}


Un deuxième problème qui remonte, c'est que si je veux faire une fusion "proprement" de la ActivableNode ainsi que du TransformNode, il faut que je fasse un héritage multiple/virtuel. C'est surtout ceci qui m'a montré que j'avais une erreur de design...

Merci à vous !

  • Partager sur Facebook
  • Partager sur Twitter
http://cpp-rendering.io : Vous trouverez tout ce dont vous avez besoin sur Vulkan / OpenGL et le rendu 3D !
3 septembre 2018 à 15:03:48

Heu, vous la sortez d'où cette implémentation de visitor avec des templates fourrés au dynamic_cast ???

Le DP Visitor n'est pas adapté à une grande hiérarchie de classe, et encore moins si elle est susceptible de "souvent" changer.

Dans votre cas, je ne pense pas que la hiérarchie de classe soit si grande, c'est donc peut-être un DP adapté, mais pourquoi ne pas utiliser une implémentation "classique", sans template ?

  • Partager sur Facebook
  • Partager sur Twitter
Je recherche un CDI/CDD/mission freelance comme Architecte Logiciel/ Expert Technique sur technologies Microsoft.
4 septembre 2018 à 1:55:17

Hello,

C'est un visiteur acyclique. On pourrait partir sur l'utilisation d'un visiteur classique, mais cela ne me permettrait pas de gérer mon soucis, et ça rendrait les choses encore plus difficiles malheureusement.

Je m'explique, si l'on a un visiteur qui attend un Activable et un Transform, et que la node en question est à la fois un Activable et un Transform, ça pose soucis.

Je pense qu'une façon de palier mon problème serait l'approche suivante :

Décomposé les états en différentes parties, un peu avec un style "décorateur".

Genre une activable node c'est un noeud avec l'état activable.

Une transform node c'est un noeud avec une transformation.

Une ActivableTransformNode c'est un noeud avec état activable + transformation.

à voir ce que ça peut donner mais cet exercice (de pure masturbation intellectuelle) m'est difficile ^^.

  • Partager sur Facebook
  • Partager sur Twitter
http://cpp-rendering.io : Vous trouverez tout ce dont vous avez besoin sur Vulkan / OpenGL et le rendu 3D !
4 septembre 2018 à 8:51:15

bacelar a écrit:

Heu, vous la sortez d'où cette implémentation de visitor avec des templates fourrés au dynamic_cast ???

Le DP Visitor n'est pas adapté à une grande hiérarchie de classe, et encore moins si elle est susceptible de "souvent" changer.

Dans votre cas, je ne pense pas que la hiérarchie de classe soit si grande, c'est donc peut-être un DP adapté, mais pourquoi ne pas utiliser une implémentation "classique", sans template ?


Cette implémentation est décrite dans le livre "Modern C++ design" de Andrei Alexandrescu. Effectivement il évite les inclusions croisées, tu peut aller t'y référer directement le chapitre est intéressant.

-
Edité par vac 4 septembre 2018 à 9:10:15

  • Partager sur Facebook
  • Partager sur Twitter
4 septembre 2018 à 15:14:46

J'ai du mal à comprendre l'utilité d'une classe "ActivableTransformNode".

Je mets un Node "Activable" et un Node "Transform" et basta.

  • Partager sur Facebook
  • Partager sur Twitter
Je recherche un CDI/CDD/mission freelance comme Architecte Logiciel/ Expert Technique sur technologies Microsoft.
4 septembre 2018 à 16:16:34

Salut !

Peut être que l'exemple est en effet mal choisi, mais je le répète, c'est un exercice de pure masturbation intellectuelle :).

  • Partager sur Facebook
  • Partager sur Twitter
http://cpp-rendering.io : Vous trouverez tout ce dont vous avez besoin sur Vulkan / OpenGL et le rendu 3D !
6 septembre 2018 à 21:06:01

J'ai fais quelques petites modifications sur le code de base qui nous permet d'éviter l'héritage virtuel.

Néanmoins ça commence à ressembler pas mal à une sorte d'ECS, du coup faudra que je vois comment adapter un scènegraph à un ECS.

Voilà le code pour le moment que j'ai en gardant le même style d'approche que précédemment :

#include <iostream>
#include <memory>
#include <vector>
#include <string>

class INode;
class INodeVisitor {
public:
    virtual void visit(INode &) = 0;
    virtual ~INodeVisitor() = default;
};

template<typename ...Ts>
class TypedNodeVisitor;

template<typename T1, typename ...Ts>
class TypedNodeVisitor<T1, Ts...> : public TypedNodeVisitor<Ts...> {
public:
    virtual void visit(INode &v) override {
        if(auto p = dynamic_cast<T1*>(std::addressof(v))) {
            apply(*p);
        }
        TypedNodeVisitor<Ts...>::visit(v);
    }

    using TypedNodeVisitor<Ts...>::apply;

    virtual void apply(T1 &) = 0;
};

template<>
class TypedNodeVisitor<> : public INodeVisitor {
public:
    void visit(INode&) override{}
    void apply(){} // use for the using apply
};

class INode {
public:
    void accept(INodeVisitor &nv) {
        accept(nv, false);
    }

    virtual ~INode() = default;

protected:
    virtual void accept(INodeVisitor &nv, bool alreadyVisited) {
        if(!alreadyVisited) {
            nv.visit(*this);
        }
    }
};

class HasGroup {
protected:
    std::vector<std::shared_ptr<INode>> m_children;

public:
    void addNode(std::shared_ptr<INode> node) {
        m_children.push_back(std::move(node));
    }
};

template<typename Parent>
class HasGroupWrapper : public HasGroup, public Parent {
protected:
    void accept(INodeVisitor &nv, bool alreadyVisited) override {
        if(!alreadyVisited)
            nv.visit(*this);
        for(auto child : m_children) {
            child->accept(nv);
        }
        Parent::accept(nv, true);
    }
};

class IsActivable {
protected:
    bool m_activate{true};
public:

    void disable() noexcept {
        m_activate = false;
    }
};

template<typename Parent>
class IsActivableWrapper : public IsActivable, public Parent {
  protected:
    void accept(INodeVisitor &nv, bool alreadyVisited) override {
        if(!alreadyVisited)
            nv.visit(*this);
        if(m_activate)
            Parent::accept(nv, true);
    }
};

class HasGeometry {
public:
    std::string getGeometry() const noexcept {
        return "Geometry";
    }
};

class CanPerformTransformation {
public:
    int m_transform = 2;
    int getTransform() const noexcept {
        return m_transform;
    }
};

template<template<typename> typename ...>
class NodeCompositer;

template<template<typename> typename First, template<typename> typename ...Others>
class NodeCompositer<First, Others...> : public First<NodeCompositer<Others...>> {};

template<>
class NodeCompositer<> : public INode {};

using Group = NodeCompositer<HasGroupWrapper>;
using ActivableNode = NodeCompositer<IsActivableWrapper, HasGroupWrapper>;
struct TransformableActivableNode : NodeCompositer<IsActivableWrapper, HasGroupWrapper>, CanPerformTransformation{};

int main()
{
    static_assert(std::is_base_of_v<INode, TransformableActivableNode>);
    static_assert(std::is_base_of_v<HasGroup, TransformableActivableNode>);
    static_assert(std::is_base_of_v<CanPerformTransformation, TransformableActivableNode>);
    static_assert(std::is_base_of_v<IsActivable, TransformableActivableNode>);
    static_assert(!std::is_base_of_v<HasGeometry, TransformableActivableNode>);

    return 0;
}



  • Partager sur Facebook
  • Partager sur Twitter
http://cpp-rendering.io : Vous trouverez tout ce dont vous avez besoin sur Vulkan / OpenGL et le rendu 3D !