There was a problem hiding this comment.
This PR brings in a new high-performance OBJ parsing path (optimized whole-buffer loader + experimental stream loader) and expands the test/fuzzing infrastructure to validate parity with the legacy loader and improve robustness for large/complex inputs.
Changes:
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file| tests/tester.cc | Enables multithreaded build for tests and adds many new unit/parity tests for optimized + typed APIs. |
| tests/README.md | Documents additional make obj-fuzz / make llvm-fuzz targets. |
| tests/opt/loadobjopt_multithread.inc | Adds a large suite of parity and multithread determinism tests for optimized and stream loaders. |
| tests/obj-fuzz/README.md | Documents the structured differential fuzzer and how to run it. |
| tests/obj-fuzz/obj_fuzz.cc | Implements a deterministic structured OBJ generator and cross-check runner. |
| tests/obj-fuzz/Makefile | Build/run rules for the structured fuzzer. |
| tests/Makefile | Links the stream loader into tester and adds convenience targets/clean steps for fuzzers. |
| tests/llvm-fuzz/README.md | Documents the libFuzzer harness and intended cross-check scope. |
| tests/llvm-fuzz/Makefile | Build/run rules for the libFuzzer harness. |
| tests/llvm-fuzz/fuzz_loaders.cc | libFuzzer entry point exercising multiple parser paths with optional cross-check asserts. |
| tests/llvm-fuzz/corpus/seed_basic.obj | Adds a seed corpus OBJ to bootstrap fuzzing. |
| tests/fuzz_common.h | Shared helpers for splitting fuzz inputs and comparing parse results across loaders. |
| loader_example.cc | Updates the example material reader to use C++11 override. |
| experimental/stream/stream_obj_loader.h | Declares the experimental stream loader API and handler interface. |
| experimental/stream/stream_obj_loader.cc | Implements the experimental stream loader (chunked parsing + ordered replay). |
| experimental/stream/README.md | Documents scope/goals/limitations of the stream parser. |
| experimental/README.md | Mentions the new stream parser entry point under experimental/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| if (tokens.size() >= 4) { | ||
| real_t maybe_r = real_t(1.0); | ||
| if (ParseRealToken(tokens[3], &maybe_r)) { | ||
| if (tokens.size() == 4) { | ||
| event.has_vertex_weight = true; | ||
| event.vertex_weight = maybe_r; | ||
| } else { | ||
| real_t maybe_g = real_t(1.0); | ||
| if (!ParseRealToken(tokens[4], &maybe_g)) { | ||
| event.has_vertex_weight = true; | ||
| event.vertex_weight = maybe_r; | ||
| } else if (tokens.size() >= 6) { | ||
| real_t maybe_b = real_t(1.0); | ||
| if (ParseRealToken(tokens[5], &maybe_b)) { | ||
| event.has_vertex_weight = true; | ||
| event.vertex_weight = maybe_r; | ||
| event.has_color = true; | ||
| event.r = maybe_r; | ||
| event.g = maybe_g; | ||
| event.b = maybe_b; | ||
| } | ||
| } | ||
| } | ||
| } |
| void test_arena_adapter_overflow_guard() { | ||
| // Verify that arena_adapter::allocate rejects SIZE_MAX/sizeof(T) overflow. | ||
| // When TINYOBJLOADER_ENABLE_EXCEPTION is not defined, the allocator returns | ||
| // nullptr on overflow. When exceptions are enabled, it throws std::bad_alloc. | ||
| tinyobj::ArenaAllocator arena; | ||
| tinyobj::arena_adapter<double> adapter(&arena); | ||
| // Request an allocation that would overflow size_t when multiplied by | ||
| // sizeof(double)=8. SIZE_MAX / 8 + 1 overflows. | ||
| double *p = adapter.allocate(SIZE_MAX / sizeof(double) + 1); | ||
| TEST_CHECK(p == nullptr); |
| namespace tinyobj { | ||
| namespace experimental_stream { | ||
| struct StreamLoadConfig { | ||
| bool triangulate; | ||
| bool default_vcols_fallback; | ||
| int num_threads; | ||
| size_t chunk_line_count; | ||
|
|
||
| StreamLoadConfig() | ||
| : triangulate(true), | ||
| default_vcols_fallback(false), | ||
| num_threads(1), | ||
| chunk_line_count(4096) {} | ||
| }; | ||
|
|
||
| bool LoadObjStreamExperimental( | ||
| attrib_t *attrib, std::vector<shape_t> *shapes, | ||
| std::vector<material_t> *materials, std::string *warn, std::string *err, | ||
| std::istream *input, MaterialReader *readMatFn, | ||
| const StreamLoadConfig &config = StreamLoadConfig()); | ||
|
|
||
| bool LoadObjStreamExperimental( | ||
| attrib_t *attrib, std::vector<shape_t> *shapes, | ||
| std::vector<material_t> *materials, std::string *warn, std::string *err, | ||
| const char *filename, const char *mtl_basedir, | ||
| const StreamLoadConfig &config = StreamLoadConfig()); | ||
| } // namespace experimental_stream | ||
| } // namespace tinyobj |
|
Addressed @copilot's review in ed77593:
Follow-on hardening that #3 surfaced: including the stream header after TINYOBJLOADER_IMPLEMENTATION is defined pulls tiny_obj_loader.h into the TU a second time, which redefined every implementation symbol. Added a TINYOBJLOADER_IMPLEMENTATION_DEFINED guard around the implementation block. Verified make check passes under both clang++ and g++, with and without -DTINYOBJLOADER_ENABLE_EXCEPTION. |
Sorry, something went wrong.
Take-over of #424 (branch copilot/optimize-parser-for-huge-meshes, originally by the Copilot coding agent).
CI did not run on #424 because it was opened by a bot, and the branch carried 31 incremental commits. This PR re-applies the identical final tree as a single squashed commit on top of current release, pushed by a maintainer so the Unit/Python workflows trigger normally.
What's included
Verification
Closes #424.
🤖 Generated with Claude Code