commit 06ea5f14c0a9a42d3eeb49e767eefd355ff5c7a0
parent 9a5a03aed72702075a55429b61454e464f53f8eb
Author: Christophe Coustet <christophe.coustet@meso-star.com>
Date: Thu, 21 Aug 2025 17:56:58 +0200
Code quality.
Fix warnings, improve comments, arg. testing and log messages.
Diffstat:
5 files changed, 90 insertions(+), 50 deletions(-)
diff --git a/src/scad.c b/src/scad.c
@@ -368,7 +368,7 @@ scad_stl_get_data_partial
int tags_initialized = 0;
res_T res = RES_OK;
- if(!geometry || !triangles) {
+ if(!geometry || !triangles || (!dont && dont_count > 0)) {
res = RES_BAD_ARG;
goto error;
}
@@ -428,8 +428,13 @@ scad_stl_get_data_partial
}
ASSERT(tcount == darray_double_size_get(triangles));
if(count0 == tcount) {
- log_message(dev, "No triangle collected for geometry '%s'.\n",
- str_cget(&geometry->name));
+ if(str_is_empty(&geometry->name)) {
+ log_message(dev, "No triangle collected for unnamed geometry '%p'.\n",
+ (void*)geometry);
+ } else {
+ log_message(dev, "No triangle collected for geometry '%s'.\n",
+ str_cget(&geometry->name));
+ }
}
exit:
@@ -794,7 +799,11 @@ scad_stl_export
struct str filename;
int initialized = 0;
- if(!geometry || (!file_name && str_is_empty(&geometry->name))) {
+ if(!geometry || (!file_name && str_is_empty(&geometry->name)) ||
+ (orientation != SCAD_KEEP_NORMALS_UNCHANGED &&
+ orientation != SCAD_FORCE_NORMALS_OUTWARD &&
+ orientation != SCAD_FORCE_NORMALS_INWARD))
+ {
res = RES_BAD_ARG;
goto error;
}
@@ -842,7 +851,12 @@ scad_stl_export_partial
struct str filename;
int initialized = 0;
- if(!geometry) {
+ if(!geometry || (!file_name && str_is_empty(&geometry->name)) ||
+ (!dont && dont_count > 0) ||
+ (orientation != SCAD_KEEP_NORMALS_UNCHANGED &&
+ orientation != SCAD_FORCE_NORMALS_OUTWARD &&
+ orientation != SCAD_FORCE_NORMALS_INWARD))
+ {
res = RES_BAD_ARG;
goto error;
}
diff --git a/src/scad.h b/src/scad.h
@@ -59,7 +59,7 @@ enum scad_verbosity_levels {
/* Mesh algorithms */
enum scad_mesh_algorithm {
SCAD_MESHADAPT = 1,
- SCAD_AUTOMATIC = 2, /* Delaunay on planes, meshAdapt elsewhere */
+ SCAD_AUTOMATIC = 2, /* Delaunay on planes, mesh adapt elsewhere */
SCAD_INITIAL_MESH_ONLY = 3, /* Avoid new point creation */
SCAD_DELAUNAY = 5,
SCAD_FRONTAL_DELAUNAY = 6,
@@ -155,7 +155,7 @@ enum scad_normals_orientation {
/* A type to specify how partitioning is done. */
enum scad_partition_flags {
SCAD_ALLOW_OVERLAPPING = BIT(0),
- SCAD_DUMP_ON_OVERLAPPING_ERROR = BIT(1)
+ SCAD_DUMP_ON_OVERLAPPING_ERROR = BIT(1) /* Dump individual geometries to STL */
};
BEGIN_DECLS
@@ -207,9 +207,7 @@ scad_scene_clear
/* Write the whole scene in a format that depends on filename extension.
* Available formats include "brep", "msh" (gmsh-specific format), "step",
- * "stl", "vtk", etc.
- * Note that due to gmsh behaviour, writing a scene that never contained any
- * geometry ends in error. */
+ * "stl", "vtk", etc. */
SCAD_API res_T
scad_scene_write
(const char* filename);
@@ -236,11 +234,11 @@ scad_add_rectangle
const double dxdy[2],
struct scad_geometry** rectangle);
-/* Add a disk in (xy) plane to the scene, defined by a the center `xyz' and
+/* Add a disk in the (xy) plane to the scene, defined by its `center' and
* `radius'. */
SCAD_API res_T
scad_add_disk
- (const double xyz[3],
+ (const double center[3],
const double radius,
struct scad_geometry** disk);
@@ -263,21 +261,21 @@ scad_add_box
const double dxdydz[3],
struct scad_geometry** box);
-/* Add a `cylinder' to the scene, defined by the center `xyz' of its first
- * circular face, the vector `axis' defining its axis and its radius `rad'. The
- * `angle' argument defines the angular opening (from 0 to 2*PI). */
+/* Add a `cylinder' to the scene, defined by the center `center' of its first
+ * circular face, the vector `axis' defining its axis and its radius `radius'.
+ * The `angle' argument defines the angular opening (from 0 to 2*PI). */
SCAD_API res_T
scad_add_cylinder
- (const double xyz[3],
+ (const double center[3],
const double axis[3],
const double radius,
const double angle,
struct scad_geometry** cylinder);
-/* Add a `sphere' of center `xyz' and radius `rad' to the scene. */
+/* Add a `sphere' of center `center' and radius `radius' to the scene. */
SCAD_API res_T
scad_add_sphere
- (const double xyz[3],
+ (const double center[3],
const double radius,
struct scad_geometry** sphere);
@@ -510,7 +508,7 @@ scad_geometries_intersect
/* Compute the boolean fragments (general fuse) resulting from the
* intersection of the geometries in `geometries', making all interfaces
* conformal.
- * `flags' should be made by Oring values from enum scad_partition_flags to
+ * `flags' should be made by ORing values from enum scad_partition_flags to
* enable non default behaviours (default is to disallow overlapping).
* The output geometries are created unnamed.
* When applied to geometries of different dimensions, the lower dimensional
@@ -651,17 +649,17 @@ scad_step_import
SCAD_API res_T
scad_stl_export
(struct scad_geometry* geometry,
- const char* filename,
+ const char* filename, /* Can be NULL if geometry name is defined */
const enum scad_normals_orientation orientation,
- const int binary); /* File format */
+ const int binary);
/* Same as previous, but geometries that are part of `exclude' are not exported. */
SCAD_API res_T
scad_stl_export_partial
(struct scad_geometry* geometry,
- struct scad_geometry** exclude,
+ struct scad_geometry** exclude, /* Can be NULL */
const size_t exclude_count,
- const char* filename,
+ const char* filename, /* Can be NULL if geometry name is defined */
const enum scad_normals_orientation orientation,
const int binary);
@@ -671,9 +669,9 @@ scad_stl_export_partial
SCAD_API res_T
scad_stl_export_split
(struct scad_geometry* geometry,
- const char* filename,
+ const char* filename, /* Can be NULL if geometry name is defined */
const enum scad_normals_orientation orientation,
- const int binary); /* File format */
+ const int binary);
/* Accumulate the mesh of the geometry `geometry' into `triangles', each triangle
* being described by 9 doubles in a STL way.
@@ -688,7 +686,7 @@ scad_stl_get_data
SCAD_API res_T
scad_stl_get_data_partial
(struct scad_geometry* geometry,
- struct scad_geometry** exclude,
+ struct scad_geometry** exclude, /* Can be NULL */
const size_t exclude_count,
struct darray_double* triangles);
diff --git a/src/scad_device.c b/src/scad_device.c
@@ -314,6 +314,7 @@ scad_dump_geometries
res_T res = RES_OK;
struct scad_device* dev = get_device();
struct htable_geometries_iterator it, end;
+ size_t cpt = 0;
ERR(check_device(FUNC_NAME));
@@ -326,8 +327,10 @@ scad_dump_geometries
while(!htable_geometries_iterator_eq(&it, &end)) {
struct scad_geometry* geom = *htable_geometries_iterator_key_get(&it);
ERR(scad_dump_geometry(geom));
+ cpt++;
htable_geometries_iterator_next(&it);
}
+ printf("Counted %ld geometries.\n", cpt);
exit:
return res;
diff --git a/src/scad_device.h b/src/scad_device.h
@@ -56,7 +56,9 @@ hash_str(const struct str* a)
#include <rsys/hash_table.h>
enum delete_policy {
+ /* Delete the tag and all its lower-dimension components */
Scad_delete_recursive,
+ /* Delete only the tag, not its lower-dimension components */
Scad_delete_non_recursive,
Scad_do_not_delete
};
diff --git a/src/scad_geometry.c b/src/scad_geometry.c
@@ -811,7 +811,7 @@ scad_geometries_clear_mesh
struct scad_device* dev = get_device();
struct mem_allocator* allocator = NULL;
- if(!geometries) {
+ if(!geometries || geometries_count == 0) {
res = RES_BAD_ARG;
goto error;
}
@@ -1569,6 +1569,7 @@ scad_geometries_fuse
* no longer used by any star-cad geometry */
gmshModelOccFuse(data1, sz1, data2, sz2, &tagout, &tagoutn, &map, &mapn,
&mapnn, -1, 0, 0, &ierr);
+ ASSERT(tagoutn % 2 == 0);
ERR(gmsh_err_to_res_T(ierr));
ERR(geometry_create(&geom));
@@ -1673,6 +1674,7 @@ scad_geometries_cut
* no longer used by any star-cad geometry */
gmshModelOccCut(data1, sz1, data2, sz2, &tagout, &tagoutn, &map, &mapn,
&mapnn, -1, 0, 0, &ierr);
+ ASSERT(tagoutn % 2 == 0);
ERR(gmsh_err_to_res_T(ierr));
ERR(geometry_create(&geom));
@@ -1745,6 +1747,7 @@ scad_geometries_intersect
* no longer used by any star-cad geometry */
gmshModelOccIntersect(data1, sz1, data2, sz2, &tagout, &tagoutn, &map, &mapn,
&mapnn, -1, 0, 0, &ierr);
+ ASSERT(tagoutn % 2 == 0);
ERR(gmsh_err_to_res_T(ierr));
ERR(geometry_create(&geom));
@@ -1985,9 +1988,6 @@ scad_geometry_extrude
int *tagout = NULL;
size_t tagoutn;
size_t i;
-#ifndef NDEBUG
- size_t j;
-#endif
int *ed, *extrude_data = NULL;
size_t extrude_sz = 0;
int ierr = 0;
@@ -2007,6 +2007,24 @@ scad_geometry_extrude
ERR(check_device(FUNC_NAME));
allocator = dev->allocator;
+ /* OCC cannot extrude 3D geometries */
+ for(i = 0; i < geom->gmsh_dimTags_n; i += 2) {
+ const int dim = geom->gmsh_dimTags[i];
+ if(dim != 2) {
+ if(str_is_empty(&geom->name)) {
+ log_error(get_device(),
+ "Cannot extrude unnamed 3D geometry '%p'.\n",
+ (void*)geom);
+ } else {
+ log_error(get_device(),
+ "Cannot extrude 3D geometry '%s'.\n",
+ str_cget(&geom->name));
+ }
+ res = RES_BAD_ARG;
+ goto error;
+ }
+ }
+
gmshModelOccExtrude(geom->gmsh_dimTags, geom->gmsh_dimTags_n, SPLIT3(dxdydz),
&tagout, &tagoutn, NULL, 0, NULL, 0, 0, &ierr);
ERR(gmsh_err_to_res_T(ierr));
@@ -2027,12 +2045,6 @@ scad_geometry_extrude
for(i = 0; i < tagoutn; i += 2) {
int dim = tagout[i];
int tag = tagout[i+1];
-#ifndef NDEBUG
- /* Expecting geom's tags not part of tagout */
- for(j = 0; j < geom->gmsh_dimTags_n; j += 2) {
- ASSERT(dim != geom->gmsh_dimTags[j] || tag != geom->gmsh_dimTags[j+1]);
- }
-#endif
if(dim == 3) {
*ed++ = dim;
*ed++ = tag;
@@ -2070,15 +2082,12 @@ scad_geometry_extrude
const int tag = geom->gmsh_dimTags[i+1];
struct tag_desc* desc = device_get_description(dim, tag);
ASSERT(dim == 2);
- if(!desc) {
- res = RES_BAD_OP;
- goto error;
- }
+ ASSERT(desc != NULL);
/* Need to protect input geometry's tags by getting a ref on output tags or
* deleting out_geometry will possibly delete them */
ERR(device_register_ref_to_tags(dim, tag, extrude_data, extrude_sz));
/* As the 2D tags will be deleted when the 3D tag they are part of are
- * deleted, they shouldn't be deleted when the geometry they belongs to are
+ * deleted, they shouldn't be deleted when the geometry they belongs to is
* released. */
desc->delete_policy = Scad_do_not_delete;
}
@@ -2203,6 +2212,8 @@ scad_geometry_copy
data1 = geom->gmsh_dimTags;
gmshModelOccCopy(data1, sz1, &tagout, &tagoutn, &ierr);
get_device()->need_synchro = 1;
+
+ ASSERT(tagoutn % 2 == 0);
ERR(gmsh_err_to_res_T(ierr));
ERR(geometry_create(©));
@@ -2438,7 +2449,7 @@ scad_geometries_partition
"Unnamed geometry '%p' overlapping (dumped in '%s).\n",
(void*)g, str_cget(&tmp));
} else {
- log_error(get_device(), "Could not dump geoemtry.\n");
+ log_error(get_device(), "Could not dump geometry.\n");
}
} else {
str_printf(&tmp, "%s_partition_error_%lu_%lu",
@@ -2450,7 +2461,7 @@ scad_geometries_partition
"Geometry '%s' overlapping (dumped in '%s).\n",
str_cget(&g->name), str_cget(&tmp));
} else {
- log_error(get_device(), "Could not dump geoemtry.\n");
+ log_error(get_device(), "Could not dump geometry.\n");
}
}
}
@@ -2573,6 +2584,9 @@ error:
}
if(tagout) {
gmshModelOccRemove(tagout, tagoutn, 1, &ierr);
+ /* Do not check ierr, first because we are in error handler already, but
+ * also because an error is expected if these OCC entities have allready
+ * been freed through the call to ref_put above */
}
goto exit;
}
@@ -2679,8 +2693,7 @@ scad_geometries_swap
for(n = 0; n < c2; n += 2) {
int dim = dt2[n];
int tag = dt2[n+1];
- if(n) { ERR(str_append_printf(&msg, ",")); }
- ERR(str_append_printf(&msg, " %d.%d", dim, tag));
+ ERR(str_append_printf(&msg, (n ? ", %d.%d" : "%d.%d"), dim, tag));
}
logger_print(dev->logger, dev->log_type, "%s.\n", str_cget(&msg));
if(str_is_empty(&g2->name)) {
@@ -2734,7 +2747,7 @@ scad_geometries_boundary
struct str msg, name;
int msg_initialized = 0, name_initialized = 0;
- if(!geometries || !out_boundaries || !out_count) {
+ if(!geometries || geometries_count == 0 || !out_boundaries || !out_count) {
res = RES_BAD_ARG;
goto error;
}
@@ -2936,7 +2949,7 @@ scad_geometry_get_normal
struct darray_int tags;
int initialized = 0;
double d, min_d = DBL_MAX, pcoord_min[2];
- int min_tag;
+ int min_tag = -1;
size_t normals_n;
if(!geom || !p || !N) {
@@ -3078,7 +3091,9 @@ scad_geometries_set_mesh_size_modifier
int* tagout[4] = { NULL, NULL, NULL, NULL };
size_t tagoutn[4] = { 0, 0, 0, 0}, i;
- if(!geometries || geometries_count == 0 || modifier <= 0) {
+ if(!geometries || geometries_count == 0 || modifier <= 0
+ || (type != SCAD_ABSOLUTE_SIZE && type != SCAD_SIZE_FACTOR))
+ {
res = RES_BAD_ARG;
goto error;
}
@@ -3137,7 +3152,14 @@ scad_geometries_set_mesh_algorithm
int* tagout[4] = { NULL, NULL, NULL, NULL };
size_t tagoutn[4], i;
- if(!geometries || geometries_count == 0) {
+ if(!geometries || geometries_count == 0 ||
+ (algorithm != SCAD_MESHADAPT &&
+ algorithm != SCAD_AUTOMATIC &&
+ algorithm != SCAD_INITIAL_MESH_ONLY &&
+ algorithm != SCAD_DELAUNAY &&
+ algorithm != SCAD_FRONTAL_DELAUNAY &&
+ algorithm != SCAD_QUASI_STRUCTURED))
+ {
res = RES_BAD_ARG;
goto error;
}
@@ -3178,7 +3200,8 @@ scad_geometries_set_periodic
int* src_dimTagout[4] = { NULL, NULL, NULL, NULL };
int* tgt_dimTagout[4] = { NULL, NULL, NULL, NULL };
int* src_tags = NULL, *tgt_tags = NULL;
- size_t src_dimTagoutn[4], tgt_dimTagoutn[4], src_tagsn = 0, tgt_tagsn = 0, i;
+ size_t src_dimTagoutn[4] = { 0,0,0,0 }, tgt_dimTagoutn[4] = { 0,0,0,0 };
+ size_t src_tagsn = 0, tgt_tagsn = 0, i;
if(!source || source_count == 0 || !target || target_count == 0 || !affine) {
res = RES_BAD_ARG;
