Menu
▾
▴
1 Attachments
#195 g-raw converter
This patch adds a g-raw converter in response to this GCI task: http://www.google-melange.com/gci/task/view/google/gci2012/7945223
I'm glad to see that somebody is working on a g-raw converter. At a first sight it looks good but it needs some polishment. E.g. it still contains a “G - S T L . C” in the comments and the copyright has to start in 2013.
Furthermore I run on an error during the compilation:
/home/rossberg/Devel/brlcad/src/conv/raw/g-raw.c: In function ‘nmg_to_raw’:
/home/rossberg/Devel/brlcad/src/conv/raw/g-raw.c:150:13: error: variable ‘facet_normal’ set but not used [-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors
Remember, in the default configuration all warnings during a BRL-CAD build are treated as errors.
Thanks for the feedback. I have fixed those problems and attached a new patch.
You are almost done:
- b isn't an option any more therefore you should omit it from the usage string
- the new-line at line 197 is unusual for a raw file, i.e. you should remove this line
After these two small changes your patch would be a perfect rewrite of g-stl to g-raw (and you should do it this way).
However, after looking through the code I realized that there is a mismatch in opening and closing of the output file: It will be opened and closed twice. But this is a problem common to g-stl and g-raw and maybe an issue for another patch.
Sorry, I should have checked the patch better.
I have fixed those problems and also added a brlman page.
Also, where are the repeated file operations? I would like to fix that but I couldn't find them.
Including the man page in the patch was a good idea:)
One issue: The g-raw copyright starts in 2013 (on the man page).
BTW, the usage strings in the C files and the man pages differ (for both g-stl and g-raw). E.g. the i flag is missing in the man page's usage string but it's explained in the text.
About the repeated file operations: I made a mistake there. I haven' noticed the output_file vs. output_directory logic.
I updated the usage strings and man pages, and made them a little more readable (the options are now formatted into paragraphs rather than being in a mass of text).
Ups, the usage strings in the C files were OK. Especially, you shouldn't add the program name (g-stl, g-raw) to it. The %s at the begin of the string will be replaced by the actual program name whatever it is.
Furthermore you should consider to split your patch in a g-raw and g-stl part.
Fixed the usage and split the patch into two parts.
In the manual pages you write "-P#", "-D#", "-a#" etc.. Do you have a reference for this style, i.e. not writing "-P num_cpus", "-D calculation_tolerance", "-a absolute_tol"?
From a quick grep it seems to be used in 32 brlcad manual pages. I saw it in rt, rtcheck, mged, rtwalk.
Last edit: Jonathan Engbert 2013-07-29
Congratulation, your patch was accepted and applied as revisions 56307 and 56321. Your named is credited in our AUTHORS file.
Great, thank you!