Ist hier dynamic_cast ok?



  • Naja, ich weiss eben in Unterklassen von Bar, was fuer Unterklassen in method() reinkommen.
    Mein konkreter Fall: Bar ist ein RenderSystem und Unterklassen sind zB D3D9RenderSystem oder OGLRenderSystem. Foo ist ein RenderTarget. RenderWindow erbt von RenderTarget und D3D9RenderWindow sowie OGLRenderWindow erbt von RenderWindow.

    Nun habe ich eben folgende Methode:
    virtual void RenderSystem::swapBuffers(RenderWindow* win) = 0;
    Alle RenderSystems koennen den BackBuffer swappen, aber WIE das geht, ist abhaengig von der Render API. Und ein D3D9RenderSystem kriegt eben IMMER ein D3D9RenderWindow uebergeben und ein OGLRenderSystem IMMER ein OGLRenderWindow. Das D3D9RenderSystem muss auf eine Methode von D3D9RenderWindow zugreifen, die eben Direct3D9 spezifisch ist und die ein OGLRenderWindow nicht hat.

    Drum mache ich das momentan so und es funktioniert ja auch wunderbar:

    D3D9RenderSystem::swapBuffers(RenderWindow* win) {
       D3D9RenderWindow* wind3d = static_cast<D3D9RenderWindow*>(win);
       win3d->getSwapChain(); 
       // Ein D3D9RenderSystem kann eben nur ein D3D9RenderWindow bekommen
       ...
    }
    

    Ist das Design jetzt so verwerflich?



  • caster2 schrieb:

    Ist das Design jetzt so verwerflich?

    Ja. Denn:

    caster2 schrieb:

    D3D9RenderSystem kriegt eben IMMER ein D3D9RenderWindow

    Du hast also an der Stelle an der Du swapBuffers aufrufst ein konkretes RenderSystem und nicht die Basisklasse. Du weiß also, was Du für einen RenderSystem-Typen hast. Wenn Du das aber weisst, wo ist dann der Sinn darin, swapBuffers virtuell in der Basisklasse festulegen? Du kannst einfach eine D3D9RenderSystem::swapBuffers anbieten, das gleich den passenden Typen bekommt.



  • Tachyon schrieb:

    caster2 schrieb:

    Ist das Design jetzt so verwerflich?

    Ja. Denn:

    caster2 schrieb:

    D3D9RenderSystem kriegt eben IMMER ein D3D9RenderWindow

    Du hast also an der Stelle an der Du swapBuffers aufrufst ein konkretes RenderSystem und nicht die Basisklasse. Du weiß also, was Du für einen RenderSystem-Typen hast. Wenn Du das aber weisst, wo ist dann der Sinn darin, swapBuffers virtuell in der Basisklasse festulegen? Du kannst einfach eine D3D9RenderSystem::swapBuffers anbieten, das gleich den passenden Typen bekommt.

    Dann verlager ich aber nur das Problem. Ein RenderTarget hat bei mir einen Zeiger auf ein RenderSystem (Jedes RenderTarget wird von einem RenderSystem erzeugt).
    Der Aufruf von swapBuffers() findet dann in der Basisklasse RenderTarget statt, z.B.:

    RenderTarget::RenderTarget(RenderSystem* r) : mRenderSystem(r) { }
    
    So wird ein D3D9RenderWindow erzeugt: new D3D9RenderWindow(d3d9RenderSystem);
    Bei einem D3D9RenderWindow ist mRenderSystem also immer vom Typ D3D9RenderSystem.
    
    D3D9RenderWindow::foo() {
       mRenderSystem->swapBuffers(this); // In einem D3D9RenderWindow ist mRenderSystem vom Typ D3D9RenderWindow.
    }
    

    Wenn ich jetzt swapBuffers aus RenderWindow entferne und in die Subklassen verschiebe (also D3D9RenderSystem::swapBuffers(D3D9RenderWindow*) statt RenderSystem::swapBuffers(RenderWindow*), dann muesste ich jetzt aber in D3D9RenderWindow::foo() casten, sprich:

    D3D9RenderWindow::foo() {
       // geht nicht mehr:mRenderSystem->swapBuffers(this); 
       D3D9RenderSystem* d3d9rs = static_cast<mRenderSystem>();
       d3d9rs->swapBuffers(this);
    }
    

    Ich hatte den Downcast also nur verschoben;/



  • Dann nimm mRenderSystem aus RenderTarget raus und lass jedes RenderTarget selber seine RenderSystem Referenz halten:

    class D3D9RenderWindow : public RenderTarget
    {
    public:
        D3D9RenderWindow(D3D9RenderSystem* r) :mRenderSystem(r){}
    private:
        D3D9RenderSystem * mRenderSystem;
    };
    


  • brotbernd schrieb:

    Dann nimm mRenderSystem aus RenderTarget raus und lass jedes RenderTarget selber seine RenderSystem Referenz halten:

    Das geht nicht, weil RenderTarget Methoden an vielen Stellen mRenderSystem benutzt.



  • caster2 schrieb:

    brotbernd schrieb:

    Dann nimm mRenderSystem aus RenderTarget raus und lass jedes RenderTarget selber seine RenderSystem Referenz halten:

    Das geht nicht, weil RenderTarget Methoden an vielen Stellen mRenderSystem benutzt.

    Jetzt ist bei mir auch die Warnlampe angegangen.
    Ich fürchte, da stimmt was nicht.



  • volkard schrieb:

    caster2 schrieb:

    brotbernd schrieb:

    Dann nimm mRenderSystem aus RenderTarget raus und lass jedes RenderTarget selber seine RenderSystem Referenz halten:

    Das geht nicht, weil RenderTarget Methoden an vielen Stellen mRenderSystem benutzt.

    Jetzt ist bei mir auch die Warnlampe angegangen.
    Ich fürchte, da stimmt was nicht.

    Ich bin offen fuer Verbesserungsvorschlaege;)

    Allerdings sehe ich nicht so recht das Problem. Ein RenderTarget benutzt eben die Methoden von RenderSystem, da prinzipiell alle RenderSystems gleich arbeiten. Nur in einem einzigen Fall: In RenderSystem::swapBuffers(RenderWindow*) braucht das D3DRenderSystem Zugriff auf D3DRenderWindow::SwapChain. Ein OGLRenderSystem braucht diesen Zugriff nicht, denn da gibt es nicht mal SwapChains. Keine Ahnung, wie ich das anders machen sollte als mit dem EINEN cast.



  • Warum hält das RenderTarget einen Zeiger auf das RenderSystem?

    Ich habe leider keine Ahnung von OpenGL oder DirectX, aber für mich hört es sich sinnvoller an, die RenderTargets an das entsprechende RenderSystem zu übergeben und nicht umgekehrt...



  • caster2 schrieb:

    brotbernd schrieb:

    Dann nimm mRenderSystem aus RenderTarget raus und lass jedes RenderTarget selber seine RenderSystem Referenz halten:

    Das geht nicht, weil RenderTarget Methoden an vielen Stellen mRenderSystem benutzt.

    class RenderTarget
    {
    private:
        virtual const RenderSystem* GetRenderSystem() const = 0;
    }
    class D3D9Renderwindow : public RenderTarget
    {
    private:
        D3D9RenderSystem mRenderSystem;
        const RenderSystem& GetRenderSystem() const { return mRenderSystem; }
    }
    

    caster2 schrieb:

    Ein RenderTarget benutzt eben die Methoden von RenderSystem, da prinzipiell alle RenderSystems gleich arbeiten. Nur in einem einzigen Fall: In RenderSystem::swapBuffers(RenderWindow*) braucht das D3DRenderSystem Zugriff auf D3DRenderWindow::SwapChain.

    Prinzipiell gleich ist eben nicht gleich. Unterscheidet sich D3DRenderWindow in nur einem winzigen Punkt von anderen RenderWindows, dann ist D3DRenderWindow kein RenderWindow (siehe oben: Liskov...).

    Für mich klingt das irgendwie alles nach einem unheimlich komplexen System, das überhaupt keine Komplexität benötigt. Sei sparsam mit Vererbungen! Es muss nicht irgendwie alles mit allem verwandt sein, nur weil man versucht OOP zu machen.



  • bmario schrieb:

    Warum hält das RenderTarget einen Zeiger auf das RenderSystem?.

    Weil jedes RenderTarget eben Funktionalitaet des RenderSystems braucht. Wenn ein RenderTarget geupdatet werden soll, muessen bestimmte Puffer geleert werden. Und das ist Aufgabe des RenderSystems.

    bmario schrieb:

    Ich habe leider keine Ahnung von OpenGL oder DirectX, aber für mich hört es sich sinnvoller an, die RenderTargets an das entsprechende RenderSystem zu übergeben und nicht umgekehrt...

    So ist es auch bereits. Die RenderTargets werden vom RenderSystem erzeugt und kriegen einen Zeiger aufs RenderSystem (this) mit. Quasi einen Zeiger auf ihren Erzeuger.



  • brotbernd schrieb:

    Prinzipiell gleich ist eben nicht gleich. Unterscheidet sich D3DRenderWindow in nur einem winzigen Punkt von anderen RenderWindows, dann ist D3DRenderWindow kein RenderWindow (siehe oben: Liskov...).

    Ich verletze nirgends das LSP. Ein D3D9RenderWindow HAT alle Methoden eines RenderWindow und fuellt diese auch sinnvoll aus!

    brotbernd schrieb:

    Für mich klingt das irgendwie alles nach einem unheimlich komplexen System, das überhaupt keine Komplexität benötigt. Sei sparsam mit Vererbungen! Es muss nicht irgendwie alles mit allem verwandt sein, nur weil man versucht OOP zu machen.

    Was ist denn an 2 Basisklassen bitte Komplex? Simpler als "Ein D3D9RenderSystem IST ein RenderSystem" und "Ein D3D9RenderWindow IST ein RenderWindow IST ein RenderTarget" gehts ja nicht mehr.
    Abgesehen davon ist das "Wrappen" von Render APIs einigermassen komplex. An manchen stellen unterscheiden sie sich eben doch deutlich.



  • Die Erfahrung lehrt mich dass es nicht wirklich sinnvoll ist eine 3D API auf dieser Ebene zu kapseln auf der dus gerade versuchst. Bau lieber einen Renderer der ein Interface bietet für das was du eigentlich damit anstellen willst und implementier dann dieses Interface jeweils mit D3D bzw. OpenGL. Ein downcast ist ein Symptom für sehr kaputtes Design und selbst abgesehen davon willst du hier schon um der Performance willen keinen dynamic_cast. Einen OOP Wrapper für OpenGL schreiben willst du nicht, glaub mir. Mit dem komischen Objektmodell von OpenGL endet das nur in Trauer und Schmerz...



  • dot schrieb:

    Die Erfahrung lehrt mich dass es nicht wirklich sinnvoll ist eine 3D API auf dieser Ebene zu kapseln auf der dus gerade versuchst. Bau lieber einen Renderer der ein Interface bietet für das was du eigentlich damit anstellen willst und implementier dann dieses Interface jeweils mit D3D bzw. OpenGL. Einen OOP Wrapper für OpenGL schreiben willst du nicht, glaub mir. Mit dem komischen Objektmodell von OpenGL endet das nur in Trauer und Schmerz...

    RenderSystem IST mein Interface zu den Render APIs....



  • caster2 schrieb:

    RenderSystem IST mein Interface zu den Render APIs....

    Ist mir klar. Was ich damit sagen wollte ist: Dein Interface ist imo zu low-level. Du versuchst einfach nur D3D bzw. OpenGL hinter einen Layer zu stellen der exakt den gleichen Abstraktionsgrad hat wie die APIs selbst. Und genau das ist meiner Erfahrung nach absolut nicht sinnvoll. Glaub mir, ich hab schon oft genug solche Wrapper geschrieben. Vor allem OpenGL in C++ zu wrappen ist einfach nur pita 😉



  • caster2 schrieb:

    Ich verletze nirgends das LSP. Ein D3D9RenderWindow HAT alle Methoden eines RenderWindow und fuellt diese auch sinnvoll aus!

    Du verletzt das LSP z.B. an dieser Stelle:

    D3D9RenderSystem::swapBuffers(RenderWindow* win)
    

    LSP schrieb:

    Preconditions cannot be strengthened in a subtype

    RenderSystem::swapBuffers hat zur Bedingung, dass *win ein gültiger Zeiger auf irgendeine RenderWindow Instanz ist.
    D3D9RenderSystem::swapBuffers hat die Bedingung dass *win ein Zeiger auf ein D3D9RenderWindow ist. Du kannst also D3D9RenderSystem nicht durch ein andereres substituieren. Das ist nun wirklich eine eindeutige Verletzung.

    D3D9RenderSystem kann nicht jedes RenderWindow behandeln, daher ist diese Abstraktion an dieser Stelle einfach sinnlos.



  • brotbernd schrieb:

    D3D9RenderSystem kann nicht jedes RenderWindow behandeln, daher ist diese Abstraktion an dieser Stelle einfach sinnlos.

    Stimmt, aber da es nur D3D9RenderWindows kriegen kann, sehe ich damit kein Problem. Vor allem faellt mir nichts eleganteres ein.

    @Dot: Ich weiss. Das abstrahieren von Render APIs ist immer ein Schmerz im Hintern und OpenGL ganz besonders. Ich wage zu behaupten, dass ein 100% elegantes Wrappen beider APIs ohne kleinere Hacks unmoeglich ist. Dazu sind sie teilweise einfach zu unterschiedlich. Ich sag nur Contexts vs Swap Chains, Lost Devices etc.



  • caster2 schrieb:

    Stimmt, aber da es nur D3D9RenderWindows kriegen kann, sehe ich damit kein Problem. Vor allem faellt mir nichts eleganteres ein.

    Und was war mit diesem Vorschlag

    class RenderSystem
    {
    public:
        virtual void MachWas() const = 0;
    };
    
    class RenderTarget
    {
    public:
        void TuWas()
        {
            GetRenderSystem()->MachWas();
            Schwipp();
        }
    private:
        virtual void Schwipp() = 0;
        virtual const RenderSystem* GetRenderSystem() const = 0;
    };
    
    class D3RenderWindow;
    class D3RenderSystem : public RenderSystem
    {
    public:
        void MachWas() const { std::cout << "D3RenderSystem macht was\n"; }
        void Schwapp(D3RenderWindow& window){}
    
    };
    
    class D3RenderWindow : public RenderTarget
    {
    public:
        D3RenderSystem mRenderSystem;
        void Schwipp()
        {
            mRenderSystem.Schwapp(*this);
        }
        const RenderSystem* GetRenderSystem() const
        {
            return &mRenderSystem;
        }
    };
    


  • caster2 schrieb:

    Ich wage zu behaupten, dass ein 100% elegantes Wrappen beider APIs ohne kleinere Hacks unmoeglich ist.

    Ja definitiv.



  • dot schrieb:

    caster2 schrieb:

    Ich wage zu behaupten, dass ein 100% elegantes Wrappen beider APIs ohne kleinere Hacks unmoeglich ist.

    Ja definitiv.

    Im Grunde gehts mir nur darum die Anzahl Hacks zu minimieren. 😃 Ganz vermeiden ist unmoeglich.

    @brotbernd: Danke! Hab leider im Moment keinen Zugriff auf meinen echten Code und kann deshalb gerade nicht sagen, ob das an irgend einer Stelle Probleme geben koennte. Aber sieht sehr interessant aus! 🙂



  • caster2 schrieb:

    Ganz vermeiden ist unmoeglich.

    Man könnte es vermeiden sinnlose Wrapper zu schreiben, dann braucht man auch keine Hacks und spart unnötigen Overhead 😉


Anmelden zum Antworten