Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Book 3.8.3: Naming of onb::local() #1088

Closed
Panjaksli opened this issue Nov 5, 2022 · 2 comments
Closed

Book 3.8.3: Naming of onb::local() #1088

Panjaksli opened this issue Nov 5, 2022 · 2 comments

Comments

@Panjaksli
Copy link

Hello, sorry if my question is dumb.
But why is this method called "local",
when we are actually transforming from local space to world space ?

@trevordblack trevordblack self-assigned this Jun 19, 2023
@trevordblack trevordblack added this to the v4.0.0-books23 milestone Jun 19, 2023
@hollasch hollasch modified the milestones: v4.0.0-book3, v4.0.0 Jul 20, 2023
@hollasch hollasch changed the title Book 3, Onb local method Book 3.8.3: Naming of onb::local() Apr 19, 2024
@hollasch hollasch assigned hollasch and unassigned trevordblack Apr 25, 2024
@hollasch
Copy link
Collaborator

hollasch commented Apr 25, 2024

"Local" is relative. I can't speak for Peter's original intent, but I would call this a model-to-local coordinate transform. But every space's "world" coordinates is some other space's "model" coordinates, because transforms concatenate. I definitely wouldn't call this a transform to "world space", as we may have many other transforms ahead of us. For example, a given quad has modeling coordinates, but then may be transformed to assemble a box, which is then transformed into the Cornell scene, which is then inverse-transformed by the camera (or not, depending on how you model viewing transforms).

All that said, I do agree that local() doesn't really provide much value as a name. I'll see if I can come up with a better one.

@hollasch
Copy link
Collaborator

After thinking about this a bit more, and given the overall generality of the orthonormal basis class, I don't think this can be made any more specific than transform(). It transforms points from the orthonormal basic coordinate system into the coordinates of the basis vectors themselves. I really don't like calling these "world" coordinates, but also don't want to name them anything else, as that would be like saying that a vec3 is necessarily in some particularly named coordinate system. I think transform() is as good as it gets.

hollasch added a commit that referenced this issue Apr 27, 2024
The `onb` class implemented several unused methods, so I've removed them
in the interest of simplicity. These include the operator[] methods to
retrieve the basis axes, as well as a transform function that took three
individual double values. Finally, instead of having an uninitialized
form with a `build_from_w()` function, the class now requires a vector
at construction time, consistent with our use of the class.

I've renamed `local()` to `transform()` for a bit more clarity, without
trying to name either the input space or the result space, as "local"
means different things at different times, and to different people.

Resolves #1088
hollasch added a commit that referenced this issue Apr 29, 2024
The `onb` class implemented several unused methods, so I've removed them
in the interest of simplicity. These include the operator[] methods to
retrieve the basis axes, as well as a transform function that took three
individual double values. Finally, instead of having an uninitialized
form with a `build_from_w()` function, the class now requires a vector
at construction time, consistent with our use of the class.

I've renamed `local()` to `transform()` for a bit more clarity, without
trying to name either the input space or the result space, as "local"
means different things at different times, and to different people.

Resolves #1088
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants