Adding centroid and surface area for ellipsoid primitive
Open Source Solid Modeling CAD
Brought to you by:
brlcad
Menu
▾
▴
Functions for ell centroid and surface area.
The patch looks much better, but still does need some work. Please respond to the following issues with an updated patch (to this tracker, not a new one unless it's for a new patch to some other file):
1) The oblate and prolate functions should be marked HIDDEN (which makes them static) and not use the rt_ prefix. That prefix would imply they're public API and they shouldn't be.
2) The indentation is all over the place with some places using tabs and others using spaces. Clean up the indentation to conform with the HACKING style guidelines. The sh/indent.sh and sh/ws.sh scripts can be run on the file or you can try running the latest astyle or you can make the few corrections manually (you should learn/know how if you don't). Looking at your file in simple text editor may help.
3) The centroid function manually copies the vertex point. We have functions and macros for that which should be used. See the VMOVE() macro in include/vmath.h for example.
That's it actually, just those three issues. The style looks great along with the naming conventions and comments. Please also comment on or close out your other first-submitted tracker patch item.
Full ell.c changes after using astyle
ell.c changes without astyle
Oblate and Prolate functions have been changed accordingly.
Changed to using VMOVE, I need to look at other macro's as well as there look to be lots of very useful ones.
I'm not sure how indentation is shown in the patch files, to me it looks wrong but I assume it's parsed in a certain way. I added two patches, the first one uses astyle on the ell.c file and alters other parts of that, the second only shows what I have added, without using astyle. Also setup my visual studio environment for the correct tab/indent lengths
That looks much much better. There are indeed lots of useful functions and macros that you should explore as you get deeper into your coding.
How do the patch files look wrong to you? They look correct here at a quick glance. You're right that the astyle-formatted patch ended up editing too much of the file. Apparently there are a couple other places not following our style that need correcting. If you were committing to the repository, you'd perform a separate commit to clean up the file, then apply your changes. As a patch, though, you did the right thing and just submit a patch that has your changes.
Someone else just beat you to the centroid function, so can you do one last update to merge with the latest and remove that function?
Just the Surface area functions
Maybe not so much wrong, as odd, the way it compresses down the indents makes it look wrong.
I'll remember to clean up files for future use. Will check through the simulation files now to make sure they're in order before I start on them
Anyway, uploaded a patch with just the surface area functions now.
Every time I read the patch, I seem to find something else... I just noticed that you're making calls to fabs() and using a 0.0001 magic tolerance number, which is not at all robust (what if we're doing molecular modeling or the sphere is simply smaller than that?). I don't fault you for it this time because see you followed suit to someone else's bad practice in ell.c in the bbox routine. You are at fault, however, for a patch that fails strict compilation due to the unused 'type' variable.
Regardless of those two issues, your patch was applied in r50724 along with corrections for those items. Please review and test the merged version.
Also, if it's "compressing down the indents", then you probably have your tabstops set wrong. We use 4-char indents with tabstops at 8 characters. Basically, it's a fixed view form that should look the same even with traditional text file tools like cat, more, and others.