#76 rt_ell_brep, rt_ehy_brep & rt_hyp_brep functions

closed-accepted
Cliff Yapp
8
2012-05-16
2012-03-30
Phoenix
No

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.

Discussion

1 2 > >> (Page 1 of 2)
  • Sean Morrison
    Sean Morrison
    2012-03-30

    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.

     
  • Sean Morrison
    Sean Morrison
    2012-03-30

    • status: open --> pending
     
  • Phoenix
    Phoenix
    2012-03-31

    The patch has been updated. Is it OK?

     
  • Phoenix
    Phoenix
    2012-04-01

    • status: pending --> open
     
  • Phoenix
    Phoenix
    2012-04-01

    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.

     
  • Phoenix
    Phoenix
    2012-04-01

    • summary: rt_ell_brep --> rt_ell_brep & rt_ehy_brep functions
     
  • Phoenix
    Phoenix
    2012-04-02

    • summary: rt_ell_brep & rt_ehy_brep functions --> rt_ell_brep, rt_ehy_brep & rt_hyp_brep functions
     
  • Sean Morrison
    Sean Morrison
    2012-04-03

    That is outstanding, Wu! Really nice work.

     
  • Sean Morrison
    Sean Morrison
    2012-04-03

    • priority: 5 --> 8
     
  • Sean Morrison
    Sean Morrison
    2012-04-03

    The indentation looks much better now. Thanks for fixing it!

     
  • Phoenix
    Phoenix
    2012-04-08

     
    Attachments
  • Phoenix
    Phoenix
    2012-04-08

     
    Attachments
  • Phoenix
    Phoenix
    2012-04-08

     
    Attachments
  • Cliff Yapp
    Cliff Yapp
    2012-05-13

    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?

     
  • Phoenix
    Phoenix
    2012-05-13

    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?

     
  • Cliff Yapp
    Cliff Yapp
    2012-05-13

    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.

     
  • Cliff Yapp
    Cliff Yapp
    2012-05-15

    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!

     
  • Cliff Yapp
    Cliff Yapp
    2012-05-15

    • labels: 622307 --> Geometry Conversion
    • assigned_to: nobody --> starseeker
    • status: open --> closed-accepted
     
  • Cliff Yapp
    Cliff Yapp
    2012-05-15

    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.

     
  • Cliff Yapp
    Cliff Yapp
    2012-05-15

    • status: closed-accepted --> open-postponed
     
  • Phoenix
    Phoenix
    2012-05-16

    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.

     
  • Cliff Yapp
    Cliff Yapp
    2012-05-16

    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...

     
  • Phoenix
    Phoenix
    2012-05-16

    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.

     
  • Cliff Yapp
    Cliff Yapp
    2012-05-16

    • status: open-postponed --> closed-accepted
     
  • Cliff Yapp
    Cliff Yapp
    2012-05-16

    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.

     
1 2 > >> (Page 1 of 2)