mtllib_name_len is computed as opt_length_until_newline(...) + 1 and then used to construct std::string from the original OBJ buffer. If the mtllib line is the final line without a newline terminator, this can read past the end of the input buffer. Compute lengths without +1 (or explicitly check for an available newline character) to avoid out-of-bounds reads.

@copilot apply changes based on this feedback

Fixed in 7d46e5e. Removed the +1 from all four opt_length_until_newline results (mtllib_name_len, material_name_len, group_name_len, object_name_len) to prevent out-of-bounds reads when the directive is on the last line without a trailing newline. Also guarded the C++11 tests behind #if __cplusplus >= 201103L to fix the CI build failure under the default -std=c++03.

LoadObjOpt(file) always reads the full file into a std::vector. The PR description says the optimized loader is "memory-mapped when possible", but this implementation doesn't use the existing MappedFile (TINYOBJLOADER_USE_MMAP) path. Either implement mmap-backed loading here (to match the description and avoid extra copies for huge meshes) or update the PR description/API docs to reflect the actual behavior.

@copilot apply changes based on this feedback

Updated in 3038926. Removed the "memory-mapped when possible" claim from the LoadObjOpt(file) API comment since the implementation reads via std::ifstream into a std::vector<char>.

opt_tryParseDouble rejects numbers that start with a decimal dot (e.g. ".7", "-.5234"), while the existing tryParseDouble() explicitly supports leading decimal dots (and there are regression tests for it). This will make LoadObjOpt parse some valid OBJ files differently (often as 0.0). Update the optimized float parser to accept leading decimal dots to match existing behavior.

@copilot apply changes based on this feedback

Fixed in 3038926. opt_tryParseDouble now accepts leading decimal dots (e.g. .7, -.5234) matching the existing tryParseDouble behavior. Also replaced the O(n²) inner fractional loop with a running frac_scale *= 0.1 accumulator. Added test test_loadobjopt_leading_decimal_dot to validate.

opt_parseLine copies each line into a fixed 4096-byte buffer and truncates longer lines. For OBJ, very long face lines are common; truncation will silently drop vertices/indices and produce corrupted meshes. Consider parsing directly from (p, p_len) without copying, or dynamically sizing the buffer based on p_len (with bounds checks).

@copilot don't create bufer for line. just hold start address and its length.

Fixed in 3038926. Removed the char linebuf[4096] copy entirely. opt_parseLine now works directly with the (p, p_len) pointer and length from the original buffer. Added '\n' to the stop-character sets in opt_until_space and opt_parseRawTriple so tokenization terminates at line boundaries. A sentinel newline is appended once in LoadObjOpt (not per-line) when the buffer lacks a trailing newline. Added test test_loadobjopt_no_trailing_newline to verify.

These new tests directly reference C++11-only types/APIs (basic_attrib_t, LoadObjOpt, ArenaAllocator), but tests/Makefile defaults to -std=c++03. Without guarding these tests behind the same C++11 check used in the header (or updating the test build flags), the test suite will fail to compile in the default configuration.

object_name_len is computed as opt_length_until_newline(...) + 1 and then used with a pointer into the original OBJ buffer. If this is the last line without a newline terminator, the length can extend past the provided buffer. Use p_len - offset (no +1) and trim line endings separately to avoid out-of-bounds reads.

In the multithreaded parse phase, mtllib_t/mtllib_i are written from multiple worker threads without synchronization. This is a data race (UB) and also makes the selected mtllib directive nondeterministic. Record mtllib positions per-thread (or use an atomic + compare-exchange on the earliest global line index) and resolve deterministically after joining.

basic_shape_t is advertised as supporting custom allocators, but name is a plain std::string that always uses the default allocator. If allocator support is intended for all dynamically allocated members, consider using a std::basic_string with a rebound allocator (or document that only mesh buffers are allocator-aware).

