simonech / ray-tracer-challenge-netcore

My attempt at implementing the The Ray Tracer Challenge book in .NET Core and C#
MIT License
26 stars 5 forks source link

Suggestion: Concrete types for Vector and Point #6

Open y2k4life opened 5 years ago

y2k4life commented 5 years ago

RE: Concrete Tuple Types

I got the book!

In one of the first post you reason about concrete classes for vector and point. I think I have some suggestions. You can review my code referenced above.

After getting into Chapter 6 and having Tuple types throughout the code and not knowing if the property or variable is a Point or a Vector I decided to look into making concrete types.

I was able to build the concrete types Tuple, Vector, and Point, it appears to be a lot of duplicate code, but the code that is duplicated are "one liners" of code. Mainly the operator methods, returning their respective type after doing an operation instead of Tuple with the W set to 0 or 1 to designate a vector or point.

I had built four classes, a RtBaseTuple which the other three are derived from, RtTuple, RtVector, and RtPoint. The base class has all the properties along with the operations, Dot, Magnitude, that return the same type regardless of the derived type. The derived classes have methods in them that will return their specific type after doing *, /, +, Normalized, Cross, etc.

I believe the author did not want to complicate things with multiple things. Also by way of using some magic was able to have a Vector created from a Point by doing math. I removed some of the magic by implying the rules instead of having them inferred. If you subtract a point from a point return a vector instead of tuple that is a vector because the W went to 0 after subtracting 1 from 1.

In my RtPoint class I have this, subtract two points and return a vector.

static public RtVector operator -(RtPoint left, RtPoint right)
    => new RtVector(left.X - right.X, left.Y - right.Y, left.Z - right.Z);

I think the biggest challenge and reason for duplicate code for a derived type is not being able to return a different type. There is no way to have a class Tuple with a virtual Negate or Normalized that you can override with a different return type . In the derived class Vector you can't override Normalized and return Vector, it is expecting Tuple to be returned. Something about co-variance. But even if you did have something like this you would still end up with duplicate code or code wrapping other code.

I think the thing that hit me was the author eluding to having Color be like a tuple and reuse the code, but there was no way of doing that and if you did you still end up with duplicate code trying to wrap X around Red, Y around Green and Z around Blue, and ignoring the W property. Hence why there is a class just for color. Also in part of the book there is math that breaks the magic, after the operation is done you have to reset the W value.

This hacking was a testament to having a boat load of tests. After screwing around and getting everything to pass the test, my exercises still worked as they should.

simonech commented 5 years ago

Thank you. Appreciate the comments.

I got a bit blocked by other “real life” stuff. I’ll have a look at what you did, but not too much as it would “spoil” my learning experience.

Making both Tuple and Color inherit from the same class is what probably I’ll do, with the base class being an array (instead of wrapping Red on X, etc.)

Sent from my iPhone

On 24 Jun 2019, at 23:28, Guy notifications@github.com wrote:

RE: Concrete Tuple Types

I got the book!

In one of the first post you reason about concrete classes for vector and point. I think I have some suggestions. You can review my code referenced above.

After getting into Chapter 6 and having Tuple types throughout the code and not knowing if the property or variable is a Point or a Vector I decided to look into making concrete types.

I was able to build the concrete types Tuple, Vector, and Point, it appears to be a lot of duplicate code, but the code that is duplicated are "one liners" of code. Mainly the operator methods, returning their respective type after doing an operation instead of Tuple with the W set to 0 or 1 to designate a vector or point.

I had built four classes, a RtBaseTuple which the other three are derived from, RtTuple, RtVector, and RtPoint. The base class has all the properties along with the operations, Dot, Magnitude, that return the same type regardless of the derived type. The derived classes have methods in them that will return their specific type after doing *, /, +, Normalized, Cross, etc.

I believe the author did not want to complicate things with multiple things. Also by way of using some magic was able to have a Vector created from a Point by doing math. I removed some of the magic by implying the rules instead of having them inferred. If you subtract a point from a point return a vector instead of tuple that is a vector because the W went to 0 after subtracting 1 from 1.

In my RtPoint class I have this, subtract two points and return a vector.

static public RtVector operator -(RtPoint left, RtPoint right) => new RtVector(left.X - right.X, left.Y - right.Y, left.Z - right.Z); I think the biggest challenge and reason for duplicate code for a derived type is not being able to return a different type. There is no way to have a class Tuple with a virtual Negate or Normalized that you can override with a different return type . In the derived class Vector you can't override Normalized and return Vector, it is expecting Tuple to be returned. Something about co-variance. But even if you did have something like this you would still end up with duplicate code or code wrapping other code.

I think the thing that hit me was the author eluding to having Color be like a tuple and reuse the code, but there was no way of doing that and if you did you still end up with duplicate code trying to wrap X around Red, Y around Green and Z around Blue, and ignoring the W property. Hence why there is a class just for color. Also in part of the book there is math that breaks the magic, after the operation is done you have to reset the W value.

This hacking was a testament to having a boat load of tests. After screwing around and getting everything to pass the test, my exercises still worked as they should.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

y2k4life commented 5 years ago

Not too many spoilers, mainly chapter 1 stuff, but I did not realize the impact until later chapters.

I have been lucky so far without having other “real life” stuff impact my progress but that will all start to change in a couple of days.

hakanai commented 3 years ago

I'm having a similar struggle over in my Kotlin version. I initially implemented all three as just Tuple.

Then once I wanted more implementations of color than what a tuple could store, I had to extract Color as its own thing so that it could have multiple implementations down the track.

Now I'm trying to do the same for Point and Vector but those are significantly harder. In a local branch I have deleted my Tuple class completely and have just implemented Point and Vector separately, which isn't too much duplication anyway, like you say. The hardest thing is massaging the test suite to separate the two concepts.

And yes the W value broke me at some point too, I think it was in the Matrix * Vector part, where the W value is just reset to zero after doing the transform. In my case I've resolved it by dropping the matrix to a 3x3 before doing the multiply, which probably saves a bunch of operations anyway. Another benefit of having separated the Vector and Point classes I guess?

What I'm wondering now is how much I would regret things if I also removed W for points. If it always has to be 1.0 anyway, why store it?