-
Notifications
You must be signed in to change notification settings - Fork 6
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
Some code debugging. #19
Conversation
- Removed the necessity for ReverseWindingOrder - Solved console error with ThreeJS color
Hello @hanbyul-here Not sure how you want to handle this here since it's my first pull request since you've made me a committer on this repo. I'm asking this because I'v seen you do pull requests on your own repo. Not sure why you want to handle things this way. But in this case here. Should I merge it? Or do you prefer to review it before? Just arking how you want to handle things on your repo. Cheers Julien |
Hey @xuv ! Sorry about late response! I was occupied with other stuffs and also actually wanted to understand better about D3's stream. This rewinding faces stuffs was something I know it is ugly but just doing it cuz it works. I really appreciate your efforts on it :) There are some little changes I want to make (ex. I want to keep background as black even if it was from not working background) . You mind me pushing little changes to this branch (and merge it to master)? |
Don't mind any change. Please go ahead. I figured you might want to keep the background black. :) I just wanted to get the error out of the way. :) |
Let me know also how you want to handle code collaboration. I'm planning to refactor a bit more while I go deeper. Like I want to find out a way to color features in 3D. So that I can better figure out what is what. |
The link was a great help, thanks! I like making pr even it is for myself so few-hours(or days)-future-me can review the pr. If you can do pr and assign me, I will try my best to respond to it. Also it will be great if you can notice me about your plan through issues (and assign yourself. If it is a feature request, or general bug, you don't have to assign yourself!), so I can roughly expect what is coming from you! I will try to set up lint soon so we can write in coherent style. |
Right now, I'm trying to "organize" the code a bit more. Not that it's not good, but since I want to add functionalities, I'm worried it's going to become long bits of code and that it will be harder to figure out what it does. |
Hello @hanbyul-here
Just what I did here is solve an error on color from Threejs. So now we have the grey background has you intended.
I also removed the use for ReverseWindingOrder function on the generated mesh. The function is still in the code, but never called. Actually, inverting the Y coordinates on the generated mesh vertices messed up the faces. So I removed that function too and changed the projection function from D3js so that the generated path come directly as they are expected and not upside down.
This simplifies the code a little bit.
I'm actually going through this in search of getting rid of all the errors in the javascript console and at the same time getting familiar with the code so I could later implement the options I'd like to add.