Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Flipped Objects part in book 2 (remove flip_face class?) #482

Closed
oxine opened this issue Apr 18, 2020 · 4 comments
Closed

Flipped Objects part in book 2 (remove flip_face class?) #482

oxine opened this issue Apr 18, 2020 · 4 comments
Milestone

Comments

@oxine
Copy link
Contributor

oxine commented Apr 18, 2020

rec.set_face_normal(r, outward_normal);

I' ve noticed that we have already fliped the normal by rec.set_face_normal in hit function, the filp class seems to be useless.We may either remove the set normal function in rect::hit or remove the flip_face class.

@hollasch hollasch self-assigned this Apr 18, 2020
@hollasch
Copy link
Collaborator

Yow. Excellent comment. A lot of work, but we didn't realize that we'd greatly simplified this part of the code. :)

@hollasch hollasch added this to the v3.1.0 milestone Apr 18, 2020
@hollasch
Copy link
Collaborator

Putting this in the v3.1.0 milestone unless we decide otherwise.

@hollasch hollasch changed the title About the Flipped Objects part in book2 Flipped Objects part in book 2 (remove flip_face class?) Apr 18, 2020
@hollasch hollasch modified the milestones: v3.1.0, v3.2.0 May 1, 2020
@hollasch hollasch removed their assignment May 6, 2020
@D-K-E
Copy link

D-K-E commented May 16, 2020

I can also confirm that I was able to render cornell box without flip_face class and was quite surprised by not having the Empty Cornell box from listing 55.

hollasch added a commit that referenced this issue May 17, 2020
With the changes made to handle surface intersection from either side
(refer to #270), we no longer have need for the `flip_face` class. This
change removes the class and the associated text in the books.

Resolves #482
Resolves #588
@hollasch
Copy link
Collaborator

Note: see #270 for the original change that implemented proper ray-surface intersection from either side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants