Skip to content

Commit 2bd1b4b

Browse files
committed
include+lib+common: Update v.h with new v_arr implementation
The code is overall nicer to look at, however, some sacrifices had to be made. I didn't foresee how mmap() & friends play with this new header + array scheme (not well), so all that stuff makes ugly expensive copies now as a stopgap. Everything seems to work, tests pass, and array stuff is now much less verbose and overbuilt, so that's nice. I'll copy the commit message from the (for now) private v.h repo commit message here verbatim: v.h: Reimplement v_arr Technically this is v3 or even higher, but I think I'm getting close to my ideal for dynamic, reallocating arrays. Usage for the previous approach looked like this: // generate scaffolding for custom type, foo_t. This generated a rather // large blob of code that had a type-safe, generic interface via // dynamic dispatch. v_arr_def(foo_t) void dostuff(void) { v_arr(foo_t) my_arr = v_arr_new(foo_t); v_arr_add(my_arr, (foo_t){ ...}); v_arr_free(my_arr); } Benefits of this previous approach were that the generated code was type safe, and I liked having an explicit "array of T" type to make it more obvious there's more going on here than just a plain pointer. The drawbacks, however, were numerous: - Passing a v_arr(foo_t) by value was really awkward and bugprone. - It required the use of that v_arr_def(T) macro in a strategic place, once for each type one would want to put in an array. I was moderately okay with this, I'd usually put the codegen after structs and other types. - v_arr_def(T) was a bit awkward in that it required the type to not have spaces, so structs had to be typedef'd in order to use it. I generally try to avoid superfluous typedefs, and I like to call structs what they are, so this was unfortunate. - Accessing elements was a bit ugly, having to use the arbitrarily named 'items' element, e.g. 'my_arr.items[i] = ...' never really felt very good. - It violated the 'zero is initialization' principle that I quite like, meaning that one had to always initialize each array with v_arr_new(T), otherwise the macro wrappers around array ops would detect a NULL ops table and trigger an abort. I *really* don't like that, and it is exactly this misfeature that prompted this rewrite. This new implementation takes cues from Sean Barrett's delightful stb_ds.h[1], mainly it reminded me to take another look at the concept of a header + plain C array. I had played around with flexible struct array members before, but this approach is even nicer. I adapted the rest from the old implementation. This new approach is very nice. I'm sure there are bugs and subtleties to work out, but no more dynamic dispatch, heaps of macro codegen and awkward semantics. v_arr now appears as a plain C array. Check this out: foo_t *my_arr = { 0 }; // Or NULL, your choice v_arr_add(my_arr, (foo_t){ ...}); // Nice! // Array size & cap are now macros: printf("len: %zu, cap: %zu\n", v_arr_len(my_arr), v_arr_cap(my_arr)); // Accessing elements feels good again: my_arr[0].member = ...; v_arr_free(my_arr); // After this, my_arr == NULL I'm really quite happy with this. Thanks to Feoramund on Twitter[2] for reminding me to take a look at stb_ds.h. TODO: Custom allocator support, more operations & tests. [1]: http://nothings.org/stb_ds [2]: https://x.com/Feoramund/status/1995117603522576692
1 parent b3f4091 commit 2bd1b4b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+1006
-674
lines changed

include/v.h

Lines changed: 446 additions & 93 deletions
Large diffs are not rendered by default.

src/common/base64.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ static const int B64index[256] = {
7575
};
7676

