The original version of function rt_ell_brep() can only deal with ellipsoids centered at the origin and A, B, C set along the X, Y, Z axis. This new one add rotation and translation to it, so it can deal with all ellipsoids.
Excellent work Phoenix, but I do see some room for improvements. If you look at the math library functions and macros you're using (vmath.h and bn.h), you'll see that MAT3X3VEC() does the transformation you're manually doing to the origin and curve points. Especially for math code that expands to a mess, it's almost always better to call existing API -- even if it means copying some data into temporaries -- because the code becomes easier to understand and maintain. Still, nice work regardless (assuming it works, haven't tested!) as this was a non-trivial patch.
The patch has been updated. Is it OK?
I also modify the rt_ehy_brep function which has the same problems. Before, only the bottom cap of ehy can be correctly drawn. Now it works well with the curve surface. Rotation and Translation are added so that all ehys can be dealt with.
That is outstanding, Wu! Really nice work.
The indentation looks much better now. Thanks for fixing it!
Testing the hyp conversion, it looks like the top and bottom plates may be "inside-out" - if I V_REVERSE y_dir after the VCROSS in hyp_brep.cpp:53, I get a correct raytrace. Can anyone confirm?
Sorry I didn't notice it before. But I find that just V_REVERSE y_dir is not enough, for the representation of the elliptical hyperboloic NURBS surface may be wrong. So I think we should also V_REVERSE Bu in hyp_brep.cpp:178 after VSCALE(Bu, y_dir, 1/MAGNITUDE(y_dir)).
But I don't know how to find whether a plate is "inside-out". Could you help me?
There are a couple ways - the more common one is to raytrace from multiple views and see if the shape "looks right" - that's how I noticed the hyp was off.
More rigorously, there is an option to the brep command to draw surface normals, although I note it's missing from the brep command usage statement (code is in src/librt/primitives/brep/brep_debug.cpp):
brep brep.s plot SN #
where # is the number of the individual surface you want normals for. Right now it's just drawing lines, so you might want to enhance what it's drawing slightly to make sure you know which way the line is pointing - sometimes it's clear from the wireframe context, but in isolation you'll probably want some kind of arrowhead.
The changes provided have been applied to the latest sources and should be included in the next release of BRL-CAD. You're encouraged to make sure that the changes were applied correctly and are working as expected. Thank you for the patch!
Actually Wu, need to re-open this for the hyp patch. The placement of the hyp for non-origin primitives was improved, but the diff test indicates there is a difference in the hyp surface shape compared to the CSG solid:
comb hyp1.c u hyp.s - hyp.s_brep
comb hyp2.c u hyp.s_brep - hyp.s
Raytracing both hyp1.c and hyp2.c should result in a clean (empty) raytrace with no image. Not precisely sure what all the problems are, but hyp definitely needs revisiting.
When I created this patch, I had found slight difference between hyp and hyp_brep when displaying them, but I think it was due to the inaccuracy of displaying. When raytracing the result of the substraction, I found the difference's much larger than I imagine. But I also found that the result of raytracing the implicit primitive hyp alone is very strange (the angle was wrong?). I wonder if it's the reason why large differences are shown in the substraction.
When you say the implicit angle looked "wrong", what do you mean? Is there a problem with the CSG implicit definition?
If I remember correctly, hyp is intended to define a closed volume based on a hyperboloid of one sheet: http://en.wikipedia.org/wiki/Hyperboloid
If we have a problem with the implicit hyp primitive definition that's definitely worth fixing...
Sorry I didn't make it clear. I mean that when we display the CSG hyp using the draw command and raytrace it using the rt command, the results look differently.
Closing this out - the issues with hyp need to be investigated - verify (and if need be, fix) implicit hyp shape definition, determine necessary changes to NURBS surface definitions to properly match it, etc. but those are separate issues from those addressed by these patches. Should be noted as TODO items for this GSoC project.