BrunoLevy / geogram

a programming library with geometric algorithms
Other
1.87k stars 128 forks source link

fix: boolean_op execute crash #95

Closed grassofsky closed 1 year ago

grassofsky commented 1 year ago

fix some errors in boolean operation, details in commit.

BrunoLevy commented 1 year ago

Hi, Would you tell me under which circumstances the crash was occuring ?

grassofsky commented 1 year ago

Hi, Would you tell me under which circumstances the crash was occuring ?

Hi,

the commit resovles two problems.

First,

    void mesh_union(Mesh& result, Mesh& A, Mesh& B) {
        mesh_boolean_operation(result, A, B, "A+B");
    }

    void mesh_intersection(Mesh& result, Mesh& A, Mesh& B) {
        mesh_boolean_operation(result, A, B, "A*B");
    }

    void mesh_difference(Mesh& result, Mesh& A, Mesh& B) {
        mesh_boolean_operation(result, A, B, "A-B");
    }

all above functions pass to mesh_boolean_operation is "A+B" or "AB" or "A-B". So, I modify union as A+B, intersection as AB.

Second

I run below command, exe crashed.

boolean_op.exe D:/teData/Mesh/spheren.obj D:/teData/Mesh/cuben.obj out-difference.obj operation=difference
 _____________________________________________________________________________
|                                                                             |
| o-[Process     ] Using Windows thread pool                                  |
|                  Multithreading enabled                                     |
|                  Available cores = 16                                       |
|                  Max used threads = 16                                      |
|                  Cancel mode disabled                                       |
| o-[config      ] Configuration file name:geogram.ini                        |
|                  Home directory:C:/Users/xiewei.zhong                       |
   ______________________
 _/ =====[Data I/O]===== \____________________________________________________
|                                                                             |
| o-[I/O         ] Output = out-difference.obj                                |
|                  Loading file D:/teData/Mesh/spheren.obj...                 |
|                  (FP64) nb_v:2562 nb_e:0 nb_f:5120 nb_b:0 tri:1 dim:3       |
|                  Attributes on vertices: point[3]                           |
|                  Loading file D:/teData/Mesh/cuben.obj...                   |
|                  (FP64) nb_v:8 nb_e:0 nb_f:12 nb_b:0 tri:1 dim:3            |
|                  Attributes on vertices: point[3]                           |
   _______________________________
 _/ =====[Boolean operation]===== \___________________________________________
|                                                                             |
| o-[Detect isect] Elapsed time: 0.469 s                                      |
| o-[Intersect   ] Intersections: 204                                         |
|                  Intersected triangles: 102                                 |
|                  Max intersections in triangle: 68                          |
| o-[Remesh isect] Elapsed time: 0.156 s                                      |
| o-[Validate    ] Detected non-manifold vertices                             |
|                     (fixed by generating 612 new vertices)                  |
|                  (FP64) nb_v:3386 nb_e:0 nb_f:5948 nb_b:816 tri:1 dim:3     |
|                  Attributes on vertices: exact_point,point[3]               |
|                  Attributes on facets: operand_bit                          |
Abnormal program termination: C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.31.31103\include\xstring(1941) : Assertion failed: cannot dereference string iterator because it is out of range (e.g. an end iterator)

After debugging, I find when call BooleanExprParser::curchar, the ptr will be expr_.end, so crash.

I fix this problem by add below logic in cur_char:

            if (ptr_ == expr_.end())
                return '\0';

the returned '\0' char will not influence other logics.

Thank you.

BrunoLevy commented 1 year ago

Thanks ! I'll take a look at that asap.

BrunoLevy commented 1 year ago

Hi,

Best wishes, -- Bruno

grassofsky commented 1 year ago

Hi,

  • Thank you very much for having detected the overflow when ptr == expr.end()

  • In the lower level, I do not want to hardwire expressions such as "A+B", "A-B" etc..., they are parsed by the expression parser (because one can also compute more complicated n-ary operations like (AB)+C ...), and the special keywords "union" and "intersection" are different from "A+B" and "AB" (union means union of everything, and intersection means intersection of everything, remember, the operation can be n-ary).

Best wishes, -- Bruno

Thanks for your replying. I will check the usage of union and intersection.