7777
void *b64decode(const char *data, const size_t inputLength, size_t *outLength) {
78-
if (!inputLength) return "";
78+
if (!inputLength) return ""; // FIXME: NULL
7979
unsigned char *p = (unsigned char *)data;
8080
const size_t pad1 = inputLength % 4 || p[inputLength - 1] == '=';
8181
const size_t pad2 = pad1 && (inputLength % 4 > 2 || p[inputLength - 2] != '=');

src/common/cr_string.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,14 @@ char *stringCopy(const char *source) {
4444
return copy;
4545
}
4646

47+
char *stringCopyN(const char *source, size_t len) {
48+
if (!source || !len)
49+
return NULL;
50+
char *copy = malloc(len + 1);
51+
strncpy(copy, source, len);
52+
return copy;
53+
}
54+
4755
char *stringConcat(const char *str1, const char *str2) {
4856
ASSERT(str1); ASSERT(str2);
4957
char *new = malloc(strlen(str1) + strlen(str2) + 1);

src/common/cr_string.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#pragma once
1010

1111
#include <stdbool.h>
12+
#include <stddef.h>
1213

1314
/// Check if two strings are equal
1415
/// @param s1 Left string
@@ -28,6 +29,7 @@ bool stringEndsWith(const char *postfix, const char *string);
2829
/// @param source String to be copied
2930
/// @return New heap-allocated string
3031
char *stringCopy(const char *source);
32+
char *stringCopyN(const char *source, size_t len);
3133

3234
/// Concatenate given strings
3335
/// @param str1 Original string

src/common/fileio.c

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -105,14 +105,20 @@ file_data file_load(const char *file_path) {
105105
if (size == 0) {
106106
return (file_data){ 0 };
107107
}
108-
#ifndef WINDOWS
108+
#if !defined(WINDOWS)
109109
int f = open(file_path, 0);
110-
void *data = mmap(NULL, size, PROT_READ, MAP_SHARED, f, 0);
110+
if (f < 0)
111+
return (file_data){ 0 };
112+
unsigned char *data = mmap(NULL, size, PROT_READ, MAP_SHARED, f, 0);
111113
if (data == MAP_FAILED) {
112114
logr(warning, "Couldn't mmap '%.*s': %s\n", (int)strlen(file_path), file_path, strerror(errno));
113115
return (file_data){ 0 };
114116
}
115-
file_data file = (file_data){ .items = data, .count = size, .capacity = size };
117+
// FIXME: static v_arr?
118+
file_data file = { 0 };
119+
v_arr_add_n(file, data, size);
120+
munmap(data, size);
121+
// file_data file = (file_data){ .items = data, .count = size, .capacity = size };
116122
return file;
117123
#else
118124
FILE *file = fopen(file_path, "rb");
@@ -125,21 +131,14 @@ file_data file_load(const char *file_path) {
125131
buf[size] = '\0';
126132
}
127133
fclose(file);
128-
file_data filedata = (file_data){ .items = buf, .count = readBytes, .capacity = readBytes };
129-
return filedata;
130-
#endif
131-
}
132-
133-
void file_free(file_data *file) {
134-
if (!file || !file->items) return;
135-
#ifndef WINDOWS
136-
munmap(file->items, file->count);
137-
#else
138-
free(file->items);
134+
// FIXME: static v_arr?
135+
file_data file = { 0 };
136+
v_arr_add_n(file, buf, readBytes);
137+
free(buf);
138+
return file;
139+
// file_data filedata = (file_data){ .items = buf, .count = readBytes, .capacity = readBytes };
140+
// return filedata;
139141
#endif
140-
file->items = NULL;
141-
file->capacity = 0;
142-
file->count = 0;
143142
}
144143

145144
void write_file(file_data data, const char *filePath) {
@@ -160,7 +159,7 @@ void write_file(file_data data, const char *filePath) {
160159
}
161160
}
162161
logr(info, "Saving result in %s\'%s\'%s\n", KGRN, backupPath ? backupPath : filePath, KNRM);
163-
fwrite(data.items, 1, data.count, file);
162+
fwrite(data, 1, v_arr_len(data), file);
164163
fclose(file);
165164

166165
//We determine the file size after saving, because the lodePNG library doesn't have a way to tell the compressed file size
@@ -252,37 +251,28 @@ char *get_file_path(const char *input) {
252251
#endif
253252
}
254253

255-
#define chunksize 65536
254+
#define chunksize 4096
256255
//Get scene data from stdin and return a pointer to it
257256
file_data read_stdin(void) {
258257
wait_for_stdin(2);
259258

260259
char chunk[chunksize];
261260

262-
size_t buf_size = 0;
263-
unsigned char *buf = NULL;
264261
int stdin_fd = fileno(stdin);
265262
int read_bytes = 0;
263+
file_data file = { 0 };
266264
while ((read_bytes = read(stdin_fd, &chunk, chunksize)) > 0) {
267-
unsigned char *old = buf;
268-
buf = realloc(buf, buf_size + read_bytes + 1);
269-
if (!buf) {
270-
logr(error, "Failed to realloc stdin buffer\n");
271-
free(old);
272-
return (file_data){ 0 };
273-
}
274-
memcpy(buf + buf_size, chunk, read_bytes);
275-
buf_size += read_bytes;
265+
v_arr_add_n(file, chunk, read_bytes);
276266
}
277267

278268
if (ferror(stdin)) {
279269
logr(error, "Failed to read from stdin\n");
280-
free(buf);
281-
return (file_data){ 0 };
270+
v_arr_free(file);
271+
return NULL;
282272
}
283273

284-
buf[buf_size ] = 0;
285-
return (file_data){ .items = buf, .count = buf_size, .capacity = buf_size };
274+
file[v_arr_len(file)] = 0;
275+
return file;
286276
}
287277

288278
char *human_file_size(unsigned long bytes, char *stat_buf) {

src/common/fileio.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,12 @@ enum fileType {
2929
glb,
3030
};
3131

32-
typedef byte file_bytes;
33-
v_arr_def(file_bytes)
34-
typedef struct file_bytes_arr file_data;
32+
typedef byte * file_data;
3533

3634
enum fileType match_file_type(const char *ext);
3735
enum fileType guess_file_type(const char *path);
3836
char *human_file_size(unsigned long bytes, char *stat_buf);
3937
file_data file_load(const char *filePath);
40-
void file_free(file_data *file);
4138
// This is a more robust file writing function, that will seek alternate directories
4239
// if the specified one wasn't writeable.
4340
void write_file(file_data file, const char *path);

src/common/hashtable.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ uint32_t hashInit(void) {
2929
return FNV_OFFSET;
3030
}
3131

32-
uint32_t hashCombine(uint32_t h, uint8_t u) {
32+
static uint32_t hashCombine(uint32_t h, uint8_t u) {
3333
return (h ^ u) * FNV_PRIME;
3434
}
3535

src/common/hashtable.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ struct hashtable {
3333

3434
// Hash functions (using FNV)
3535
uint32_t hashInit(void);
36-
uint32_t hashCombine(uint32_t, uint8_t);
3736
uint32_t hashBytes(uint32_t, const void *, size_t);
37+
#define hash_item(hash, item) hashBytes(hash, &(item), sizeof((uintptr_t)(item)))
3838
uint32_t hashString(uint32_t, const char *);
3939

4040
struct hashtable *newHashtable(bool (*compare)(const void *, const void *), struct block **pool);

src/common/json_loader.c

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -259,13 +259,13 @@ struct transform parse_composite_transform(const cJSON *transforms) {
259259
return composite;
260260
}
261261

262-
struct cr_shader_node *check_overrides(struct mesh_material_arr file_mats, size_t mat_idx, const cJSON *overrides) {
263-
if (mat_idx >= file_mats.count) return NULL;
262+
struct cr_shader_node *check_overrides(struct mesh_material *file_mats, size_t mat_idx, const cJSON *overrides) {
263+
if (mat_idx >= v_arr_len(file_mats)) return NULL;
264264
const cJSON *override = NULL;
265265
if (!cJSON_IsArray(overrides)) return NULL;
266266
cJSON_ArrayForEach(override, overrides) {
267267
const cJSON *name = cJSON_GetObjectItem(override, "replace");
268-
if (cJSON_IsString(name) && stringEquals(name->valuestring, file_mats.items[mat_idx].name)) {
268+
if (cJSON_IsString(name) && stringEquals(name->valuestring, file_mats[mat_idx].name)) {
269269
return cr_shader_node_build(override);
270270
}
271271
}
@@ -297,24 +297,24 @@ static void parse_mesh(struct cr_renderer *r, const cJSON *data, int idx, int me
297297
long ms = us / 1000;
298298
logr(debug, "Parsing file %-35s took %li %s\n", file_name, ms > 0 ? ms : us, ms > 0 ? "ms" : "μs");
299299

300-
if (!result.meshes.count) return;
300+
if (!v_arr_len(result.meshes)) return;
301301

302302
struct cr_vertex_buf_param vbuf = {
303-
.vertices = (struct cr_vector *)result.geometry.vertices.items,
304-
.vertex_count = result.geometry.vertices.count,
305-
.normals = (struct cr_vector *)result.geometry.normals.items,
306-
.normal_count = result.geometry.normals.count,
307-
.tex_coords = (struct cr_coord *)result.geometry.texture_coords.items,
308-
.tex_coord_count = result.geometry.texture_coords.count,
303+
.vertices = (struct cr_vector *)result.geometry.vertices,
304+
.vertex_count = v_arr_len(result.geometry.vertices),
305+
.normals = (struct cr_vector *)result.geometry.normals,
306+
.normal_count = v_arr_len(result.geometry.normals),
307+
.tex_coords = (struct cr_coord *)result.geometry.texture_coords,
308+
.tex_coord_count = v_arr_len(result.geometry.texture_coords),
309309
};
310310
// Per JSON 'meshes' array element, these apply to materials before we assign them to instances
311311
const struct cJSON *global_overrides = cJSON_GetObjectItem(data, "materials");
312312

313313
// Copy mesh materials to set
314314
cr_material_set file_set = cr_scene_new_material_set(scene);
315-
for (size_t i = 0; i < result.materials.count; ++i) {
315+
for (size_t i = 0; i < v_arr_len(result.materials); ++i) {
316316
struct cr_shader_node *maybe_override = check_overrides(result.materials, i, global_overrides);
317-
cr_material_set_add(scene, file_set, maybe_override ? maybe_override : result.materials.items[i].mat);
317+
cr_material_set_add(scene, file_set, maybe_override ? maybe_override : result.materials[i].mat);
318318
if (maybe_override) cr_shader_node_free(maybe_override);
319319
}
320320

@@ -336,11 +336,11 @@ static void parse_mesh(struct cr_renderer *r, const cJSON *data, int idx, int me
336336
const cJSON *instances = pick_instances ? pick_instances : add_instances;
337337
if (!cJSON_IsArray(pick_instances)) {
338338
// Generate one instance for every mesh, identity transform.
339-
for (size_t i = 0; i < result.meshes.count; ++i) {
340-
cr_mesh mesh = cr_scene_mesh_new(scene, result.meshes.items[i].name);
339+
for (size_t i = 0; i < v_arr_len(result.meshes); ++i) {
340+
cr_mesh mesh = cr_scene_mesh_new(scene, result.meshes[i].name);
341341
cr_mesh_bind_vertex_buf(scene, mesh, vbuf);
342342

343-
cr_mesh_bind_faces(scene, mesh, result.meshes.items[i].faces.items, result.meshes.items[i].faces.count);
343+
cr_mesh_bind_faces(scene, mesh, result.meshes[i].faces, v_arr_len(result.meshes[i].faces));
344344
cr_instance m_instance = cr_instance_new(scene, mesh, cr_object_mesh);
345345
cr_instance_bind_material_set(scene, m_instance, file_set);
346346
cr_instance_set_transform(scene, m_instance, parse_composite_transform(cJSON_GetObjectItem(data, "transforms")).A.mtx);
@@ -355,13 +355,13 @@ static void parse_mesh(struct cr_renderer *r, const cJSON *data, int idx, int me
355355
if (!mesh_name) continue;
356356
// Find this mesh in parse result, and add it to the scene if it isn't there yet.
357357
cr_mesh mesh = -1;
358-
for (size_t i = 0; i < result.meshes.count; ++i) {
359-
if (stringEquals(result.meshes.items[i].name, mesh_name)) {
360-
mesh = cr_scene_get_mesh(scene, result.meshes.items[i].name);
358+
for (size_t i = 0; i < v_arr_len(result.meshes); ++i) {
359+
if (stringEquals(result.meshes[i].name, mesh_name)) {
360+
mesh = cr_scene_get_mesh(scene, result.meshes[i].name);
361361
if (mesh < 0) {
362-
mesh = cr_scene_mesh_new(scene, result.meshes.items[i].name);
362+
mesh = cr_scene_mesh_new(scene, result.meshes[i].name);
363363
cr_mesh_bind_vertex_buf(scene, mesh, vbuf);
364-
cr_mesh_bind_faces(scene, mesh, result.meshes.items[i].faces.items, result.meshes.items[i].faces.count);
364+
cr_mesh_bind_faces(scene, mesh, result.meshes[i].faces, v_arr_len(result.meshes[i].faces));
365365
cr_mesh_finalize(scene, mesh);
366366
}
367367
}
@@ -373,12 +373,12 @@ static void parse_mesh(struct cr_renderer *r, const cJSON *data, int idx, int me
373373
// For the instance materials, we iterate the mesh materials, check if a "replace" exists with that name,
374374
// if one does, use that, otherwise grab the mesh material.
375375
const cJSON *instance_overrides = cJSON_GetObjectItem(instance, "materials");
376-
for (size_t i = 0; i < result.materials.count; ++i) {
376+
for (size_t i = 0; i < v_arr_len(result.materials); ++i) {
377377
struct cr_shader_node *material = NULL;
378378
// Find the material we want to use. Check if instance overrides it, otherwise use mesh global one
379379
material = check_overrides(result.materials, i, instance_overrides);
380380
// If material is NULL here, it gets set to an obnoxious material internally.
381-
cr_material_set_add(scene, instance_set, material ? material : result.materials.items[i].mat);
381+
cr_material_set_add(scene, instance_set, material ? material : result.materials[i].mat);
382382
cr_shader_node_free(material);
383383
}
384384
cr_instance_set_transform(scene, new, parse_composite_transform(cJSON_GetObjectItem(instance, "transforms")).A.mtx);
@@ -387,13 +387,18 @@ static void parse_mesh(struct cr_renderer *r, const cJSON *data, int idx, int me
387387

388388
done:
389389

390-
result.meshes.elem_free = ext_mesh_free;
391-
ext_mesh_arr_free(&result.meshes);
392-
result.materials.elem_free = mesh_material_free;
393-
mesh_material_arr_free(&result.materials);
394-
vector_arr_free(&result.geometry.vertices);
395-
vector_arr_free(&result.geometry.normals);
396-
coord_arr_free(&result.geometry.texture_coords);
390+
// FIXME: impl elem_free
391+
for (size_t i = 0; i < v_arr_len(result.meshes); ++i)
392+
ext_mesh_free(&result.meshes[i]);
393+
// result.meshes.elem_free = ext_mesh_free;
394+
v_arr_free(result.meshes);
395+
for (size_t i = 0; i < v_arr_len(result.materials); ++i)
396+
mesh_material_free(&result.materials[i]);
397+
// result.materials.elem_free = mesh_material_free;
398+
v_arr_free(result.materials);
399+
v_arr_free(result.geometry.vertices);
400+
v_arr_free(result.geometry.normals);
401+
v_arr_free(result.geometry.texture_coords);
397402
}
398403

399404
static void parse_meshes(struct cr_renderer *r, const cJSON *data) {

src/common/loaders/formats/gltf/gltf.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ unsigned char *parse_buffer(const cJSON *data) {
7575
return NULL;
7676
}
7777
file_data data = file_load(uri_string);
78-
if (data.count != expected_bytes) {
79-
logr(warning, "Invalid buffer while parsing glTF. Loaded file %s length %zu, expected %zu", uri_string, data.count, expected_bytes);
78+
if (v_arr_len(data) != expected_bytes) {
79+
logr(warning, "Invalid buffer while parsing glTF. Loaded file %s length %zu, expected %zu", uri_string, v_arr_len(data), expected_bytes);
8080
}
81-
return data.items;
81+
return data;
8282
}
8383

8484
return NULL;
@@ -192,8 +192,8 @@ struct mesh *parse_glb_meshes(const char *data, size_t *meshCount) {
192192

193193
struct mesh *parse_glTF_meshes(const char *filePath, size_t *meshCount) {
194194
file_data contents = file_load(filePath);
195-
if (stringStartsWith("glTF", (char *)contents.items)) return parse_glb_meshes((char *)contents.items, meshCount);
196-
const cJSON *data = cJSON_Parse((char *)contents.items);
195+
if (stringStartsWith("glTF", (char *)contents)) return parse_glb_meshes((char *)contents, meshCount);
196+
const cJSON *data = cJSON_Parse((char *)contents);
197197

198198
const cJSON *asset = cJSON_GetObjectItem(data, "asset");
199199
if (asset) {

0 commit comments

Comments
 (0)