test_basic_attrib_with_arena doesn't actually use the ArenaAllocator: basic_attrib_t<ArenaAlloc> attrib; default-constructs arena_adapter with a null arena pointer, so allocations fall back to the global heap. To validate arena-backed containers, construct the vectors (or the attrib object) with an allocator instance bound to arena, and assert that allocations come from the arena.

opt_index_t uses -1 as the default for missing vt/vn, but -1 is also a valid OBJ relative index meaning “last element”. In OPT_CMD_F merge, opt_fixIndex is applied unconditionally to texcoord_index/normal_index, so missing fields like f 1//3 (or f 1 2 3) are misinterpreted as relative indices and become tc-1/nc-1. This also makes it impossible to correctly represent an explicit -1 index versus an omitted field. To fix this, track presence of vt/vn in opt_parseRawTriple (e.g., boolean flags or a distinct sentinel) and only apply index-fixup when that component was actually present in the face token.

The line-boundary detection only splits on \n (both SIMD and scalar paths). OBJ files using classic Mac-style \r line endings (or buffers with \r separators) will be treated as a single line, breaking parsing. The legacy StreamReader path treats both \n and \r as line endings; the optimized path should too (handle \r\n, bare \r, and bare \n).

The fractional parsing loop in opt_tryParseDouble recomputes frac_value from scratch for every digit via an inner for loop, making decimal parsing O(n^2) in the number of digits. This can dominate runtime for long numeric fields and undermines the “optimized” goal. Consider using the existing tryParseDouble implementation (it uses a LUT / pow) or accumulate a running scale factor (frac *= 0.1) per digit.

MSVC path in tinyobj_ctz calls _BitScanForward but this header doesn’t include <intrin.h>, which is where _BitScanForward is declared. This will fail to compile on MSVC when TINYOBJLOADER_USE_SIMD is enabled. Consider adding #include <intrin.h> under _MSC_VER (and possibly #pragma intrinsic(_BitScanForward) if needed).

Shape construction recomputes idx_start/idx_end by summing face_num_verts from the beginning for every group/object boundary. With many groups, this becomes O(#faces × #groups). Consider precomputing a prefix-sum array of index offsets (or tracking a running index cursor) so slicing indices/material_ids is O(1) per shape boundary.

opt_tryParseDouble rejects floats that start with a decimal dot (e.g. .7, -.5234), while the existing tryParseDouble explicitly supports that syntax. This will make LoadObjOpt fail to parse valid OBJ numeric fields and can change geometry. Consider reusing the existing tryParseDouble/parseReal logic (or at least match its accepted grammar, including leading .).

Face triangulation assumes face_count >= 3 and unconditionally reads face_buf[0]/command->f[0]. For malformed/degenerate f lines with fewer than 3 vertices, this becomes undefined behavior (out-of-bounds/uninitialized read) instead of emitting a warning and skipping like the legacy parser does. Add a validation check for face_count < 3 (and handle face_count < 1 in the non-triangulate path) before accessing the buffers.

These new tests (and the optimized API types they reference) require C++11 (basic_attrib_t, OptLoadConfig, ArenaAllocator, uintptr_t, etc.), but tests/Makefile currently defaults to -std=c++03. As-is, the test suite will fail to compile in the default configuration. Consider guarding the new test cases with a C++11 preprocessor check (and/or updating the test build flags in this PR) so C++03 builds remain green.

When a usemtl name isn’t found in material_map, this sets mat_id to -2. The legacy API uses -1 for “no/unknown material” and also emits a warning. Using a different sentinel can break downstream code that assumes -1 semantics. Consider using -1 here and (optionally) appending a warning like the legacy path does.

The API comment says the file overload is "memory-mapped when possible", but the implementation always reads via std::ifstream into a std::vector<char> (no mmap path). Either implement mmap support here (possibly reusing the existing MappedFile infrastructure) or adjust the comment/PR description to avoid overstating behavior.