-
Notifications
You must be signed in to change notification settings - Fork 142
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
Build on Windows - incorrectly saved .ppm format #9
Comments
Yes, this issue had already been adressed by this commit. |
I love your writeups and walkthroughs! Thanks for putting them together. However, a big obstacle for beginners is that the first code snippets (in both tinyraycaster and tinyraytracer repositories) do not produce matching output when copied/pasted and compiled on Windows machines. I understand that you and many other enthusiasts develop on Linux, but there is nowhere (that I could find) where the reader is warned that the code must be modified in order to follow along on Windows. A bold note at the beginning or quick reminder under the code snippet would be much appreciated. Thanks again for all your hard work. |
From my experience, bold reminders are ignored by most readers. That is why I keep this current issue open even if in later commits the bug was fixed. |
It's not only about ignoring reminders. It's about someone, who is trying to start from the very beginning step by step, as you recommend in your tutorials, and taking the link to initial commit to try it himself. Then he realizes that output is different comparing to what it should be and it's unclear why, and it takes some time to find where the problem hides. Also for the beginner it's not trivial to find the problem himself. |
I do agree with you, this is problematic, but I do not see a satisfactory solution to the problem. I do create bugs (a lot). This repository is a real history of one of my saturdays, not something I prepared for months before. While modifying the very first commit is possible, I do not master git sufficiently to feel a firm ground under my feet with this kind of solution. |
Just notice: I think, this is possible to edit it directly on github just as text file, but I can't guarantee consequences. |
well... go and ask there? |
You can rewrite git history :) You can edit the early commits, and force push the repo to update it. I find that doing this via command line is no fun, and github doesn't provide any tools for that either - the power tool you want is smartgit - it's free for use in open source development. When you open this repo using smartgit, you can go to the log and either move the commit that fixes the issue back in the history, or right click on the first commits and edit them, rewriting the history. It's fairly unproblematic. |
It's not, it will break the repo on future git pulls for everyone who cloned it pre-rewrite, and it'll also change all commit hashes anyway (so at least direct links to commits won't be fixed with this). completely unrelated minor side note: maybe this repo would be a good match to be tagged |
Wow!Very thank you,i have been in this problems for more than 2H!!! |
Hi!
First of all thank you for great tutorials on 3D graphics.
There is an issue when building project on Windows with GCC. In initial commit in file
tinyraycaster.cpp
in functiondrop_ppm_image
you have:On Windows GCC replaces
\n
in output file with platform specific code0x0D0A
, what is incorrect (it should be0x0A
in the file). This result in incorrect *.ppm image representation.This happens because of opening file in "text" mode. To fix this, file should be opened in "binary" mode, for example like this:
Then everything works correctly. I think this patch would not affect other platforms.
The text was updated successfully, but these errors were encountered: