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

NormalNode not fully fixed #17126

Closed
4 tasks done
njarraud opened this issue Jul 30, 2019 · 6 comments
Closed
4 tasks done

NormalNode not fully fixed #17126

njarraud opened this issue Jul 30, 2019 · 6 comments
Labels
Milestone

Comments

@njarraud
Copy link
Contributor

njarraud commented Jul 30, 2019

Description of the problem

NormalNode has been updated a few days ago however I do not believe that it has fixed the issue with the LOCAL scope.

See jsfiddle with the dev version of three.js

  • Objects displaying world normal seem to be represented properly: normal doesn't depend on the camera but only on the orientation of the object.

  • Objects displaying local normal seem to be represented incorrectly: normal shall not depend on the camera and shall be identical independently of the object orientation. This is not the case and it currently behaves the same way as the view normal.

  • Objects displaying view (camera) normal seem to be represented properly: normal are based on the camera view.

The solution with vObjectNormal presented in this post seems to help but I don't know if it is a proper fix or only works in some cases.

NormalMapNode output doesn't seem to be correct either and it may be related to this issue. I will do some more testing on this one. Once we manage to get NormalNode outputting the expected values, we shall then recheck normalMapNode and BumpMapNode.

normal

I created the exact same configuration in Blender where normals are properly represented for all 3 scopes. You can download the blend file here.

normalBlender

/ping @sunag

Three.js version
  • Dev
  • r106
Browser
  • All of them
OS
  • All of them
@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 31, 2019

/ping @sunag

@sunag
Copy link
Collaborator

sunag commented Jul 31, 2019

@njarraud Thank your nice explanation... I will see this in this weekend.

The solution with vObjectNormal presented in this post seems to help but I don't know if it is a proper fix or only works in some cases.

Yes, I want to remove requires property as soon as possible. I need before fix NormalNode.LOCAL fix other nodes that use LOCAL instead of VIEW.

@sunag
Copy link
Collaborator

sunag commented Aug 5, 2019

@njarraud This is already fixed but should I publishing the PR withing a few days.

@njarraud
Copy link
Contributor Author

njarraud commented Aug 5, 2019

Thanks. Really looking forward to testing it!

@WestLangley WestLangley added this to the r108 milestone Aug 20, 2019
@sunag
Copy link
Collaborator

sunag commented Aug 26, 2019

@njarraud Sorry the delay, I fix the NormalNode.NORMAL. (I put VIEW by mistake 4d5fdc7) You can see in this PR #17265. Revision 8, contains big changes, for this reason, it can take a while

@mrdoob mrdoob modified the milestones: r108, r109 Aug 27, 2019
@njarraud
Copy link
Contributor Author

Awesome. I currently use my own branch with quick fixes for the problems I encountered. Modifications look good. I'll try them when PR will be merged with the dev branch. Thanks again.

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

No branches or pull requests

5 participants