Implementing different types of light sources in a Graphics project












3












$begingroup$


I'm working on a Computer Graphics project and I found a way to implement different types of light sources which I'm not sure is ideal.



There are two types of light sources: parallel, and point.



Point light source is moving around the mesh, and Parallel light source "can be imagined as a point light source lying in infinity". In other words a parallel light source has a direction which is constant throughout the mesh while a point light source is illuminating in all directions, but has a position which effects the direction of the light compared to the surface it's lighting.



I'm having trouble figuring out the right abstraction for all these types. Here's my solution:
I have one abstract base class called LightSource:



class LightSource: public IMovable,public IRotatable
{
protected:
glm::vec4 color;
public:
...
// Virtual Setters
virtual void SetDirection(const glm::vec4& _direction) =0;
virtual void SetLocation (const glm::vec4& _location) =0;

// Virtual Getters
virtual const glm::vec4* GetDirection() const =0;
virtual const glm::vec4* GetLocation() const =0;

// Inherited via IMovable
virtual void Move(const glm::vec3 direction) override =0;

// Inherited via IRotatable
virtual void RotateX(const float angle) override =0;
virtual void RotateY(const float angle) override =0;
virtual void RotateZ(const float angle) override =0;
};


and two derived classes:



class PointLightSource : public LightSource
{
private:
...
public:
// Constructors
...
// Base class
virtual const glm::vec4 * GetDirection() const override { return nullptr; };
virtual const glm::vec4 * GetLocation() const override { return &location; };

// Base class setters
virtual void SetDirection(const glm::vec4 & _direction) override { return; }
virtual void SetLocation(const glm::vec4 & _location) override { location = _location; }

// Inherited via LightSource
virtual void Move(const glm::vec3 direction) override;
virtual void RotateX(const float angle) override {} // Point light source emits light everywhere
virtual void RotateY(const float angle) override {}
virtual void RotateZ(const float angle) override {}
...
};


and:



class ParallelLightSource : public LightSource
{
private:
...
public:
...
// Setters
virtual void SetDirection(const glm::vec4 & _direction) override { direction = glm::normalize(_direction); }
virtual void SetLocation(const glm::vec4 & _location) override { return; }

// Getters
virtual const glm::vec4 * GetDirection() const override { return &direction; }
virtual const glm::vec4 * GetLocation() const override { return nullptr; };

// Inherited via LightSource
virtual void RotateX(const float angle) override;
virtual void RotateY(const float angle) override;
virtual void RotateZ(const float angle) override;

// Can't move a parallel light source
virtual void Move(const glm::vec3 direction) override {}
...
};


LightSource virtually implements two interfaces. one is called IMovable and it's used for every object that moves around the mesh (whether it's a model, a camera, or a light source).
another interface is called IRotatable, and it's used for objects that can rotate. Not every movable object is rotatable, and point light source is a good example of that, so I did two interfaces.



Essentially what I want is to be able to move point light source but not rotate them, and rotate parallel light sources without moving them, and do all that without checking for their actual type due to polymorphism. The problem is that every LightSource is both IMovable and IRotatable, so I can't distinguish between the derived types.



My awkward solution was to have two virtual functions: GetLocation and GetDirection that return pointers to vectors. the ParallelLightSource will return nullptr on GetLocation, and PointLightSource will return nullptr on GetDirection.



In the menus part of the code, I receive a vector of LightSource pointers and I want to display their appropriate menus. There are (as you guessed) two types of menus that are relevant here, one for IMovable objects and one for IRotatable objects.
I call the GetLocation & GetDirection functions and skip the appropriate controls if the pointer I got is a nullptr.



const glm::vec4* lightLocation =  activeLight->GetLocation();
const glm::vec4* lightDirection = activeLight->GetDirection();

...
if (lightLocation != nullptr)
{
moveObjectControls(activeLight,"Light");
}
if (lightDirection != nullptr)
{
newDirection = *lightDirection;
xyzSliders(newDirection, "Direction", worldRadius);
}


Passing around vector pointers rather than vectors by value is problematic. For example, now I actually do need ParallelLightSource to implement GetLocation and return an actual value. I'd have to either have a location class member which will always be equal to -direction * someLargeNumber, or refactor and have GetLocation and GetDirection return values, which will create another mess in the menus because I don't want to see controls that "move" a parallel light source or "rotate" a point light source because it doesn't make any sense, so I'd have to check for the derived types somehow which breaks polymorphism.



One solution I thought of is having booleans to indicate whether I want this light to show the IMovable/IRotatble menus, but that doesn't feel right because I don't think it's the light source responsibility to know how it is presented in the menus.



Another solution would be to hold one vector for parallel light sources and one for point light sources, but that doesn't sound smart because it's not very scalable and every time I'll implement a new type of light source I'll have to change the menus.



I'm quite new at programming (at least in terms of OOP), so I wonder what's the ideal solution to a situation like this. Let me know if you have other comments as well.










share|improve this question







New contributor




PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.







$endgroup$

















    3












    $begingroup$


    I'm working on a Computer Graphics project and I found a way to implement different types of light sources which I'm not sure is ideal.



    There are two types of light sources: parallel, and point.



    Point light source is moving around the mesh, and Parallel light source "can be imagined as a point light source lying in infinity". In other words a parallel light source has a direction which is constant throughout the mesh while a point light source is illuminating in all directions, but has a position which effects the direction of the light compared to the surface it's lighting.



    I'm having trouble figuring out the right abstraction for all these types. Here's my solution:
    I have one abstract base class called LightSource:



    class LightSource: public IMovable,public IRotatable
    {
    protected:
    glm::vec4 color;
    public:
    ...
    // Virtual Setters
    virtual void SetDirection(const glm::vec4& _direction) =0;
    virtual void SetLocation (const glm::vec4& _location) =0;

    // Virtual Getters
    virtual const glm::vec4* GetDirection() const =0;
    virtual const glm::vec4* GetLocation() const =0;

    // Inherited via IMovable
    virtual void Move(const glm::vec3 direction) override =0;

    // Inherited via IRotatable
    virtual void RotateX(const float angle) override =0;
    virtual void RotateY(const float angle) override =0;
    virtual void RotateZ(const float angle) override =0;
    };


    and two derived classes:



    class PointLightSource : public LightSource
    {
    private:
    ...
    public:
    // Constructors
    ...
    // Base class
    virtual const glm::vec4 * GetDirection() const override { return nullptr; };
    virtual const glm::vec4 * GetLocation() const override { return &location; };

    // Base class setters
    virtual void SetDirection(const glm::vec4 & _direction) override { return; }
    virtual void SetLocation(const glm::vec4 & _location) override { location = _location; }

    // Inherited via LightSource
    virtual void Move(const glm::vec3 direction) override;
    virtual void RotateX(const float angle) override {} // Point light source emits light everywhere
    virtual void RotateY(const float angle) override {}
    virtual void RotateZ(const float angle) override {}
    ...
    };


    and:



    class ParallelLightSource : public LightSource
    {
    private:
    ...
    public:
    ...
    // Setters
    virtual void SetDirection(const glm::vec4 & _direction) override { direction = glm::normalize(_direction); }
    virtual void SetLocation(const glm::vec4 & _location) override { return; }

    // Getters
    virtual const glm::vec4 * GetDirection() const override { return &direction; }
    virtual const glm::vec4 * GetLocation() const override { return nullptr; };

    // Inherited via LightSource
    virtual void RotateX(const float angle) override;
    virtual void RotateY(const float angle) override;
    virtual void RotateZ(const float angle) override;

    // Can't move a parallel light source
    virtual void Move(const glm::vec3 direction) override {}
    ...
    };


    LightSource virtually implements two interfaces. one is called IMovable and it's used for every object that moves around the mesh (whether it's a model, a camera, or a light source).
    another interface is called IRotatable, and it's used for objects that can rotate. Not every movable object is rotatable, and point light source is a good example of that, so I did two interfaces.



    Essentially what I want is to be able to move point light source but not rotate them, and rotate parallel light sources without moving them, and do all that without checking for their actual type due to polymorphism. The problem is that every LightSource is both IMovable and IRotatable, so I can't distinguish between the derived types.



    My awkward solution was to have two virtual functions: GetLocation and GetDirection that return pointers to vectors. the ParallelLightSource will return nullptr on GetLocation, and PointLightSource will return nullptr on GetDirection.



    In the menus part of the code, I receive a vector of LightSource pointers and I want to display their appropriate menus. There are (as you guessed) two types of menus that are relevant here, one for IMovable objects and one for IRotatable objects.
    I call the GetLocation & GetDirection functions and skip the appropriate controls if the pointer I got is a nullptr.



    const glm::vec4* lightLocation =  activeLight->GetLocation();
    const glm::vec4* lightDirection = activeLight->GetDirection();

    ...
    if (lightLocation != nullptr)
    {
    moveObjectControls(activeLight,"Light");
    }
    if (lightDirection != nullptr)
    {
    newDirection = *lightDirection;
    xyzSliders(newDirection, "Direction", worldRadius);
    }


    Passing around vector pointers rather than vectors by value is problematic. For example, now I actually do need ParallelLightSource to implement GetLocation and return an actual value. I'd have to either have a location class member which will always be equal to -direction * someLargeNumber, or refactor and have GetLocation and GetDirection return values, which will create another mess in the menus because I don't want to see controls that "move" a parallel light source or "rotate" a point light source because it doesn't make any sense, so I'd have to check for the derived types somehow which breaks polymorphism.



    One solution I thought of is having booleans to indicate whether I want this light to show the IMovable/IRotatble menus, but that doesn't feel right because I don't think it's the light source responsibility to know how it is presented in the menus.



    Another solution would be to hold one vector for parallel light sources and one for point light sources, but that doesn't sound smart because it's not very scalable and every time I'll implement a new type of light source I'll have to change the menus.



    I'm quite new at programming (at least in terms of OOP), so I wonder what's the ideal solution to a situation like this. Let me know if you have other comments as well.










    share|improve this question







    New contributor




    PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
    Check out our Code of Conduct.







    $endgroup$















      3












      3








      3


      2



      $begingroup$


      I'm working on a Computer Graphics project and I found a way to implement different types of light sources which I'm not sure is ideal.



      There are two types of light sources: parallel, and point.



      Point light source is moving around the mesh, and Parallel light source "can be imagined as a point light source lying in infinity". In other words a parallel light source has a direction which is constant throughout the mesh while a point light source is illuminating in all directions, but has a position which effects the direction of the light compared to the surface it's lighting.



      I'm having trouble figuring out the right abstraction for all these types. Here's my solution:
      I have one abstract base class called LightSource:



      class LightSource: public IMovable,public IRotatable
      {
      protected:
      glm::vec4 color;
      public:
      ...
      // Virtual Setters
      virtual void SetDirection(const glm::vec4& _direction) =0;
      virtual void SetLocation (const glm::vec4& _location) =0;

      // Virtual Getters
      virtual const glm::vec4* GetDirection() const =0;
      virtual const glm::vec4* GetLocation() const =0;

      // Inherited via IMovable
      virtual void Move(const glm::vec3 direction) override =0;

      // Inherited via IRotatable
      virtual void RotateX(const float angle) override =0;
      virtual void RotateY(const float angle) override =0;
      virtual void RotateZ(const float angle) override =0;
      };


      and two derived classes:



      class PointLightSource : public LightSource
      {
      private:
      ...
      public:
      // Constructors
      ...
      // Base class
      virtual const glm::vec4 * GetDirection() const override { return nullptr; };
      virtual const glm::vec4 * GetLocation() const override { return &location; };

      // Base class setters
      virtual void SetDirection(const glm::vec4 & _direction) override { return; }
      virtual void SetLocation(const glm::vec4 & _location) override { location = _location; }

      // Inherited via LightSource
      virtual void Move(const glm::vec3 direction) override;
      virtual void RotateX(const float angle) override {} // Point light source emits light everywhere
      virtual void RotateY(const float angle) override {}
      virtual void RotateZ(const float angle) override {}
      ...
      };


      and:



      class ParallelLightSource : public LightSource
      {
      private:
      ...
      public:
      ...
      // Setters
      virtual void SetDirection(const glm::vec4 & _direction) override { direction = glm::normalize(_direction); }
      virtual void SetLocation(const glm::vec4 & _location) override { return; }

      // Getters
      virtual const glm::vec4 * GetDirection() const override { return &direction; }
      virtual const glm::vec4 * GetLocation() const override { return nullptr; };

      // Inherited via LightSource
      virtual void RotateX(const float angle) override;
      virtual void RotateY(const float angle) override;
      virtual void RotateZ(const float angle) override;

      // Can't move a parallel light source
      virtual void Move(const glm::vec3 direction) override {}
      ...
      };


      LightSource virtually implements two interfaces. one is called IMovable and it's used for every object that moves around the mesh (whether it's a model, a camera, or a light source).
      another interface is called IRotatable, and it's used for objects that can rotate. Not every movable object is rotatable, and point light source is a good example of that, so I did two interfaces.



      Essentially what I want is to be able to move point light source but not rotate them, and rotate parallel light sources without moving them, and do all that without checking for their actual type due to polymorphism. The problem is that every LightSource is both IMovable and IRotatable, so I can't distinguish between the derived types.



      My awkward solution was to have two virtual functions: GetLocation and GetDirection that return pointers to vectors. the ParallelLightSource will return nullptr on GetLocation, and PointLightSource will return nullptr on GetDirection.



      In the menus part of the code, I receive a vector of LightSource pointers and I want to display their appropriate menus. There are (as you guessed) two types of menus that are relevant here, one for IMovable objects and one for IRotatable objects.
      I call the GetLocation & GetDirection functions and skip the appropriate controls if the pointer I got is a nullptr.



      const glm::vec4* lightLocation =  activeLight->GetLocation();
      const glm::vec4* lightDirection = activeLight->GetDirection();

      ...
      if (lightLocation != nullptr)
      {
      moveObjectControls(activeLight,"Light");
      }
      if (lightDirection != nullptr)
      {
      newDirection = *lightDirection;
      xyzSliders(newDirection, "Direction", worldRadius);
      }


      Passing around vector pointers rather than vectors by value is problematic. For example, now I actually do need ParallelLightSource to implement GetLocation and return an actual value. I'd have to either have a location class member which will always be equal to -direction * someLargeNumber, or refactor and have GetLocation and GetDirection return values, which will create another mess in the menus because I don't want to see controls that "move" a parallel light source or "rotate" a point light source because it doesn't make any sense, so I'd have to check for the derived types somehow which breaks polymorphism.



      One solution I thought of is having booleans to indicate whether I want this light to show the IMovable/IRotatble menus, but that doesn't feel right because I don't think it's the light source responsibility to know how it is presented in the menus.



      Another solution would be to hold one vector for parallel light sources and one for point light sources, but that doesn't sound smart because it's not very scalable and every time I'll implement a new type of light source I'll have to change the menus.



      I'm quite new at programming (at least in terms of OOP), so I wonder what's the ideal solution to a situation like this. Let me know if you have other comments as well.










      share|improve this question







      New contributor




      PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.







      $endgroup$




      I'm working on a Computer Graphics project and I found a way to implement different types of light sources which I'm not sure is ideal.



      There are two types of light sources: parallel, and point.



      Point light source is moving around the mesh, and Parallel light source "can be imagined as a point light source lying in infinity". In other words a parallel light source has a direction which is constant throughout the mesh while a point light source is illuminating in all directions, but has a position which effects the direction of the light compared to the surface it's lighting.



      I'm having trouble figuring out the right abstraction for all these types. Here's my solution:
      I have one abstract base class called LightSource:



      class LightSource: public IMovable,public IRotatable
      {
      protected:
      glm::vec4 color;
      public:
      ...
      // Virtual Setters
      virtual void SetDirection(const glm::vec4& _direction) =0;
      virtual void SetLocation (const glm::vec4& _location) =0;

      // Virtual Getters
      virtual const glm::vec4* GetDirection() const =0;
      virtual const glm::vec4* GetLocation() const =0;

      // Inherited via IMovable
      virtual void Move(const glm::vec3 direction) override =0;

      // Inherited via IRotatable
      virtual void RotateX(const float angle) override =0;
      virtual void RotateY(const float angle) override =0;
      virtual void RotateZ(const float angle) override =0;
      };


      and two derived classes:



      class PointLightSource : public LightSource
      {
      private:
      ...
      public:
      // Constructors
      ...
      // Base class
      virtual const glm::vec4 * GetDirection() const override { return nullptr; };
      virtual const glm::vec4 * GetLocation() const override { return &location; };

      // Base class setters
      virtual void SetDirection(const glm::vec4 & _direction) override { return; }
      virtual void SetLocation(const glm::vec4 & _location) override { location = _location; }

      // Inherited via LightSource
      virtual void Move(const glm::vec3 direction) override;
      virtual void RotateX(const float angle) override {} // Point light source emits light everywhere
      virtual void RotateY(const float angle) override {}
      virtual void RotateZ(const float angle) override {}
      ...
      };


      and:



      class ParallelLightSource : public LightSource
      {
      private:
      ...
      public:
      ...
      // Setters
      virtual void SetDirection(const glm::vec4 & _direction) override { direction = glm::normalize(_direction); }
      virtual void SetLocation(const glm::vec4 & _location) override { return; }

      // Getters
      virtual const glm::vec4 * GetDirection() const override { return &direction; }
      virtual const glm::vec4 * GetLocation() const override { return nullptr; };

      // Inherited via LightSource
      virtual void RotateX(const float angle) override;
      virtual void RotateY(const float angle) override;
      virtual void RotateZ(const float angle) override;

      // Can't move a parallel light source
      virtual void Move(const glm::vec3 direction) override {}
      ...
      };


      LightSource virtually implements two interfaces. one is called IMovable and it's used for every object that moves around the mesh (whether it's a model, a camera, or a light source).
      another interface is called IRotatable, and it's used for objects that can rotate. Not every movable object is rotatable, and point light source is a good example of that, so I did two interfaces.



      Essentially what I want is to be able to move point light source but not rotate them, and rotate parallel light sources without moving them, and do all that without checking for their actual type due to polymorphism. The problem is that every LightSource is both IMovable and IRotatable, so I can't distinguish between the derived types.



      My awkward solution was to have two virtual functions: GetLocation and GetDirection that return pointers to vectors. the ParallelLightSource will return nullptr on GetLocation, and PointLightSource will return nullptr on GetDirection.



      In the menus part of the code, I receive a vector of LightSource pointers and I want to display their appropriate menus. There are (as you guessed) two types of menus that are relevant here, one for IMovable objects and one for IRotatable objects.
      I call the GetLocation & GetDirection functions and skip the appropriate controls if the pointer I got is a nullptr.



      const glm::vec4* lightLocation =  activeLight->GetLocation();
      const glm::vec4* lightDirection = activeLight->GetDirection();

      ...
      if (lightLocation != nullptr)
      {
      moveObjectControls(activeLight,"Light");
      }
      if (lightDirection != nullptr)
      {
      newDirection = *lightDirection;
      xyzSliders(newDirection, "Direction", worldRadius);
      }


      Passing around vector pointers rather than vectors by value is problematic. For example, now I actually do need ParallelLightSource to implement GetLocation and return an actual value. I'd have to either have a location class member which will always be equal to -direction * someLargeNumber, or refactor and have GetLocation and GetDirection return values, which will create another mess in the menus because I don't want to see controls that "move" a parallel light source or "rotate" a point light source because it doesn't make any sense, so I'd have to check for the derived types somehow which breaks polymorphism.



      One solution I thought of is having booleans to indicate whether I want this light to show the IMovable/IRotatble menus, but that doesn't feel right because I don't think it's the light source responsibility to know how it is presented in the menus.



      Another solution would be to hold one vector for parallel light sources and one for point light sources, but that doesn't sound smart because it's not very scalable and every time I'll implement a new type of light source I'll have to change the menus.



      I'm quite new at programming (at least in terms of OOP), so I wonder what's the ideal solution to a situation like this. Let me know if you have other comments as well.







      c++ object-oriented inheritance polymorphism






      share|improve this question







      New contributor




      PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.











      share|improve this question







      New contributor




      PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.









      share|improve this question




      share|improve this question






      New contributor




      PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.









      asked 2 hours ago









      PanthersFan92PanthersFan92

      1163




      1163




      New contributor




      PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.





      New contributor





      PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.






      PanthersFan92 is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
      Check out our Code of Conduct.






















          2 Answers
          2






          active

          oldest

          votes


















          4












          $begingroup$

          Why is LightSource inheriting from IMovable and IRotatable if not every LightSource is movable or rotatable? LightSource should only contain functions that relate to light sources, for instance intensity or color. The position and the direction are not properties of a light source here. Instead I'd make ParallelLightSource inherit from IRotatable and PointLightSource inherit from IMovable.



          From your minimal example I don't see any need for the base class LightSource but you may have left this stuff out to illustrate your problem. If you do not need it, remove it.



          I understand you want to edit your sources using menus, but I have no idea how your application organizes the data here.
          If you really need the base class LightSource you may use RTTI to check whether your object is a IMovable or an IRotatable when creating the menu (dynamic_cast<IMovable*>(ptr) checks if ptr is a subclass of IMovable).






          share|improve this answer








          New contributor




          Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
          Check out our Code of Conduct.






          $endgroup$













          • $begingroup$
            I need the base class because I have a vector of all the lights, so it's a vector of pointers to LightSource objects
            $endgroup$
            – PanthersFan92
            38 mins ago










          • $begingroup$
            Dynamic casting sounds interesting. I think it might solve it. Thanks!
            $endgroup$
            – PanthersFan92
            36 mins ago



















          3












          $begingroup$

          Overriding RotateX, RotateY and RotateZ to prevent rotation of your point light source is going to cause you grief when you add another type of light source: the spot light. A spot light is a directional point light source, and would be both IMovable and IRotatable. But if you try to derive it from PointLightSource, you’d end up having to add back in the the functionality you took out by overriding.



          You probably want to remove IMovable and IRotatable from LightSource, and only add them to derived types that have those attributes.



          What is a non-movable, non-rotatable light source you ask? How about AmbientLight?






          share|improve this answer









          $endgroup$













            Your Answer





            StackExchange.ifUsing("editor", function () {
            return StackExchange.using("mathjaxEditing", function () {
            StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
            StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
            });
            });
            }, "mathjax-editing");

            StackExchange.ifUsing("editor", function () {
            StackExchange.using("externalEditor", function () {
            StackExchange.using("snippets", function () {
            StackExchange.snippets.init();
            });
            });
            }, "code-snippets");

            StackExchange.ready(function() {
            var channelOptions = {
            tags: "".split(" "),
            id: "196"
            };
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function() {
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled) {
            StackExchange.using("snippets", function() {
            createEditor();
            });
            }
            else {
            createEditor();
            }
            });

            function createEditor() {
            StackExchange.prepareEditor({
            heartbeatType: 'answer',
            autoActivateHeartbeat: false,
            convertImagesToLinks: false,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            imageUploader: {
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            },
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            });


            }
            });






            PanthersFan92 is a new contributor. Be nice, and check out our Code of Conduct.










            draft saved

            draft discarded


















            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f212417%2fimplementing-different-types-of-light-sources-in-a-graphics-project%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown

























            2 Answers
            2






            active

            oldest

            votes








            2 Answers
            2






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes









            4












            $begingroup$

            Why is LightSource inheriting from IMovable and IRotatable if not every LightSource is movable or rotatable? LightSource should only contain functions that relate to light sources, for instance intensity or color. The position and the direction are not properties of a light source here. Instead I'd make ParallelLightSource inherit from IRotatable and PointLightSource inherit from IMovable.



            From your minimal example I don't see any need for the base class LightSource but you may have left this stuff out to illustrate your problem. If you do not need it, remove it.



            I understand you want to edit your sources using menus, but I have no idea how your application organizes the data here.
            If you really need the base class LightSource you may use RTTI to check whether your object is a IMovable or an IRotatable when creating the menu (dynamic_cast<IMovable*>(ptr) checks if ptr is a subclass of IMovable).






            share|improve this answer








            New contributor




            Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            $endgroup$













            • $begingroup$
              I need the base class because I have a vector of all the lights, so it's a vector of pointers to LightSource objects
              $endgroup$
              – PanthersFan92
              38 mins ago










            • $begingroup$
              Dynamic casting sounds interesting. I think it might solve it. Thanks!
              $endgroup$
              – PanthersFan92
              36 mins ago
















            4












            $begingroup$

            Why is LightSource inheriting from IMovable and IRotatable if not every LightSource is movable or rotatable? LightSource should only contain functions that relate to light sources, for instance intensity or color. The position and the direction are not properties of a light source here. Instead I'd make ParallelLightSource inherit from IRotatable and PointLightSource inherit from IMovable.



            From your minimal example I don't see any need for the base class LightSource but you may have left this stuff out to illustrate your problem. If you do not need it, remove it.



            I understand you want to edit your sources using menus, but I have no idea how your application organizes the data here.
            If you really need the base class LightSource you may use RTTI to check whether your object is a IMovable or an IRotatable when creating the menu (dynamic_cast<IMovable*>(ptr) checks if ptr is a subclass of IMovable).






            share|improve this answer








            New contributor




            Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            $endgroup$













            • $begingroup$
              I need the base class because I have a vector of all the lights, so it's a vector of pointers to LightSource objects
              $endgroup$
              – PanthersFan92
              38 mins ago










            • $begingroup$
              Dynamic casting sounds interesting. I think it might solve it. Thanks!
              $endgroup$
              – PanthersFan92
              36 mins ago














            4












            4








            4





            $begingroup$

            Why is LightSource inheriting from IMovable and IRotatable if not every LightSource is movable or rotatable? LightSource should only contain functions that relate to light sources, for instance intensity or color. The position and the direction are not properties of a light source here. Instead I'd make ParallelLightSource inherit from IRotatable and PointLightSource inherit from IMovable.



            From your minimal example I don't see any need for the base class LightSource but you may have left this stuff out to illustrate your problem. If you do not need it, remove it.



            I understand you want to edit your sources using menus, but I have no idea how your application organizes the data here.
            If you really need the base class LightSource you may use RTTI to check whether your object is a IMovable or an IRotatable when creating the menu (dynamic_cast<IMovable*>(ptr) checks if ptr is a subclass of IMovable).






            share|improve this answer








            New contributor




            Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            $endgroup$



            Why is LightSource inheriting from IMovable and IRotatable if not every LightSource is movable or rotatable? LightSource should only contain functions that relate to light sources, for instance intensity or color. The position and the direction are not properties of a light source here. Instead I'd make ParallelLightSource inherit from IRotatable and PointLightSource inherit from IMovable.



            From your minimal example I don't see any need for the base class LightSource but you may have left this stuff out to illustrate your problem. If you do not need it, remove it.



            I understand you want to edit your sources using menus, but I have no idea how your application organizes the data here.
            If you really need the base class LightSource you may use RTTI to check whether your object is a IMovable or an IRotatable when creating the menu (dynamic_cast<IMovable*>(ptr) checks if ptr is a subclass of IMovable).







            share|improve this answer








            New contributor




            Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.









            share|improve this answer



            share|improve this answer






            New contributor




            Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.









            answered 1 hour ago









            CornholioCornholio

            1165




            1165




            New contributor




            Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.





            New contributor





            Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.






            Cornholio is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
            Check out our Code of Conduct.












            • $begingroup$
              I need the base class because I have a vector of all the lights, so it's a vector of pointers to LightSource objects
              $endgroup$
              – PanthersFan92
              38 mins ago










            • $begingroup$
              Dynamic casting sounds interesting. I think it might solve it. Thanks!
              $endgroup$
              – PanthersFan92
              36 mins ago


















            • $begingroup$
              I need the base class because I have a vector of all the lights, so it's a vector of pointers to LightSource objects
              $endgroup$
              – PanthersFan92
              38 mins ago










            • $begingroup$
              Dynamic casting sounds interesting. I think it might solve it. Thanks!
              $endgroup$
              – PanthersFan92
              36 mins ago
















            $begingroup$
            I need the base class because I have a vector of all the lights, so it's a vector of pointers to LightSource objects
            $endgroup$
            – PanthersFan92
            38 mins ago




            $begingroup$
            I need the base class because I have a vector of all the lights, so it's a vector of pointers to LightSource objects
            $endgroup$
            – PanthersFan92
            38 mins ago












            $begingroup$
            Dynamic casting sounds interesting. I think it might solve it. Thanks!
            $endgroup$
            – PanthersFan92
            36 mins ago




            $begingroup$
            Dynamic casting sounds interesting. I think it might solve it. Thanks!
            $endgroup$
            – PanthersFan92
            36 mins ago













            3












            $begingroup$

            Overriding RotateX, RotateY and RotateZ to prevent rotation of your point light source is going to cause you grief when you add another type of light source: the spot light. A spot light is a directional point light source, and would be both IMovable and IRotatable. But if you try to derive it from PointLightSource, you’d end up having to add back in the the functionality you took out by overriding.



            You probably want to remove IMovable and IRotatable from LightSource, and only add them to derived types that have those attributes.



            What is a non-movable, non-rotatable light source you ask? How about AmbientLight?






            share|improve this answer









            $endgroup$


















              3












              $begingroup$

              Overriding RotateX, RotateY and RotateZ to prevent rotation of your point light source is going to cause you grief when you add another type of light source: the spot light. A spot light is a directional point light source, and would be both IMovable and IRotatable. But if you try to derive it from PointLightSource, you’d end up having to add back in the the functionality you took out by overriding.



              You probably want to remove IMovable and IRotatable from LightSource, and only add them to derived types that have those attributes.



              What is a non-movable, non-rotatable light source you ask? How about AmbientLight?






              share|improve this answer









              $endgroup$
















                3












                3








                3





                $begingroup$

                Overriding RotateX, RotateY and RotateZ to prevent rotation of your point light source is going to cause you grief when you add another type of light source: the spot light. A spot light is a directional point light source, and would be both IMovable and IRotatable. But if you try to derive it from PointLightSource, you’d end up having to add back in the the functionality you took out by overriding.



                You probably want to remove IMovable and IRotatable from LightSource, and only add them to derived types that have those attributes.



                What is a non-movable, non-rotatable light source you ask? How about AmbientLight?






                share|improve this answer









                $endgroup$



                Overriding RotateX, RotateY and RotateZ to prevent rotation of your point light source is going to cause you grief when you add another type of light source: the spot light. A spot light is a directional point light source, and would be both IMovable and IRotatable. But if you try to derive it from PointLightSource, you’d end up having to add back in the the functionality you took out by overriding.



                You probably want to remove IMovable and IRotatable from LightSource, and only add them to derived types that have those attributes.



                What is a non-movable, non-rotatable light source you ask? How about AmbientLight?







                share|improve this answer












                share|improve this answer



                share|improve this answer










                answered 1 hour ago









                AJNeufeldAJNeufeld

                4,672318




                4,672318






















                    PanthersFan92 is a new contributor. Be nice, and check out our Code of Conduct.










                    draft saved

                    draft discarded


















                    PanthersFan92 is a new contributor. Be nice, and check out our Code of Conduct.













                    PanthersFan92 is a new contributor. Be nice, and check out our Code of Conduct.












                    PanthersFan92 is a new contributor. Be nice, and check out our Code of Conduct.
















                    Thanks for contributing an answer to Code Review Stack Exchange!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid



                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.


                    Use MathJax to format equations. MathJax reference.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function () {
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f212417%2fimplementing-different-types-of-light-sources-in-a-graphics-project%23new-answer', 'question_page');
                    }
                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    Why is a white electrical wire connected to 2 black wires?

                    Waikiki

                    What are all the squawk codes?