From 9636b1190debf77c5da1cd74290327411348cc36 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 06:41:03 -0400 Subject: [PATCH 01/10] A mesh face of more indices than any real one fails its LOD's decode decodeVolumeFaces held a face's vertex count to the 65,535 its 16-bit indices reach, but put no bound on its index count. A TriangleList of zeros deflates about a thousand to one, so a LOD of about 250 KB passes the 256 MB unzip cap with a face of about 128M indices, and cacheOptimize spends seconds and gigabytes of scratch on it, on the mesh thread of every viewer that sees it. LL_SCULPT_MESH_MAX_FACE_INDICES allows 32 indices for each vertex the indices reach, 2,097,152 in all. Of 136,000 faces in a viewer's mesh cache the largest held 360,564, about six for each of its 61,401 vertices, and none held as many as 30 for each. A face past the bound fails its LOD before its indices are allocated, as a face of too many vertices does. Co-Authored-By: Claude Opus 5.5 --- indra/llmath/llvolume.cpp | 8 ++++ indra/llmath/llvolume.h | 6 +++ indra/llmath/tests/llvolume_test.cpp | 65 ++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/indra/llmath/llvolume.cpp b/indra/llmath/llvolume.cpp index a8cecf7f7a..b099bc08fc 100644 --- a/indra/llmath/llvolume.cpp +++ b/indra/llmath/llvolume.cpp @@ -2616,6 +2616,14 @@ bool LLVolume::decodeVolumeFaces(const LLSD& mdl) LL_WARNS() << "Incomplete triangle discarded from face! Indices count " << num_indices << " was not divisible by 3. face index: " << i << " Total: " << face_count << LL_ENDL; num_indices -= indices_to_discard; } + + if (num_indices > (size_t)LL_SCULPT_MESH_MAX_FACE_INDICES) + { + LL_WARNS() << "Invalid index count " << num_indices << " exceeds maximum for face index: " << i << LL_ENDL; + mVolumeFaces.clear(); + return false; + } + face.resizeIndices(static_cast(num_indices)); if (num_indices > 2 && !face.mIndices) diff --git a/indra/llmath/llvolume.h b/indra/llmath/llvolume.h index 4e41753984..39878cb0f8 100644 --- a/indra/llmath/llvolume.h +++ b/indra/llmath/llvolume.h @@ -214,6 +214,12 @@ constexpr U8 LL_SCULPT_FLAG_MASK = LL_SCULPT_FLAG_INVERT | LL_SCULPT_FLAG_MIRROR constexpr S32 LL_SCULPT_MESH_MAX_FACES = 8; +// The most indices a mesh face may hold: 32 for each of the 65,536 vertices +// its 16-bit indices reach. A large face has about six for each of its +// vertices, and the largest of 136,000 faces in a viewer's mesh cache held +// 360,564. A LOD with a face past this does not decode. +constexpr S32 LL_SCULPT_MESH_MAX_FACE_INDICES = 32 * 65536; + extern bool gDebugGL; class LLProfileParams diff --git a/indra/llmath/tests/llvolume_test.cpp b/indra/llmath/tests/llvolume_test.cpp index 70fd3dc5aa..1068574ec4 100644 --- a/indra/llmath/tests/llvolume_test.cpp +++ b/indra/llmath/tests/llvolume_test.cpp @@ -4350,6 +4350,71 @@ namespace } } + // One triangle of a mesh face as the wire carries it, its vertices on + // the unit cube's axes and each value sixteen bits, low byte first. + LLSD wireTriangle() + { + auto binary = [](std::initializer_list values) + { + LLSD::Binary bytes; + for (U16 value : values) + { + bytes.push_back((U8)(value & 0xff)); + bytes.push_back((U8)(value >> 8)); + } + return LLSD(bytes); + }; + LLSD face; + face["Position"] = binary({ 0, 0, 0, 65535, 0, 0, 0, 65535, 0 }); + face["TriangleList"] = binary({ 0, 1, 2 }); + face["PositionDomain"]["Min"] = LLVector3(-0.5f, -0.5f, -0.5f).getValue(); + face["PositionDomain"]["Max"] = LLVector3(0.5f, 0.5f, 0.5f).getValue(); + face["TexCoord0Domain"]["Min"] = LLVector2(0.f, 0.f).getValue(); + face["TexCoord0Domain"]["Max"] = LLVector2(1.f, 1.f).getValue(); + return face; + } + + // A LOD of the one face, decoded into a mesh volume of its own. + LLPointer decodeLOD(const LLSD& face, bool& decoded) + { + LLSD mdl = LLSD::emptyArray(); + mdl.append(face); + LLVolumeParams params; + params.setType(LL_PCODE_PROFILE_SQUARE, LL_PCODE_PATH_LINE); + params.setSculptID(LLUUID("00000000-0000-0000-0000-00000000000c"), LL_SCULPT_TYPE_MESH); + LLPointer volume = new LLVolume(params, LLVolumeLODGroup::getVolumeScaleFromDetail(3)); + decoded = ll_test::VolumeProbe::decode(*volume, mdl); + return volume; + } + + // A face's TriangleList of zeros deflates to almost nothing, so that one + // the unzip cap lets through could hold a hundred million indices, and + // cost every viewer that decodes it seconds and gigabytes. A LOD with a + // face of more indices than LL_SCULPT_MESH_MAX_FACE_INDICES does not + // decode; one with a face of as many as that does. + TEST_CASE("llvolume: a mesh face of more indices than a face may hold does not decode", "[llvolume]") + { + const S32 most = LL_SCULPT_MESH_MAX_FACE_INDICES - LL_SCULPT_MESH_MAX_FACE_INDICES % 3; + for (const S32 indices : { most, most + 3 }) + { + CAPTURE(indices); + LLSD face = wireTriangle(); + face["TriangleList"] = LLSD(LLSD::Binary((size_t)indices * sizeof(U16), 0)); + bool decoded = false; + LLPointer volume = decodeLOD(face, decoded); + CHECK(decoded == (indices <= LL_SCULPT_MESH_MAX_FACE_INDICES)); + if (decoded) + { + REQUIRE(volume->getNumVolumeFaces() == 1); + CHECK(volume->getVolumeFace(0).mNumIndices == indices); + } + else + { + CHECK(volume->getNumVolumeFaces() == 0); + } + } + } + // A circle path's stations step around it and end at its end cut. The // steps add up with rounding, and a station they bring a rounding short // of the end would leave a row with no height before the end's: no two From a60d81a111a9536952f15b641b1b4ce506e7a223 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 06:41:26 -0400 Subject: [PATCH 02/10] A mesh face's weights are not read past their end The weights loop checked once an influence that a byte was left, then read up to three: the influence's two bytes of weight and the next joint. A Weights array that ends inside an influence was read past its end, and the bytes after it taken for a weight and a joint. Each read is now checked first. An array that ends inside an influence ends there: its vertex keeps the influences before it, and the vertices after take no influence, as those an array runs out before already do. This is LL's code. Co-Authored-By: Claude Opus 5.5 --- indra/llmath/llvolume.cpp | 12 ++++++-- indra/llmath/tests/llvolume_test.cpp | 46 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/indra/llmath/llvolume.cpp b/indra/llmath/llvolume.cpp index b099bc08fc..a964b7712b 100644 --- a/indra/llmath/llvolume.cpp +++ b/indra/llmath/llvolume.cpp @@ -2828,8 +2828,16 @@ bool LLVolume::decodeVolumeFaces(const LLSD& mdl) U32 joints[4] = {0,0,0,0}; LLVector4 joints_with_weights(0,0,0,0); - while (joint != END_INFLUENCES && idx < weights.size()) + while (joint != END_INFLUENCES) { + // An influence is its joint and two bytes of weight; + // an array that ends inside one ends there, the + // vertex keeping the influences before it. + if (weights.size() - idx < 2) + { + idx = (U32)weights.size(); + break; + } U16 influence = weights[idx++]; influence |= ((U16) weights[idx++] << 8); @@ -2838,7 +2846,7 @@ bool LLVolume::decodeVolumeFaces(const LLSD& mdl) joints[cur_influence] = joint; cur_influence++; - if (cur_influence >= 4) + if (cur_influence >= 4 || idx >= weights.size()) { joint = END_INFLUENCES; } diff --git a/indra/llmath/tests/llvolume_test.cpp b/indra/llmath/tests/llvolume_test.cpp index 1068574ec4..1a1e448a0d 100644 --- a/indra/llmath/tests/llvolume_test.cpp +++ b/indra/llmath/tests/llvolume_test.cpp @@ -4415,6 +4415,52 @@ namespace } } + // A rigged face's weights are an influence after another, each a joint + // and two bytes of weight. An array that ends inside an influence leaves + // its vertex the influences before it, and is not read past its end. + TEST_CASE("llvolume: a mesh face whose weights end inside an influence keeps the influences before it", "[llvolume]") + { + struct Cut + { + const char* what; + LLSD::Binary weights; + S32 joint; // the first vertex's first joint, or -1 for none + }; + const Cut cuts[] = { + { "joint 5 at half, then joint 7 and one byte of its weight", { 5, 0xff, 0x7f, 7, 0xff }, 5 }, + { "joint 5 and one byte of its weight", { 5, 0xff }, -1 }, + }; + for (const Cut& cut : cuts) + { + CAPTURE(cut.what); + LLSD face = wireTriangle(); + face["Weights"] = LLSD(cut.weights); + bool decoded = false; + LLPointer volume = decodeLOD(face, decoded); + REQUIRE(decoded); + const LLVolumeFace& rigged = volume->getVolumeFace(0); + REQUIRE(rigged.mNumVertices == 3); + REQUIRE(rigged.mWeights); + for (S32 v = 0; v < rigged.mNumVertices; ++v) + { + CAPTURE(v); + const LLVector4a& weight = rigged.mWeights[v]; + if (v == 0 && cut.joint >= 0) + { + CHECK((S32)weight[0] == cut.joint); + } + else + { + // No influence: all of the weight on the first joint. + CHECK(weight[0] == 0.999f); + } + CHECK(weight[1] == 0.f); + CHECK(weight[2] == 0.f); + CHECK(weight[3] == 0.f); + } + } + } + // A circle path's stations step around it and end at its end cut. The // steps add up with rounding, and a station they bring a rounding short // of the end would leave a row with no height before the end's: no two From 5fee21f66eb2129dd99d3d84f6e6c9f8c7176fae Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 06:41:42 -0400 Subject: [PATCH 03/10] cacheOptimize copies only the attributes a face has LLVolumeFace::cacheOptimize takes normals and texture coordinates as optional, needing both only for tangents, but its write loop copied both from every face: one without them, as the DAE loader leaves a face whose source has none, was read through a null pointer. Only the mesh decode calls it today, and a decoded face has both. Each is now copied only where the face has it, and the reordered face comes out without what it came without, as ALMeshSimplifier's faces do. Co-Authored-By: Claude Opus 5.5 --- indra/llmath/llvolume.cpp | 21 +++++++- indra/llmath/tests/llvolume_test.cpp | 76 ++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 2 deletions(-) diff --git a/indra/llmath/llvolume.cpp b/indra/llmath/llvolume.cpp index a964b7712b..b2016ea529 100644 --- a/indra/llmath/llvolume.cpp +++ b/indra/llmath/llvolume.cpp @@ -5329,8 +5329,14 @@ bool LLVolumeFace::cacheOptimize(bool gen_tangents) const U32 from = first_corner ? indices[corner] : (U32)u; mPositions[to] = old_positions[from]; - mNormals[to] = old_normals[from]; - mTexCoords[to] = old_tex_coords[from]; + if (old_normals) + { + mNormals[to] = old_normals[from]; + } + if (old_tex_coords) + { + mTexCoords[to] = old_tex_coords[from]; + } if (old_weights) { mWeights[to] = old_weights[from]; @@ -5351,6 +5357,17 @@ bool LLVolumeFace::cacheOptimize(bool gen_tangents) mIndices[i] = (U16)place[ordered[i]]; } + // A face without normals or texture coordinates, as the DAE loader + // makes one, keeps none. + if (!old_normals) + { + mNormals = nullptr; + } + if (!old_tex_coords) + { + mTexCoords = nullptr; + } + ll_aligned_free<64>(old_positions); ll_aligned_free_16(old_weights); ll_aligned_free_16(old_tangents); diff --git a/indra/llmath/tests/llvolume_test.cpp b/indra/llmath/tests/llvolume_test.cpp index 1a1e448a0d..9f1d0d592f 100644 --- a/indra/llmath/tests/llvolume_test.cpp +++ b/indra/llmath/tests/llvolume_test.cpp @@ -4461,6 +4461,82 @@ namespace } } + // A face may come without normals or texture coordinates, as the DAE + // loader leaves one whose source has none. Reordered without tangents, + // which need both, it keeps every corner's position and comes out + // without what it came without. + TEST_CASE("llvolume: a face without normals or texture coordinates reorders without them", "[llvolume]") + { + struct Without + { + const char* what; + bool normals; + bool tex_coords; + }; + const Without cases[] = { + { "normals", true, false }, + { "texture coordinates", false, true }, + { "either", true, true }, + }; + const CorpusMesh mesh = uvSphere(16, 8); + for (const Without& without : cases) + { + CAPTURE(without.what); + // Each triangle as its corners' positions, and normals and + // texture coordinates where the face keeps them, from its least + // corner on; sorted, as the reordering leaves them in another + // order. + using Corner = std::array; + auto triangles = [&](const LLVolumeFace& f) + { + std::vector> out(f.mNumIndices / 3); + for (size_t t = 0; t < out.size(); ++t) + { + for (S32 k = 0; k < 3; ++k) + { + const U16 v = f.mIndices[t * 3 + k]; + Corner& corner = out[t][k]; + corner.fill(0.f); + std::memcpy(&corner[0], f.mPositions[v].getF32ptr(), 3 * sizeof(F32)); + if (!without.normals) + { + std::memcpy(&corner[3], f.mNormals[v].getF32ptr(), 3 * sizeof(F32)); + } + if (!without.tex_coords) + { + std::memcpy(&corner[6], f.mTexCoords[v].mV, 2 * sizeof(F32)); + } + } + std::rotate(out[t].begin(), std::min_element(out[t].begin(), out[t].end()), out[t].end()); + } + std::sort(out.begin(), out.end()); + return out; + }; + + LLVolumeFace face = mesh.face(); + const auto before = triangles(face); + if (without.normals) + { + face.mNormals = nullptr; + } + if (without.tex_coords) + { + face.mTexCoords = nullptr; + } + REQUIRE(face.cacheOptimize(true)); + CHECK((face.mNormals != nullptr) == !without.normals); + CHECK((face.mTexCoords != nullptr) == !without.tex_coords); + CHECK(!face.mTangents); + REQUIRE(face.mNumIndices == (S32)before.size() * 3); + for (S32 i = 0; i < face.mNumIndices; ++i) + { + CAPTURE(i); + REQUIRE(face.mIndices[i] < face.mNumVertices); + } + CHECK(triangles(face) == before); + } + } + // A circle path's stations step around it and end at its end cut. The // steps add up with rounding, and a station they bring a rounding short // of the end would leave a row with no height before the end's: no two From 0daac8b0159035dbeea4cd78f309fa5fb78437d8 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 06:42:19 -0400 Subject: [PATCH 04/10] ALScratchArena checks every return's order, and its freed arena is gone A release build checked nothing as a block came back: one given back while a block taken after it was still out moved the top down past that block, and the next taker was handed memory still in use. Only an llassert caught it, where SHOW_ASSERT is on. The check is a compare against the header the block already carries, so every build now makes it. A block given back out of order, or twice, leaves the top where it is, so that no block still in use is handed out again; its room is lost to the thread, and the first such return is logged. It warns rather than raises a fatal error: the arena's takers run on the mesh threads, where a fatal error may not be raised, and the arena is sound after it. The arena's destructor freed its memory and left mBase pointing at it, so a block taken on the thread after, as by a thread_local destroyed after the arena, was written into memory already given back. Clearing mBase in that destructor would not hold: GCC drops the stores an object makes to itself in its own destructor (-flifetime-dse), and the object is past its lifetime once it has run. The arena's state now has nothing to destroy and lasts as long as its thread. A thread_local of its own, made with the arena, frees the memory as the thread ends and marks the arena unavailable, so that a block taken after comes from the heap. Co-Authored-By: Claude Opus 5.5 --- indra/llcommon/alscratcharena.cpp | 43 ++++++++++++-- indra/llcommon/alscratcharena.h | 7 ++- indra/llcommon/tests/alscratcharena_test.cpp | 60 ++++++++++++++++++++ 3 files changed, 103 insertions(+), 7 deletions(-) diff --git a/indra/llcommon/alscratcharena.cpp b/indra/llcommon/alscratcharena.cpp index 01b5322376..6f77f6d71e 100644 --- a/indra/llcommon/alscratcharena.cpp +++ b/indra/llcommon/alscratcharena.cpp @@ -37,21 +37,40 @@ namespace // size, by which its return is checked to be the last one taken. constexpr size_t HEADER = ALScratchArena::ALIGNMENT; + // The thread's arena. Nothing is done to destroy it, so that it lasts + // as long as its thread, past every thread_local's destructor; + // ArenaRelease frees its memory. struct Arena { U8* mBase = nullptr; size_t mTop = 0; - // The arena could not be made: every block comes from the heap. + // The arena could not be made, or is freed: every block comes from + // the heap. bool mUnavailable = false; ALScratchArena::Counters mCounters; + }; + + thread_local Arena tArena; + + // Frees its thread's arena as the thread ends. It is made with the + // arena, so a thread_local made before is destroyed after it, and what + // that takes then comes from the heap. + struct ArenaRelease + { + Arena* mArena = nullptr; - ~Arena() + ~ArenaRelease() { - ll_aligned_free<64>(mBase); + if (mArena) + { + ll_aligned_free<64>(mArena->mBase); + mArena->mBase = nullptr; + mArena->mUnavailable = true; + } } }; - thread_local Arena tArena; + thread_local ArenaRelease tArenaRelease; } // namespace //static @@ -62,6 +81,7 @@ void* ALScratchArena::allocate(size_t bytes) { arena.mBase = static_cast(ll_aligned_malloc<64>(CAPACITY)); arena.mUnavailable = !arena.mBase; + tArenaRelease.mArena = &arena; } // Compared before it is rounded, so that a size near the top of size_t, @@ -93,8 +113,19 @@ void ALScratchArena::deallocate(void* block) U8* p = static_cast(block); if (arena.mBase && p >= arena.mBase && p < arena.mBase + CAPACITY) { - llassert(p + *reinterpret_cast(p - HEADER) == arena.mBase + arena.mTop); - arena.mTop = static_cast(p - HEADER - arena.mBase); + // Only the block on top moves the top down. Moving it down to any + // other would hand the blocks above, still in use, to the next + // taker, so the top stays, and the block's room is lost to the + // thread. + const size_t start = static_cast(p - arena.mBase); + if (start >= HEADER && start <= arena.mTop && *reinterpret_cast(p - HEADER) == arena.mTop - start) + { + arena.mTop = start - HEADER; + } + else + { + LL_WARNS_ONCE("ALScratchArena") << "A block was given back out of order or twice" << LL_ENDL; + } return; } ::operator delete(block); diff --git a/indra/llcommon/alscratcharena.h b/indra/llcommon/alscratcharena.h index 467db078c6..1ffaa69135 100644 --- a/indra/llcommon/alscratcharena.h +++ b/indra/llcommon/alscratcharena.h @@ -41,7 +41,12 @@ // // The order is the caller's to keep: a block goes back before every block // taken after it has. Nothing holding a block may yield to a coroutine -// that takes from the same thread's arena. +// that takes from the same thread's arena. A block given back out of that +// order, or twice, leaves the top where it is, so that no block still in +// use is handed out again: its room is lost to the thread, and the first +// such return is logged. The arena is freed as its thread ends, and a +// block taken after, as by a thread_local destroyed after it, comes from +// the heap. class LL_COMMON_API ALScratchArena { public: diff --git a/indra/llcommon/tests/alscratcharena_test.cpp b/indra/llcommon/tests/alscratcharena_test.cpp index 5f99b2a8fb..af8d98ac15 100644 --- a/indra/llcommon/tests/alscratcharena_test.cpp +++ b/indra/llcommon/tests/alscratcharena_test.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include namespace @@ -155,6 +156,65 @@ namespace ALScratchArena::deallocate(after); } + // A block given back while one taken after it is still out leaves the + // top where it is: the next block is taken above the one still out, not + // over it. The thread is the case's own, and the lost room goes with + // its arena. + TEST_CASE("alscratcharena: a block given back out of order leaves the blocks above it to their holders", "[alscratcharena]") + { + void* below = nullptr; + void* above = nullptr; + void* next = nullptr; + std::thread([&] + { + below = ALScratchArena::allocate(64); + above = ALScratchArena::allocate(64); + ALScratchArena::deallocate(below); + next = ALScratchArena::allocate(64); + ALScratchArena::deallocate(next); + ALScratchArena::deallocate(above); + }) + .join(); + CHECK(address(above) > address(below)); + CHECK(address(next) >= address(above) + 64); + } + + // A thread_local made before its thread first takes a block is + // destroyed after the thread's arena is freed, and what it takes then + // comes from the heap. A block taken from the freed arena would be + // written into memory already given back, and the destructor would not + // get as far as saying what it was served. + struct LateTaker + { + // What the arena served the destructor, once it has finished. + std::optional* served = nullptr; + + ~LateTaker() + { + if (served) + { + ALScratchArena::resetCounters(); + ALScratchArena::deallocate(ALScratchArena::allocate(64)); + *served = ALScratchArena::counters(); + } + } + }; + thread_local LateTaker tLateTaker; + + TEST_CASE("alscratcharena: a block taken after the thread's arena is freed comes from the heap", "[alscratcharena]") + { + std::optional late; + std::thread([&] + { + tLateTaker.served = ⪭ + ALScratchArena::deallocate(ALScratchArena::allocate(64)); + }) + .join(); + REQUIRE(late.has_value()); + CHECK(late->arena_blocks == 0); + CHECK(late->heap_blocks == 1); + } + TEST_CASE("alscratcharena: each thread takes from an arena of its own", "[alscratcharena]") { void* mine = ALScratchArena::allocate(64); From 68a39b2b32893ee4ede5f4882803f9fad4abed69 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 09:47:20 -0400 Subject: [PATCH 05/10] A thread's arena outlives the blocks still out of it as the thread ends ArenaRelease freed its thread's arena and cleared its base as the thread ended, whatever was still out of it. A thread_local made before the arena, as a thread_local Scope would be, is destroyed after the release; a block it held was then memory already freed, and giving it back failed the arena's range check and passed an arena pointer to operator delete. The release now only stops the arena taking blocks, so that what is taken after comes from the heap, and the arena is freed as its top comes back to the start: at once where nothing is out, or when the last block out is given back. An arena whose room was lost to a block given back out of order never comes back to the start, and is never freed: four megabytes for the life of the process, once for each thread that gave a block back out of order. Nothing is logged as the thread ends; the misordered return was logged when it happened, and a block still out then is no fault of its holder's. The case for a block given back out of order now gives that block back again once it is on top, so that its thread's arena is freed and the sanitizer job's leak check finds nothing. Co-Authored-By: Claude Opus 5.5 --- indra/llcommon/alscratcharena.cpp | 29 ++++++++---- indra/llcommon/alscratcharena.h | 15 ++++-- indra/llcommon/tests/alscratcharena_test.cpp | 48 +++++++++++++++++++- 3 files changed, 76 insertions(+), 16 deletions(-) diff --git a/indra/llcommon/alscratcharena.cpp b/indra/llcommon/alscratcharena.cpp index 6f77f6d71e..6dfe5ea9a1 100644 --- a/indra/llcommon/alscratcharena.cpp +++ b/indra/llcommon/alscratcharena.cpp @@ -39,22 +39,33 @@ namespace // The thread's arena. Nothing is done to destroy it, so that it lasts // as long as its thread, past every thread_local's destructor; - // ArenaRelease frees its memory. + // ArenaRelease ends it. struct Arena { U8* mBase = nullptr; size_t mTop = 0; - // The arena could not be made, or is freed: every block comes from - // the heap. + // The arena could not be made, or its thread has ended: every block + // comes from the heap. bool mUnavailable = false; ALScratchArena::Counters mCounters; }; thread_local Arena tArena; - // Frees its thread's arena as the thread ends. It is made with the - // arena, so a thread_local made before is destroyed after it, and what - // that takes then comes from the heap. + // Frees an arena its thread has ended once no block is out of it. + void freeIfEnded(Arena& arena) + { + if (arena.mUnavailable && arena.mTop == 0) + { + ll_aligned_free<64>(arena.mBase); + arena.mBase = nullptr; + } + } + + // Ends its thread's arena as the thread ends. It is made with the arena, + // so a thread_local made before is destroyed after it: what that takes + // then comes from the heap, and a block it holds goes back into the + // arena, which is freed once the last is back. struct ArenaRelease { Arena* mArena = nullptr; @@ -63,9 +74,8 @@ namespace { if (mArena) { - ll_aligned_free<64>(mArena->mBase); - mArena->mBase = nullptr; mArena->mUnavailable = true; + freeIfEnded(*mArena); } } }; @@ -87,7 +97,7 @@ void* ALScratchArena::allocate(size_t bytes) // Compared before it is rounded, so that a size near the top of size_t, // as meshoptimizer asks for when a count overflows, goes to the heap. const size_t room = CAPACITY - arena.mTop; - if (arena.mBase && room > HEADER && bytes <= room - HEADER) + if (!arena.mUnavailable && room > HEADER && bytes <= room - HEADER) { const size_t size = (bytes + ALIGNMENT - 1) & ~(ALIGNMENT - 1); if (size <= room - HEADER) @@ -121,6 +131,7 @@ void ALScratchArena::deallocate(void* block) if (start >= HEADER && start <= arena.mTop && *reinterpret_cast(p - HEADER) == arena.mTop - start) { arena.mTop = start - HEADER; + freeIfEnded(arena); } else { diff --git a/indra/llcommon/alscratcharena.h b/indra/llcommon/alscratcharena.h index 1ffaa69135..3f33cde7db 100644 --- a/indra/llcommon/alscratcharena.h +++ b/indra/llcommon/alscratcharena.h @@ -42,11 +42,16 @@ // The order is the caller's to keep: a block goes back before every block // taken after it has. Nothing holding a block may yield to a coroutine // that takes from the same thread's arena. A block given back out of that -// order, or twice, leaves the top where it is, so that no block still in -// use is handed out again: its room is lost to the thread, and the first -// such return is logged. The arena is freed as its thread ends, and a -// block taken after, as by a thread_local destroyed after it, comes from -// the heap. +// order leaves the top where it is, so that no block still in use is +// handed out again: its room is lost to the thread, and the first such +// return is logged. A block given back twice is passed over the same way, +// unless it is the block on top. +// +// As its thread ends the arena takes no more blocks: one taken after, as +// by a thread_local destroyed after it, comes from the heap. The arena is +// freed once every block taken from it is back, so that a block a +// thread_local holds stays its holder's until given back. An arena that +// has lost room to a block given back out of order is never freed. class LL_COMMON_API ALScratchArena { public: diff --git a/indra/llcommon/tests/alscratcharena_test.cpp b/indra/llcommon/tests/alscratcharena_test.cpp index af8d98ac15..dd37a7c11e 100644 --- a/indra/llcommon/tests/alscratcharena_test.cpp +++ b/indra/llcommon/tests/alscratcharena_test.cpp @@ -158,13 +158,15 @@ namespace // A block given back while one taken after it is still out leaves the // top where it is: the next block is taken above the one still out, not - // over it. The thread is the case's own, and the lost room goes with - // its arena. + // over it. The thread is the case's own. Given back again once it is on + // top, the first block brings the top back to the start, so that the + // arena is freed as the thread ends. TEST_CASE("alscratcharena: a block given back out of order leaves the blocks above it to their holders", "[alscratcharena]") { void* below = nullptr; void* above = nullptr; void* next = nullptr; + void* again = nullptr; std::thread([&] { below = ALScratchArena::allocate(64); @@ -173,10 +175,52 @@ namespace next = ALScratchArena::allocate(64); ALScratchArena::deallocate(next); ALScratchArena::deallocate(above); + ALScratchArena::deallocate(below); + again = ALScratchArena::allocate(64); + ALScratchArena::deallocate(again); }) .join(); CHECK(address(above) > address(below)); CHECK(address(next) >= address(above) + 64); + CHECK(again == below); + } + + // A thread_local made before its thread first takes a block, holding + // one as the thread ends, is destroyed after the arena is ended. The + // block is still its own, to write and to give back into the arena; + // what it takes after comes from the heap. + struct LateHolder + { + void* block = nullptr; + // What the arena served the destructor, once it has finished. + std::optional* served = nullptr; + + ~LateHolder() + { + if (served) + { + std::memset(block, 0xab, 64); + ALScratchArena::deallocate(block); + ALScratchArena::resetCounters(); + ALScratchArena::deallocate(ALScratchArena::allocate(64)); + *served = ALScratchArena::counters(); + } + } + }; + thread_local LateHolder tLateHolder; + + TEST_CASE("alscratcharena: a block still out as its thread ends goes back into the arena", "[alscratcharena]") + { + std::optional late; + std::thread([&] + { + tLateHolder.served = ⪭ + tLateHolder.block = ALScratchArena::allocate(64); + }) + .join(); + REQUIRE(late.has_value()); + CHECK(late->arena_blocks == 0); + CHECK(late->heap_blocks == 1); } // A thread_local made before its thread first takes a block is From 92f6d3bf582daaf633afcad0499049811ed0b039 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 09:48:06 -0400 Subject: [PATCH 06/10] A mesh LOD of more indices than any real one fails its decode The bound on a face's indices bounds what cacheOptimize asks for one face, but not what a LOD asks for: faces each under it could still carry between them every index the 256 MB unzip cap lets through, about 128M, and each is decoded and then optimized. decodeVolumeFaces now sums the TriangleLists of a LOD's faces before it makes any, and fails the LOD past LL_SCULPT_MESH_MAX_LOD_INDICES. The bound is two faces at the most a face may hold, 4,194,304. The largest of 66,505 LODs in a viewer's mesh cache, of 18,793 meshes, held 526,422 between its faces, an eighth of it, and none held more than the eight faces an upload is split into. Twice the face bound keeps that bound the tighter one for a single face, and the LOD's decode work within about twice one face's. Co-Authored-By: Claude Opus 5.5 --- indra/llmath/llvolume.cpp | 17 +++++++++++ indra/llmath/llvolume.h | 5 ++++ indra/llmath/tests/llvolume_test.cpp | 43 +++++++++++++++++++++++++--- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/indra/llmath/llvolume.cpp b/indra/llmath/llvolume.cpp index b2016ea529..c3c3f12d2c 100644 --- a/indra/llmath/llvolume.cpp +++ b/indra/llmath/llvolume.cpp @@ -2582,6 +2582,23 @@ bool LLVolume::decodeVolumeFaces(const LLSD& mdl) return false; } + // The LOD's indices between its faces, summed before any face is + // made. + size_t lod_indices = 0; + for (size_t i = 0; i < face_count; ++i) + { + if (!mdl[i].has("NoGeometry")) + { + lod_indices += mdl[i]["TriangleList"].asBinary().size() / sizeof(U16); + } + } + if (lod_indices > (size_t)LL_SCULPT_MESH_MAX_LOD_INDICES) + { + LL_WARNS() << "Invalid index count " << lod_indices << " exceeds maximum for a LOD of " << face_count << " faces" << LL_ENDL; + mVolumeFaces.clear(); + return false; + } + mVolumeFaces.resize(face_count); for (size_t i = 0; i < face_count; ++i) diff --git a/indra/llmath/llvolume.h b/indra/llmath/llvolume.h index 39878cb0f8..be70a9b0dd 100644 --- a/indra/llmath/llvolume.h +++ b/indra/llmath/llvolume.h @@ -220,6 +220,11 @@ constexpr S32 LL_SCULPT_MESH_MAX_FACES = 8; // 360,564. A LOD with a face past this does not decode. constexpr S32 LL_SCULPT_MESH_MAX_FACE_INDICES = 32 * 65536; +// The most indices a mesh LOD's faces may hold between them: two faces of +// as many as a face may hold. The largest of 66,000 LODs in a viewer's mesh +// cache held 526,422, an eighth of this. A LOD past this does not decode. +constexpr S32 LL_SCULPT_MESH_MAX_LOD_INDICES = 2 * LL_SCULPT_MESH_MAX_FACE_INDICES; + extern bool gDebugGL; class LLProfileParams diff --git a/indra/llmath/tests/llvolume_test.cpp b/indra/llmath/tests/llvolume_test.cpp index 9f1d0d592f..d63e26ad11 100644 --- a/indra/llmath/tests/llvolume_test.cpp +++ b/indra/llmath/tests/llvolume_test.cpp @@ -4374,11 +4374,9 @@ namespace return face; } - // A LOD of the one face, decoded into a mesh volume of its own. - LLPointer decodeLOD(const LLSD& face, bool& decoded) + // A LOD of the faces, decoded into a mesh volume of its own. + LLPointer decodeFaces(const LLSD& mdl, bool& decoded) { - LLSD mdl = LLSD::emptyArray(); - mdl.append(face); LLVolumeParams params; params.setType(LL_PCODE_PROFILE_SQUARE, LL_PCODE_PATH_LINE); params.setSculptID(LLUUID("00000000-0000-0000-0000-00000000000c"), LL_SCULPT_TYPE_MESH); @@ -4387,6 +4385,14 @@ namespace return volume; } + // A LOD of the one face, decoded into a mesh volume of its own. + LLPointer decodeLOD(const LLSD& face, bool& decoded) + { + LLSD mdl = LLSD::emptyArray(); + mdl.append(face); + return decodeFaces(mdl, decoded); + } + // A face's TriangleList of zeros deflates to almost nothing, so that one // the unzip cap lets through could hold a hundred million indices, and // cost every viewer that decodes it seconds and gigabytes. A LOD with a @@ -4415,6 +4421,35 @@ namespace } } + // Faces each within LL_SCULPT_MESH_MAX_FACE_INDICES could still carry, + // between them, all the indices the unzip cap lets through. A LOD whose + // faces hold more than LL_SCULPT_MESH_MAX_LOD_INDICES between them does + // not decode; one whose hold as many as that does. + TEST_CASE("llvolume: a mesh LOD of more indices than a LOD may hold does not decode", "[llvolume]") + { + const S32 most = LL_SCULPT_MESH_MAX_FACE_INDICES - LL_SCULPT_MESH_MAX_FACE_INDICES % 3; + REQUIRE(2 * most <= LL_SCULPT_MESH_MAX_LOD_INDICES); + REQUIRE(2 * most + 6 > LL_SCULPT_MESH_MAX_LOD_INDICES); + for (const S32 last : { 0, 6 }) + { + CAPTURE(last); + LLSD mdl = LLSD::emptyArray(); + for (const S32 indices : { most, most, last }) + { + if (indices > 0) + { + LLSD face = wireTriangle(); + face["TriangleList"] = LLSD(LLSD::Binary((size_t)indices * sizeof(U16), 0)); + mdl.append(face); + } + } + bool decoded = false; + LLPointer volume = decodeFaces(mdl, decoded); + CHECK(decoded == (last == 0)); + CHECK(volume->getNumVolumeFaces() == (decoded ? (S32)mdl.size() : 0)); + } + } + // A rigged face's weights are an influence after another, each a joint // and two bytes of weight. An array that ends inside an influence leaves // its vertex the influences before it, and is not read past its end. From fd02953f7f471c06475334519129105e8c3bcb42 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 19:30:21 -0400 Subject: [PATCH 07/10] remap welds a face by the attributes it has LLVolumeFace::remap, which the GLTF loader runs on every face it loads and the DAE loader unless told not to optimize, handed meshoptimizer the face's normals and texture coordinates as streams to weld by and to remap, null or not, and read through a null one. A face whose source had none of either reached it only by way of a copy, which gave it room for them that nothing wrote, so no upload has crashed there; it welded by whatever that room held. remap now leaves a null stream out of the weld, remaps only what the face has, and comes out without what it came without. A copy is to keep a face without them next. Co-Authored-By: Claude Opus 5.5 --- indra/llmath/llvolume.cpp | 42 ++++++++++++++----- indra/llmath/tests/llvolume_test.cpp | 61 ++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 11 deletions(-) diff --git a/indra/llmath/llvolume.cpp b/indra/llmath/llvolume.cpp index c3c3f12d2c..4a3ed652f0 100644 --- a/indra/llmath/llvolume.cpp +++ b/indra/llmath/llvolume.cpp @@ -5030,16 +5030,21 @@ void LLVolumeFace::remap() } // Each vertex's place among the vertices that differ in position, normal - // or texture coordinate, or ~0u where no index uses it. - const meshopt_Stream streams[] = { - { mPositions, sizeof(F32) * 3, sizeof(LLVector4a) }, - { mNormals, sizeof(F32) * 3, sizeof(LLVector4a) }, - { mTexCoords, sizeof(LLVector2), sizeof(LLVector2) }, - }; + // or texture coordinate, or ~0u where no index uses it. Normals and + // texture coordinates the face has none of take no part. + meshopt_Stream streams[3] = { { mPositions, sizeof(F32) * 3, sizeof(LLVector4a) } }; + size_t stream_count = 1; + if (mNormals) + { + streams[stream_count++] = { mNormals, sizeof(F32) * 3, sizeof(LLVector4a) }; + } + if (mTexCoords) + { + streams[stream_count++] = { mTexCoords, sizeof(LLVector2), sizeof(LLVector2) }; + } U32* remap = scratch.allocate(mNumVertices); S32 remap_vertices_count = static_cast(meshopt_generateVertexRemapMulti(remap, checked, checked ? mNumIndices : mNumVertices, - mNumVertices, streams, - sizeof(streams) / sizeof(streams[0]))); + mNumVertices, streams, stream_count)); meshopt_remapIndexBuffer(indices, checked, mNumIndices, remap); // Allocate new buffers @@ -5051,14 +5056,29 @@ void LLVolumeFace::remap() LLVector4a* remap_normals = remap_positions + remap_vertices_count; LLVector2* remap_tex_coords = (LLVector2*)(remap_normals + remap_vertices_count); - // Fill the buffers + // Fill the buffers, the normals and texture coordinates only where the + // face has them for (S32 i = 0; i < mNumIndices; ++i) { remap_indices[i] = static_cast(indices[i]); } meshopt_remapVertexBuffer(remap_positions, mPositions, mNumVertices, sizeof(LLVector4a), remap); - meshopt_remapVertexBuffer(remap_normals, mNormals, mNumVertices, sizeof(LLVector4a), remap); - meshopt_remapVertexBuffer(remap_tex_coords, mTexCoords, mNumVertices, sizeof(LLVector2), remap); + if (mNormals) + { + meshopt_remapVertexBuffer(remap_normals, mNormals, mNumVertices, sizeof(LLVector4a), remap); + } + else + { + remap_normals = nullptr; + } + if (mTexCoords) + { + meshopt_remapVertexBuffer(remap_tex_coords, mTexCoords, mNumVertices, sizeof(LLVector2), remap); + } + else + { + remap_tex_coords = nullptr; + } // Free unused buffers ll_aligned_free_16(mIndices); diff --git a/indra/llmath/tests/llvolume_test.cpp b/indra/llmath/tests/llvolume_test.cpp index d63e26ad11..95ae51a5ca 100644 --- a/indra/llmath/tests/llvolume_test.cpp +++ b/indra/llmath/tests/llvolume_test.cpp @@ -4572,6 +4572,67 @@ namespace } } + // A face may come without normals or texture coordinates, as the DAE + // loader leaves one whose source has none. remap welds its vertices by + // what it has, and it comes out without what it came without. Two + // triangles share an edge whose two vertices are each held twice, the + // copies apart in both normal and texture coordinate: they weld only + // where the face has neither. + TEST_CASE("llvolume: a face without normals or texture coordinates remaps without them", "[llvolume]") + { + struct Without + { + const char* what; + bool normals; + bool tex_coords; + S32 vertices; // after the weld + }; + const Without cases[] = { + { "normals", true, false, 6 }, + { "texture coordinates", false, true, 6 }, + { "either", true, true, 4 }, + }; + CorpusMesh mesh; + mesh.positions = { { 0, 0, 0 }, { 1, 0, 0 }, { 0, 1, 0 }, { 1, 0, 0 }, { 0, 1, 0 }, { 1, 1, 0 } }; + mesh.normals = { { 0, 0, 1 }, { 0, 0, 1 }, { 0, 0, 1 }, { 0, 0.6f, 0.8f }, { 0, 0.6f, 0.8f }, { 0, 0, 1 } }; + mesh.uvs = { { 0, 0 }, { 0.5f, 0 }, { 0, 0.5f }, { 0.5f, 0.5f }, { 0.5f, 1 }, { 1, 1 } }; + mesh.indices = { 0, 1, 2, 4, 3, 5 }; + for (const Without& without : cases) + { + CAPTURE(without.what); + LLVolumeFace face = mesh.face(); + if (without.normals) + { + face.mNormals = nullptr; + } + if (without.tex_coords) + { + face.mTexCoords = nullptr; + } + face.remap(); + CHECK((face.mNormals != nullptr) == !without.normals); + CHECK((face.mTexCoords != nullptr) == !without.tex_coords); + CHECK(face.mNumVertices == without.vertices); + REQUIRE(face.mNumIndices == (S32)mesh.indices.size()); + for (S32 i = 0; i < face.mNumIndices; ++i) + { + CAPTURE(i); + const U16 v = face.mIndices[i]; + const U16 was = mesh.indices[i]; + REQUIRE(v < face.mNumVertices); + CHECK(v3(face.mPositions[v]) == mesh.positions[was]); + if (!without.normals) + { + CHECK(v3(face.mNormals[v]) == mesh.normals[was]); + } + if (!without.tex_coords) + { + CHECK(face.mTexCoords[v] == mesh.uvs[was]); + } + } + } + } + // A circle path's stations step around it and end at its end cut. The // steps add up with rounding, and a station they bring a rounding short // of the end would leave a row with no height before the end's: no two From 4d4d9f875f805df1478c4999eff6e5027f96c593 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 09:49:42 -0400 Subject: [PATCH 08/10] A copy of a face without normals or texture coordinates is without them LLVolumeFace::operator=, which the copy constructor runs too, made room for the normals and texture coordinates with the positions, as resizeVertices always does, and skipped copying either where the source had none. The copy then had pointers to them that nothing had written, which cacheOptimize, now copying only what a face has, and tangent generation take for real ones. The upload's loaders copy every face as they sort it by material, so a DAE without normals or UVs went on with whatever those buffers held. A copy now comes out without what its source is without. swapData exchanges the pointers with the positions and keeps the gap closed. Of resizeVertices' other callers, those that rebuild a face that may lack either already clear the pointer after: the DAE loader, LLModel::setVolumeFaceData, the normal generator, cacheOptimize and ALMeshSimplifier. Every reader on the upload's path checks for null already, and remap does since the change before this. Co-Authored-By: Claude Opus 5.5 --- indra/llmath/llvolume.cpp | 11 ++++++ indra/llmath/tests/llvolume_test.cpp | 56 ++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/indra/llmath/llvolume.cpp b/indra/llmath/llvolume.cpp index 4a3ed652f0..6a79f1677c 100644 --- a/indra/llmath/llvolume.cpp +++ b/indra/llmath/llvolume.cpp @@ -4842,6 +4842,17 @@ LLVolumeFace& LLVolumeFace::operator=(const LLVolumeFace& src) LLVector4a::memcpyNonAliased16((F32*) mTexCoords, (F32*) src.mTexCoords, tc_size); } + // A source without normals or texture coordinates makes a face + // without them, not one whose are unwritten. + if (!src.mNormals) + { + mNormals = NULL; + } + if (!src.mTexCoords) + { + mTexCoords = NULL; + } + if (src.mTangents) { allocateTangents(src.mNumVertices); diff --git a/indra/llmath/tests/llvolume_test.cpp b/indra/llmath/tests/llvolume_test.cpp index 95ae51a5ca..7404b7f0d3 100644 --- a/indra/llmath/tests/llvolume_test.cpp +++ b/indra/llmath/tests/llvolume_test.cpp @@ -4633,6 +4633,62 @@ namespace } } + // A face copied from one without normals or texture coordinates, by + // the copy constructor or by assignment, comes out without them too, + // rather than with room for them that nothing wrote. + TEST_CASE("llvolume: a copy of a face without normals or texture coordinates is without them", "[llvolume]") + { + struct Without + { + const char* what; + bool normals; + bool tex_coords; + }; + const Without cases[] = { + { "normals", true, false }, + { "texture coordinates", false, true }, + { "either", true, true }, + }; + const CorpusMesh mesh = uvSphere(16, 8); + for (const Without& without : cases) + { + CAPTURE(without.what); + LLVolumeFace face = mesh.face(); + if (without.normals) + { + face.mNormals = nullptr; + } + if (without.tex_coords) + { + face.mTexCoords = nullptr; + } + const LLVolumeFace constructed(face); + LLVolumeFace assigned; + assigned = face; + const LLVolumeFace* copies[] = { &constructed, &assigned }; + for (const LLVolumeFace* copy : copies) + { + INFO((copy == &constructed ? "by the copy constructor" : "by assignment")); + CHECK((copy->mNormals != nullptr) == !without.normals); + CHECK((copy->mTexCoords != nullptr) == !without.tex_coords); + REQUIRE(copy->mNumVertices == face.mNumVertices); + for (S32 v = 0; v < face.mNumVertices; ++v) + { + CAPTURE(v); + CHECK(v3(copy->mPositions[v]) == v3(face.mPositions[v])); + if (!without.normals) + { + CHECK(v3(copy->mNormals[v]) == v3(face.mNormals[v])); + } + if (!without.tex_coords) + { + CHECK(copy->mTexCoords[v] == face.mTexCoords[v]); + } + } + } + } + } + // A circle path's stations step around it and end at its end cut. The // steps add up with rounding, and a station they bring a rounding short // of the end would leave a row with no height before the end's: no two From 2f23dbcf3efbf7ed0aa5ca529484a296806926f0 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 19:56:01 -0400 Subject: [PATCH 09/10] A mesh LOD of more faces than an upload makes fails its decode decodeVolumeFaces made a face for every entry of a LOD. A face with no geometry costs nothing against the index budget, but is made all the same, with buffers of its own, so a LOD of a million empty entries, a few megabytes unzipped, cost a million faces. Past 255 the face count also wraps the texture entry count the renderer takes from it. A LOD of more faces than LL_SCULPT_MESH_MAX_FACES now fails its decode before any face is made. The bound is LL_SCULPT_MESH_MAX_FACES itself, eight: an upload splits a model into meshes of at most eight faces, and none of the 66,505 LODs nor of the 564 physics shapes in a viewer's mesh cache held more. A wider bound would admit only faces no upload makes, and every face it admitted would cost as much as a real one. Co-Authored-By: Claude Opus 5.5 --- indra/llmath/llvolume.cpp | 9 +++++++++ indra/llmath/llvolume.h | 3 +++ indra/llmath/tests/llvolume_test.cpp | 23 +++++++++++++++++++++++ 3 files changed, 35 insertions(+) diff --git a/indra/llmath/llvolume.cpp b/indra/llmath/llvolume.cpp index 6a79f1677c..2fd7c5e6ea 100644 --- a/indra/llmath/llvolume.cpp +++ b/indra/llmath/llvolume.cpp @@ -2582,6 +2582,15 @@ bool LLVolume::decodeVolumeFaces(const LLSD& mdl) return false; } + // A face of no indices costs nothing against the index budget, but + // is made all the same. + if (face_count > (size_t)LL_SCULPT_MESH_MAX_FACES) + { + LL_WARNS() << "Invalid face count " << face_count << " exceeds maximum" << LL_ENDL; + mVolumeFaces.clear(); + return false; + } + // The LOD's indices between its faces, summed before any face is // made. size_t lod_indices = 0; diff --git a/indra/llmath/llvolume.h b/indra/llmath/llvolume.h index be70a9b0dd..81dfeeaa5d 100644 --- a/indra/llmath/llvolume.h +++ b/indra/llmath/llvolume.h @@ -212,6 +212,9 @@ constexpr U8 LL_SCULPT_FLAG_INVERT = 64; constexpr U8 LL_SCULPT_FLAG_MIRROR = 128; constexpr U8 LL_SCULPT_FLAG_MASK = LL_SCULPT_FLAG_INVERT | LL_SCULPT_FLAG_MIRROR; +// The most faces a mesh LOD may hold: an upload splits a model into +// meshes of as many, and none of 66,000 LODs in a viewer's mesh cache held +// more. A LOD of more does not decode. constexpr S32 LL_SCULPT_MESH_MAX_FACES = 8; // The most indices a mesh face may hold: 32 for each of the 65,536 vertices diff --git a/indra/llmath/tests/llvolume_test.cpp b/indra/llmath/tests/llvolume_test.cpp index 7404b7f0d3..45076d795b 100644 --- a/indra/llmath/tests/llvolume_test.cpp +++ b/indra/llmath/tests/llvolume_test.cpp @@ -4450,6 +4450,29 @@ namespace } } + // A face with no geometry costs nothing against the index budget, but + // is made all the same, so that a LOD of many would cost what the index + // budget does not count. A LOD of more than LL_SCULPT_MESH_MAX_FACES + // faces does not decode; one of as many as that does. + TEST_CASE("llvolume: a mesh LOD of more faces than a LOD may hold does not decode", "[llvolume]") + { + for (const S32 faces : { LL_SCULPT_MESH_MAX_FACES, LL_SCULPT_MESH_MAX_FACES + 1 }) + { + CAPTURE(faces); + LLSD mdl = LLSD::emptyArray(); + for (S32 f = 0; f < faces; ++f) + { + LLSD face; + face["NoGeometry"] = true; + mdl.append(face); + } + bool decoded = false; + LLPointer volume = decodeFaces(mdl, decoded); + CHECK(decoded == (faces <= LL_SCULPT_MESH_MAX_FACES)); + CHECK(volume->getNumVolumeFaces() == (decoded ? faces : 0)); + } + } + // A rigged face's weights are an influence after another, each a joint // and two bytes of weight. An array that ends inside an influence leaves // its vertex the influences before it, and is not read past its end. From fd312fd4f677b75939be37126ab2455337361ee8 Mon Sep 17 00:00:00 2001 From: Rye Date: Sun, 25 Oct 2026 21:57:53 -0400 Subject: [PATCH 10/10] A face with no area is drawn and touched along the normal it reports A face with no area keeps no planar normal and makes it again at each call, and getPlanarNormal returns the one it makes: one plane along the face's one normal, where it has one. The planar coordinates and tangents made it and then read the member back, found nothing kept, and projected the face per vertex while getPlanarNormal said one plane, so a touch, projected along its normal renormalized, could land off the vertex it touched. The sanitizer build found it on caps tapered to a point, whose area comes to nothing there. They now take the normal getPlanarNormal gives. Co-Authored-By: Claude Opus 5.5 --- indra/llmath/llvolume.cpp | 30 ++++----------------------- indra/llmath/tests/llvolume_test.cpp | 31 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/indra/llmath/llvolume.cpp b/indra/llmath/llvolume.cpp index 2fd7c5e6ea..1a0af41176 100644 --- a/indra/llmath/llvolume.cpp +++ b/indra/llmath/llvolume.cpp @@ -5801,14 +5801,7 @@ void LLVolumeFace::getPlanarBasis(const LLVector4a& normal, F32 switch_x, LLVect void LLVolumeFace::getPlanarTexCoords(const LLVector4a& scale, const alsimd::TexTransform* transform, F32* dst) const { - // read back from the member, whole: a vector rebuilt from its parts - // waits on their stores - if (mPlanarNormal[3] == 0.f) - { - makePlanarNormal(); - } - LLVector4a planar; - planar.loadua(mPlanarNormal); + const LLVector4a planar = getPlanarNormal(); if (planar[3] > 0.f) { LLVector4a binormal, tangent; @@ -5823,12 +5816,7 @@ void LLVolumeFace::getPlanarTexCoords(const LLVector4a& scale, const alsimd::Tex LLVector2 LLVolumeFace::getPlanarTexCoord(const LLVector4a& position, const LLVector4a& normal, const alsimd::TexTransform* transform) const { - if (mPlanarNormal[3] == 0.f) - { - makePlanarNormal(); - } - LLVector4a planar; - planar.loadua(mPlanarNormal); + const LLVector4a planar = getPlanarNormal(); const LLVector4a unscaled(1.f); LLVector2 tc; if (planar[3] > 0.f) @@ -5846,12 +5834,7 @@ LLVector2 LLVolumeFace::getPlanarTexCoord(const LLVector4a& position, const LLVe void LLVolumeFace::getPlanarTangents(const LLVector4a& scale, LLVector4a* dst) const { - if (mPlanarNormal[3] == 0.f) - { - makePlanarNormal(); - } - LLVector4a planar; - planar.loadua(mPlanarNormal); + const LLVector4a planar = getPlanarNormal(); if (planar[3] > 0.f) { LLVector4a binormal, tangent; @@ -5866,12 +5849,7 @@ void LLVolumeFace::getPlanarTangents(const LLVector4a& scale, LLVector4a* dst) c LLVector4a LLVolumeFace::getPlanarTangent(const LLVector4a& normal, const LLVector4a& scale) const { - if (mPlanarNormal[3] == 0.f) - { - makePlanarNormal(); - } - LLVector4a planar; - planar.loadua(mPlanarNormal); + const LLVector4a planar = getPlanarNormal(); LLVector4a out; if (planar[3] > 0.f) { diff --git a/indra/llmath/tests/llvolume_test.cpp b/indra/llmath/tests/llvolume_test.cpp index 45076d795b..4b698ffb8c 100644 --- a/indra/llmath/tests/llvolume_test.cpp +++ b/indra/llmath/tests/llvolume_test.cpp @@ -6532,6 +6532,37 @@ namespace CHECK(report.failures == 0); } + // A face with no area keeps no planar normal, and one with one normal is + // one plane along it all the same, made again at each call: it is drawn + // and touched along that normal as a face that keeps it is, whatever the + // touch's normal rounds to. Its normal here is off unit length, as a + // mesh's quantized normals are, so a touch projected along its own + // renormalized normal lands elsewhere. + TEST_CASE("llvolume: a face with no area is drawn and touched along its one normal", "[llvolume]") + { + LLVolumeFace face; + face.resizeVertices(3); + face.resizeIndices(3); + REQUIRE(face.mNumVertices == 3); + REQUIRE(face.mNumIndices == 3); + for (S32 v = 0; v < 3; ++v) + { + face.mPositions[v].set(0.25f, -0.375f, 0.125f); + face.mNormals[v].set(0.f, 0.6f, 0.9f); + face.mTexCoords[v].set(0.f, 0.f); + face.mIndices[v] = static_cast(v); + } + const LLVector4a planar = face.getPlanarNormal(); + CHECK(planar[3] > 0.f); + CHECK(std::memcmp(planar.getF32ptr(), face.mNormals[0].getF32ptr(), 3 * sizeof(F32)) == 0); + + Report report; + S64 touched = 0; + checkTouches(face, "a face of no area", report, touched); + INFO(report.text()); + CHECK(report.failures == 0); + } + // A normal map planar texgen lays out shades by the frame of its own // coordinates. Where a face is projected as one plane those are affine // in position, and its planar tangents are the ones its own tangent