Repository navigation
Uploader LOD and GLTF load efficiency follow-ups - #571
Merged
Merged
Conversation
The header says target's faces are as they were where simplifyModel returns -1. Its fallback for a face that would take more vertices than 16-bit indices reach did not keep that: it ran simplifyFace on every face, which empties a face it cannot simplify, and the faces compacted before the one that did not fit were already written. A fallback that kept no triangle returned -1 with the target changed. simplifyModel now counts each face's vertices before it writes any face, and the fallback simplifies the faces into index lists first, through simplifyOwnFace, the body simplifyFace now shares: nothing is written unless a triangle is kept. No caller depends on either behaviour, and the fallback's faces are still simplifyFace's. The fallback had no test. A sloppy simplification, whose grid can take a triangle's vertices from other faces, is what reaches it, and the new case builds one: a sheet whose second face lies on its first, simplified as one mesh, puts 90,000 vertices under the first face. Each face of the result must be the face simplifyFace makes of it. With the fallback switched off the case fails on the compaction assert. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… again When the sloppy pass fails or keeps more than the precise modes did, Auto falls back to the closest of its ALL, UV and NONE passes. It got that result back by running simplify_model again in that mode. On a distant LOD with a triangle limit, where the prune check can run each mode's simplification twice, that was two of the eight attribute-aware simplifications Auto made of the model, where develop before the prim generation stack made four position-only ones. The mode loop now copies the closest mode's faces when a later pass is about to replace them, and the fallback assigns them back: the same faces bit for bit, as meshoptimizer is deterministic. Measured with the cascade replayed outside the viewer, develop's against this one, RelWithDebInfo, best of three: - A model Auto falls back on, two clusters of tiny cubes, each cluster as two faces at the same positions, 82,944 triangles: LOD 2 from 4 to 3 attribute-aware simplifications, 64.9 to 52.3 ms; LODs 1 and 0 from 8 to 6, 122.9 to 97.5 ms and 124.0 to 96.5 ms. - The mesh cache never takes the fallback: 1,285 models of 1,000 triangles or more, 3,855 LOD cases, 7,364 simplifications either way, 30.8 s against 30.6 s, every result identical. 389 of the cases made six, three modes each run twice by the prune check, and then took sloppy; the copy does not reach those. - Large uploads, bodies of 117,106 triangles and a garment of 78,807, never fall back either, and take the same time. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Primitive::prep unrolled each primitive the file gives no tangents, made MikkTSpace tangents for it with meshopt_generateTangents and welded it again with the tangents as a stream. Only the uploader and local meshes load GLTF now, through LLGLTFLoader, which reads positions, normals, texture coordinates, joints and weights and never mTangents. The tangents were most of prep's geometry work, and their stream split vertices the upload then carried. - A primitive without normals is still unrolled for its flat normals and welded again, now without tangents. - Any other surface primitive the file gives no tangents is welded in place, on the streams the unrolled weld used less the tangents: the same vertices in the same order, without the unrolled copy. Its strips and fans still become the triangle list the loader reads. - Points and lines get no tangents. Measured with prep replayed outside the viewer on the same accessor data, this commit's helpers copied in verbatim: every triangle is develop's, corner by corner in position, normal and texture coordinate, for every primitive as its file gives it, with its normals dropped, and for strips, fans and lists made up for it, indexed or not. Vertices as uploaded, develop against this: - CarConcept 173,510 to 162,766, AntiqueCamera 21,233 to 21,170, MosquitoInAmber 18,680 to 18,678. - A Blender export of 293,495 triangles 159,515 to 159,510; one of 1.9 million triangles, and the files that give their own tangents, unchanged. Prep's geometry work: the 1.9 million triangle file 613 to 58 ms, the 293,495 triangle one 95 to 7.6 ms, CarConcept 46 to 4.6 ms; flat normals, the large file with its normals dropped, 936 to 519 ms. Should GLTF scenes be rendered again, their tangents need making again. The compatible mode removed here also gave a triangle of no UV area a different w from the Blender MikkTSpace it replaced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from the two review passes over the prim generation stack (#556–#562, #567). Each commit is its own change; every number below comes from replaying the code outside the viewer, develop's against this branch's, RelWithDebInfo.
Auto mode keeps its closest result (8f11d19)
When sloppy fails or keeps more than the precise modes did, Auto fell back to the closest of its ALL, UV and NONE passes by simplifying again in that mode. The mode loop now copies the closest mode's faces when a later pass is about to replace them, and the fallback assigns them back. The output is bit-identical in every case measured.
This saved nothing on real models. None of the mesh cache's 3,855 LOD cases (1,285 models of 1,000+ triangles) and none of the large uploads tried (bodies of 117,106 triangles, a 78,807-triangle garment) reached that fallback. Simplification counts and times are the same before and after on all of them. Only a model built to fall back shows the saving: two clusters of tiny cubes, each cluster as two faces at the same positions, 82,944 triangles.
What Auto's time actually goes on, from the cache run:
simplifyModelagrees with its header (815ccfe)The header promises target's faces are as they were when
simplifyModelreturns -1. The fallback for a face over 16-bit indices broke that: it ransimplifyFaceon every face, which empties a face it cannot simplify. The code now counts each face's vertices before writing anything, and the fallback simplifies into index lists first, throughsimplifyOwnFace, the bodysimplifyFacenow shares. No caller depended on either behaviour.The fallback is only reachable through a sloppy simplification and had no test. The new test builds one that puts 90,000 vertices under one face, and fails with the fallback switched off.
GLTF primitives no longer make tangents nothing reads (d315437)
Primitive::prepunrolled every primitive without tangents, made MikkTSpace tangents for it, and welded it again with the tangents as a stream.LLGLTFLoadernever readsmTangents.Every triangle is identical to develop's, corner by corner. That holds for every primitive of the files tested, as they come, with their normals dropped (the flat-normal path), and for made-up strips, fans and lists, indexed or not.
If GLTF scenes are rendered again, tangents need generating again. The compatible mode removed here also wrote a different w than the old Blender MikkTSpace for triangles with no UV area.
Testing
llmath_tests: 459 cases (develop's 458 plus the new one), all passing apart from develop's own skip and expected failure.[almeshsimplifier]passes; its hidden real-mesh case passes on 1,450 cached meshes.llprimitive_tests: 49/49.llmodelpreview.cppandgltf/primitive.cppcompile against a rebuilt viewer PCH.Left out
meshopt_generateNormals(the uploader's Generate Normals) compares every pair of corners at a vertex inmergeNormalGroups. That's O(valence²): a 20,000-triangle fan point is about 2×10⁸ tests. The fix belongs in the registry's meshoptimizer patch with a baseline bump, so it's left for a separate change.🤖 Generated with Claude Code