Add xatlas native library and improve tool robustness - #191
Open
SashaRX wants to merge 76 commits into
Open
Conversation
Use jq + bash regex for package.json parsing and semver validation, stop persisting the checkout credential, and pass step outputs via env instead of interpolating them into the shell. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Drop the contents:write publish job from build-native.yml, default the workflow to read-only permissions, and pin the actions to verified SHAs. Native artifacts are now uploaded only; a maintainer commits them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Forwarding %* let cmd.exe re-parse metacharacters from the original command line. Whitelist the documented flag/positional forms and quote each argument explicitly instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Phase 0 wrote raw package.json values into /tmp/skills-overhaul.env, which Phase 3 later sources — a command-substitution injection chain. Emit the file with `declare -p` so values cannot be reinterpreted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
npm ignores .gitignore entirely once .npmignore exists, so local Unity, IDE and build-intermediate artifacts could be published. Add the missing patterns; no tracked file is affected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
FetchContent pulled jpcy/xatlas at GIT_TAG master, so every native build compiled whatever HEAD happened to be. Vendor a reviewed snapshot under Native~/third_party/xatlas and point CMake at it; meshoptimizer keeps its pinned tag. The vendored xatlas.cpp/xatlas.h are byte-identical to upstream jpcy/xatlas f700c77. The PR's copy carried ~34 extra lines (an unreferenced s_preserveChartScale flag plus "SashaRX.UnityMeshLab fork" comments) that are called from nowhere in this repo; those were stripped so the snapshot stays a verbatim upstream copy. The prebuilt binaries under Plugins/ are intentionally left at their current versions — they must be rebuilt from these vendored sources via the build-native workflow and committed by a maintainer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
resolution * internalOversample was computed in uint and the pack cost in long, so both could wrap and silently bypass the pack-cost safety budget before reaching native xatlas. Resolve dimensions through ulong and reject out-of-range values; saturate ComputePackCost at long.MaxValue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
BenchmarkSweep.WriteSummaryCsv wrote user-controlled paths unescaped, so a cell could start with =, +, - or @ and be evaluated as a formula on open. Prefix such values with an apostrophe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
FbxMetricsExporter.WriteCsv wrote modelName/lodGroupName/rendererName unescaped, so a cell could start with =, +, - or @ and be evaluated as a formula on open. Prefix such values with an apostrophe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
BenchmarkRecorder.Csv wrote user-controlled asset names unescaped. Prefix values leading with =, +, -, @, tab, CR or LF with an apostrophe so the exported cell stays plain text. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
render_model interpolated the model name into the nav link href without escaping, unlike render_index which already escaped the same pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…, #159, #161) Follow-up to the three CSV formula-injection fixes, which each grew a near-identical private Csv() helper with slightly different prefix sets. Move the logic into internal static CsvUtil.Escape (superset behaviour: =, +, -, @, tab, CR, LF) and make BenchmarkSweep, FbxMetricsExporter and BenchmarkRecorder delegate to it. Each type keeps its private Csv entry point, so the tests added by those PRs are unchanged and still pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
#151) Bounds-check hull/vertex/triangle ranges and index encodings from project-controlled sidecars before allocating or building meshes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…189) Clamp atlas resolution and padding before they reach xatlas, and reject sidecar save paths that escape the Assets folder. The resolution ceiling is 16384 so manually typed values are not silently reduced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…182) Store the optimized mesh colors in the sidecar and use them directly on replay, so merged and orphan vertices no longer end up black. The remap path stays as the fallback for legacy sidecars without color data. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Reject sidecar UV channel indices outside Mesh.SetUVs' 0..7 range before replay: the primary channel falls back to UV2, the auxiliary channel is skipped with a warning. Channel 0 stays valid because AO bakes can legitimately target UV0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Reject negative or non-multiple-of-three submesh index counts and accumulate the running total as long, so crafted counts can no longer wrap past the equality guard into a bad allocation. Bumps the postprocessor version so affected models are reimported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
A partially rebuilt remap left zeroed optimized vertices still referenced by restored triangles. Abort the replay instead, and stop counting legitimate -1 entries as matches. The quadratic nearest-neighbour pass 2 is removed with it: sidecars are imported automatically, so that scan can stall the Editor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Cap candidate comparisons in PickBestCandidate and RemapUvSetIfNeeded so degenerate sidecars cannot make an import quadratic, replace the duplicate-bucket rescan with a per-bucket cursor, and make the unused-sidecar fallback use swap-remove instead of List.RemoveAt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Finite doubles above float.MaxValue cast to Infinity when packed into the flat UV buffer handed to native xatlas; reject them like other non-finite results and keep the original UVs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Cap diagnostic UV2 snapshots per run, skip meshes above the UvPngWriter vertex/index limits, and reject oversized or negatively indexed input in UvPngWriter.Render. The snapshot budget is only consumed by meshes that actually yielded UV2 data. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
TransferResult.uv2 is allocated before the cancel checkpoints, so a cancelled transfer returned a zero-filled UV2 array that callers wrote to the mesh. Route every checkpoint through CancelTransfer, which restores the null-UV2 failure contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Async repack yields back to the editor while the native bridge still owns a single process-global atlas, so a second entry could destroy an in-flight atlas. Guard every managed session with a fail-fast reentrancy flag released after xatlasDestroy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Reject negative padding (which underflows to a huge uint in native xatlas), NaN/out-of-range stretch thresholds, out-of-range ARAP iterations, and cartesian products that explode the cell count. Surface the reason in the sweep UI and refuse to start. Also states the 0..200 ARAP range in the suite tooltip (folded in from #133). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…131) ComputeTotal3DAreaMeters copies mesh.vertices and every submesh index array; it ran at two OnGUI sites on every repaint. Cache the result keyed by the exact source-mesh references and recompute only on a mesh-set change; ExecRepackCoreImpl refreshes the same cache with the value it already computes. Labels stay live and the control count is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Mathf.Abs(int.MinValue) throws OverflowException, and the centroid/bbox hash can produce it. Route all five palette-index sites in UvCanvasView and ShellColorModelPreview through a shared NonNegativeColorKey helper and mark the hash arithmetic unchecked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
FindOverlapGroups is a quadratic shell-pair scan. Skip it above 512 source shells, log a warning, and fall back to the original fixed retry count so a highly fragmented mesh cannot stall the Editor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
SpatialPartitioner.DetectOverlap compared every face pair inside each grid cell, worst case O(gridCells * faces^2). Replace it with an inclusion-exclusion incidence count that is linear in face-cell memberships while preserving the shared-vertex exclusion contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
The hover pick read mesh.vertices and ran shell extraction over every triangle on each ~33 ms mousemove. Check index metadata first and enforce a per-hover triangle budget, sized (100k) to still cover typical LOD0 game meshes, with a rate-limited warning so a skipped mesh is visible rather than silently unhoverable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Cleanup, LOD generation and Model Builder all parsed the trailing _LOD<n> suffix with int.Parse and trusted the result: overflowing digits threw OverflowException and huge indices sized LOD arrays/loops unboundedly. Parse with int.TryParse and reject indices beyond the eight levels LODGroup supports. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
mesh.Clear() defaults to keepVertexLayout=true, so tangent/color/UV channels were retained once vertex data was re-set at the same count and the strip was a no-op. Pass false to drop the layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
NormalizeShellWinding was dead code and RepackSingle/RepackMulti hardcoded flippedShells=0 on the assumption that Weld had normalized winding, which it never did. Standalone Repack now normalizes its own local UV0 copies, so the caller's UV0 channel stays untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
The guards only checked for empty UV lists, but TransferCore/Extract index srcUv0/srcUv2 by vertex index, so a non-empty but short channel threw IndexOutOfRangeException. Require the channel length to match vertexCount. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
localVertCount * totalStride used unchecked int arithmetic to size the byte[] fed to unsafe pointer writes and the native call, so a huge mesh could overflow into an undersized buffer and write out of bounds. Size the buffers through a long-based budget check and validate the returned vertex count. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Group the 54 incorporated PRs under Unreleased ▸ Security (CI/workflows, vendored xatlas, sidecar replay validation, resource limits, output escaping) and Unreleased ▸ Fixed (UV transfer/repack correctness, undo and preview handling, LOD parsing, FBX export, collision meshes). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…low-up) NormalizeExportHierarchy clones each transformed node's mesh before baking the transform into vertex data (#175). DestroyImmediate on the temp export root only frees GameObjects, so those Mesh copies leaked on every export. Collect the copies through an optional sink list and release them via a new DestroyTempMeshes helper in the same finally blocks that destroy tempRoot — after ModelExporter.ExportObjects has read their vertex data. Only meshes created by the clone step are destroyed; sharedMesh originals are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (30)
WalkthroughИзменения усиливают CI и native-сборку, добавляют проверки входных данных и лимиты ресурсов, обновляют UV transfer, repack, LOD, AO и экспорт, а также добавляют регрессионные тесты и безопасную обработку CSV, HTML и shell-значений. ChangesЕдиный набор изменений
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/meta-check.yml (1)
89-97: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winИспользуйте строгий шаблон SemVer в обоих workflow.
Текущий шаблон принимает ведущие нули, например
01.02.003. Замените его в.github/workflows/meta-check.yml#L92и.github/workflows/version-bump.yml#L31на^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/meta-check.yml around lines 89 - 97, Replace the version-validation regex in the Check version format step of .github/workflows/meta-check.yml (89-97) with the strict SemVer pattern ^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$, rejecting leading zeros while allowing zero; apply the same regex change in .github/workflows/version-bump.yml (30-34).
🧹 Nitpick comments (4)
Native~/third_party/xatlas/xatlas.cpp (1)
1-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winЗафиксируйте ревизию снапшота xatlas.
README.mdсодержит URL upstream, но не содержит хэш коммита и дату. ДобавьтеNative~/third_party/xatlas/VERSIONс этими данными. Код xatlas не изменяйте.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Native`~/third_party/xatlas/xatlas.cpp around lines 1 - 42, Add Native~/third_party/xatlas/VERSION containing the xatlas upstream commit hash and snapshot date, using the upstream URL referenced by README.md to identify the revision. Do not modify xatlas.cpp or any other xatlas source code.Native~/src/collision.cpp (1)
66-74: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winПроверьте остальные параметры на границе native ABI.
ConvexDecomp_Computeвсё ещё преобразуетmaxHulls,resolution,maxVertsPerHullиminEdgeLengthвuint32_t. Отрицательные значения превращаются в большие значения и нарушают ограничения V-HACD. Отклоняйте такие значения в native-коде. ПроверяйтеfillModeпо диапазону0..2; преобразование вVHACD::FillModeне является неопределённым поведением, но неизвестное значение не выбирает ни одну ветвьVoxelize.Управляемый UI передаёт
maxHulls: 1..64,resolution: 10000..1000000,maxVertsPerHull: 8..255,minEdgeLength: 1..8иfillMode: 0..2.ConvexDecompSettingsи P/Invoke-метод остаются публичными, поэтому вызывающий код может обойти эти ограничения.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Native`~/src/collision.cpp around lines 66 - 74, Validate all remaining ConvexDecomp_Compute ABI inputs natively: reject non-positive or otherwise invalid values for maxHulls, resolution, maxVertsPerHull, and minEdgeLength before converting them to uint32_t, and reject fillMode values outside 0..2 before constructing VHACD::FillMode. Preserve the managed UI ranges as the accepted native boundary and return the existing failure result for invalid input.Editor/Tools/CleanupTool.cs (1)
1531-1542: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winДублирование
TryParseLodIndexвModelBuilderTool.cs.Согласно графу связей,
Editor/Tools/ModelBuilderTool.csсодержит идентичную реализациюTryParseLodIndex(та же регулярка, тот же порогMaxLodLevels). Логика разбора и ограничения индекса LOD дублирована в двух файлах. Обновление порога или паттерна в одном месте без синхронизации с другим создаёт риск расхождения поведения.Перенесите
TryParseLodIndex(и константуMaxLodLevels) в общий утилитарный класс (например,MeshHygieneUtility), уже используемый обоими инструментами.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Editor/Tools/CleanupTool.cs` around lines 1531 - 1542, Move the shared TryParseLodIndex implementation and MaxLodLevels constant from the tool-specific classes into the existing MeshHygieneUtility class, then update CleanupTool and ModelBuilderTool to call the utility and remove their duplicate definitions while preserving the current regex, case-insensitive matching, parsing, and bounds behavior.Editor/Framework/UvCanvasView.cs (1)
913-919: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueДублирование
NonNegativeColorKeyв двух файлах.Editor/Framework/UvCanvasView.csиEditor/ShellColorModelPreview.csнезависимо определяют идентичный приватный статический методNonNegativeColorKey, решающий одну и ту же задачу — безопасную нормализацию хеш-ключа без переполненияMathf.Abs(int.MinValue). Общий корень причины — отсутствие разделяемого helper-класса для этой операции.
Editor/Framework/UvCanvasView.cs#L913-L919: удалите локальный методNonNegativeColorKey, перенесите его в общий статический helper-класс (например, рядом сUvtLogили в новыйUvColorUtil).Editor/ShellColorModelPreview.cs#L107-L111: замените локальный методNonNegativeColorKeyвызовом того же общего helper-класса.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Editor/Framework/UvCanvasView.cs` around lines 913 - 919, Вынесите дублирующийся статический метод NonNegativeColorKey в общий helper-класс, сохранив безопасную обработку int.MinValue. В Editor/Framework/UvCanvasView.cs, строки 913-919, удалите локальную реализацию и используйте общий helper; в Editor/ShellColorModelPreview.cs, строки 107-111, также удалите локальную реализацию и замените её вызовом того же helper-класса.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-native.yml:
- Line 41: Update the actions/checkout step in the native build workflow to set
persist-credentials to false, ensuring the checkout does not store GITHUB_TOKEN
in .git/config.
In @.github/workflows/meta-check.yml:
- Line 87: Require exactly one root JSON object in both workflows: update
.github/workflows/meta-check.yml lines 87-87 to use jq -e -s 'length == 1 and
(.[0] | type == "object")' package.json, and add the same validation in
.github/workflows/version-bump.yml lines 30-40 before package.json is read or
written.
In @.github/workflows/version-bump.yml:
- Around line 35-37: In the version-bump step around CURRENT parsing and
NEW_PATCH calculation, validate that PATCH is below Bash’s maximum supported
integer before performing PATCH + 1. Reject the overflow boundary and stop the
workflow with a clear error instead of constructing or committing an invalid
NEW_VERSION.
In @.npmignore:
- Around line 21-56: Добавьте шаблон `.codex/` в `.npmignore`, чтобы каталог с
внутренними инструкциями, включая `.codex/agents/build.toml`, исключался из
npm-пакета при публикации.
In `@Editor/CsvUtil.cs`:
- Around line 21-31: Update the CSV handling used by BenchmarkSweep.AggregateRun
to preserve logical RFC 4180 records when fields contain CR/LF, rather than
splitting physical lines via File.ReadAllLines; alternatively normalize CR/LF in
Escape before writing. Ensure BenchmarkRecorder.BuildCsv and AggregateRun
round-trip a renderer name containing a newline without corrupting metrics or
sweep selection, and add the requested regression test.
In `@Editor/SpatialPartitioner.cs`:
- Around line 302-333: In DetectOverlap, deduplicate the vertex-pair keys
generated for each triangle before incrementing pairCounts, so a degenerate
triangle contributes at most once per unique pair. Update the pair-counting
logic around VertexPairKey and preserve the existing vertex and triple counting
behavior.
In `@Editor/Tools/ModelBuilderTool.cs`:
- Around line 13-14: Apply MaxLodLevels consistently in NormalizeHierarchy,
RebuildLodGroupFromNames, and AddLodLevel so no LOD name or group entry exceeds
eight levels; clamp or reject higher indices while preserving valid levels, and
emit an explicit warning when higher levels are encountered.
In `@Editor/Uv2AssetPostprocessor.cs`:
- Around line 806-818: Update the color restoration logic around
entry.optimizedColors so optColors is allocated whenever entry.optimizedColors
is non-null and its length equals optCount, regardless of rawColors
availability. Preserve copying optimized colors as the authoritative source,
while only using rawColors/remap for fallback reconstruction when optimized
colors are unavailable.
In `@Editor/UvPngWriter.cs`:
- Around line 50-53: Update the validation condition in UvPngWriter to reject
tris arrays whose length is not divisible by three by adding a tris.Length % 3
!= 0 check. Preserve the existing validation and return false behavior for all
invalid inputs.
In `@Editor/VertexAOBaker.Blur.cs`:
- Around line 80-95: В циклах поиска вокруг вершины добавьте отдельный счётчик
проверенных кандидатов, увеличивайте его непосредственно перед вызовом
TryConnectSeamVerts и ограничивайте им все три цикла и внутренний цикл.
Сохраните matched только для подсчёта успешных соединений, чтобы поиск
прекращался после MaxSeamCandidatesPerVertex проверок даже при отсутствии
совпадений.
In `@Editor/VertexAOBaker.cs`:
- Around line 216-219: Update the parallel baking flow in the method containing
Parallel.For so cancellation sets a shared cancellation flag and all iterations
stop contributing to correction and totalWeight once cancellation is observed.
After Parallel.For, check that flag and return the original ao copy instead of
applying partial results; preserve normal result application when the bake
completes without cancellation.
In `@Editor/XatlasRepack.cs`:
- Around line 187-203: Update the public repack flow around AcquireNativeSession
so session contention is reported through RepackResult.error instead of escaping
as an InvalidOperationException, preserving the existing result-based error
contract for RepackSingle and RepackMultiCore. Also ensure RepackUv always
destroys its temporary Instantiate mesh in a finally block, including when
RepackSingle fails before returning.
In `@Tests/Editor/VertexAOBakerBlurTests.cs`:
- Line 4: Удалите проверку времени выполнения через Stopwatch и условие
ElapsedMilliseconds < 20000 в тесте VertexAOBakerBlurTests, оставив ограничение
Timeout(30000) для зависших тестов; удалите ставший ненужным using
System.Diagnostics.
In `@Tools`~/gen.bat:
- Line 25: Validate the gallery-id and output arguments before the command
invocations in the batch script, especially the `%~5` value used by the
generation command and the corresponding arguments at the other referenced
commands. Permit only a strict safe character set, reject invalid values before
invoking `python`, and ensure the validated values cannot inject quotes or
cmd.exe metacharacters.
---
Outside diff comments:
In @.github/workflows/meta-check.yml:
- Around line 89-97: Replace the version-validation regex in the Check version
format step of .github/workflows/meta-check.yml (89-97) with the strict SemVer
pattern ^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$, rejecting leading
zeros while allowing zero; apply the same regex change in
.github/workflows/version-bump.yml (30-34).
---
Nitpick comments:
In `@Editor/Framework/UvCanvasView.cs`:
- Around line 913-919: Вынесите дублирующийся статический метод
NonNegativeColorKey в общий helper-класс, сохранив безопасную обработку
int.MinValue. В Editor/Framework/UvCanvasView.cs, строки 913-919, удалите
локальную реализацию и используйте общий helper; в
Editor/ShellColorModelPreview.cs, строки 107-111, также удалите локальную
реализацию и замените её вызовом того же helper-класса.
In `@Editor/Tools/CleanupTool.cs`:
- Around line 1531-1542: Move the shared TryParseLodIndex implementation and
MaxLodLevels constant from the tool-specific classes into the existing
MeshHygieneUtility class, then update CleanupTool and ModelBuilderTool to call
the utility and remove their duplicate definitions while preserving the current
regex, case-insensitive matching, parsing, and bounds behavior.
In `@Native`~/src/collision.cpp:
- Around line 66-74: Validate all remaining ConvexDecomp_Compute ABI inputs
natively: reject non-positive or otherwise invalid values for maxHulls,
resolution, maxVertsPerHull, and minEdgeLength before converting them to
uint32_t, and reject fillMode values outside 0..2 before constructing
VHACD::FillMode. Preserve the managed UI ranges as the accepted native boundary
and return the existing failure result for invalid input.
In `@Native`~/third_party/xatlas/xatlas.cpp:
- Around line 1-42: Add Native~/third_party/xatlas/VERSION containing the xatlas
upstream commit hash and snapshot date, using the upstream URL referenced by
README.md to identify the revision. Do not modify xatlas.cpp or any other xatlas
source code.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db1a2353-bdde-489b-89e6-4120bcc5df6f
📒 Files selected for processing (59)
.claude/skills/skills-overhaul-plan.md.github/workflows/build-native.yml.github/workflows/meta-check.yml.github/workflows/version-bump.yml.npmignoreCHANGELOG.mdDocumentation~/EXPERIMENTS.mdDocumentation~/TRANSFER_BENCHMARK.mdEditor/ArapParameterization.csEditor/BenchmarkRecorder.csEditor/BenchmarkSweep.csEditor/CollisionMeshBuilder.csEditor/CsvUtil.csEditor/CsvUtil.cs.metaEditor/FbxMetricsExporter.csEditor/Framework/UvCanvasView.csEditor/Framework/UvToolHub.csEditor/GroupedShellTransfer.csEditor/MeshOptimizer.csEditor/Settings/TestSuiteAsset.csEditor/ShellColorModelPreview.csEditor/SpatialPartitioner.csEditor/Tools/CleanupTool.csEditor/Tools/CollisionMeshTool.csEditor/Tools/LightmapTransferTool.csEditor/Tools/LodGenerationTool.csEditor/Tools/ModelBuilderTool.csEditor/Tools/VertexAOTool.csEditor/Uv2AssetPostprocessor.csEditor/Uv2DataAsset.csEditor/UvPngWriter.csEditor/VertexAOBaker.Blur.csEditor/VertexAOBaker.Gpu.csEditor/VertexAOBaker.csEditor/XatlasRepack.csNative~/CMakeLists.txtNative~/src/collision.cppNative~/third_party/xatlas/xatlas.cppNative~/third_party/xatlas/xatlas.hREADME.mdTests/Editor/BenchmarkRecorderTests.csTests/Editor/BenchmarkRecorderTests.cs.metaTests/Editor/BenchmarkSweepTests.csTests/Editor/BenchmarkSweepTests.cs.metaTests/Editor/CleanupToolTests.csTests/Editor/CleanupToolTests.cs.metaTests/Editor/FbxMetricsExporterTests.csTests/Editor/FbxMetricsExporterTests.cs.metaTests/Editor/LodGenerationToolTests.csTests/Editor/LodGenerationToolTests.cs.metaTests/Editor/SpatialPartitionerTests.csTests/Editor/SpatialPartitionerTests.cs.metaTests/Editor/ToolSettingsValidationTests.csTests/Editor/ToolSettingsValidationTests.cs.metaTests/Editor/VertexAOBakerBlurTests.csTests/Editor/VertexAOBakerBlurTests.cs.metaTests/Editor/XatlasRepackGroupMergeTests.csTools~/build_gallery.pyTools~/gen.bat
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78a6e7a0f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+55
to
+63
| if (!vertices || !indices || vertexCount <= 0 || indexCount < 3 || indexCount % 3 != 0) | ||
| return nullptr; | ||
|
|
||
| // V-HACD treats indices as unsigned and dereferences them without bounds checks. | ||
| // Validate at the native API boundary so malformed meshes cannot cause OOB reads. | ||
| for (int i = 0; i < indexCount; i++) | ||
| { | ||
| if (indices[i] < 0 || indices[i] >= vertexCount) | ||
| return nullptr; |
There was a problem hiding this comment.
Package consumers execute the prebuilt files under Plugins/, but all three plugin blobs are unchanged even though this native boundary now adds index validation and recursion limits. The checked-in Linux plugin still returns a non-null V-HACD context for an out-of-range triangle index that this source rejects, so the advertised native hardening is absent from the shipped package; rebuild from this source and commit the resulting platform artifacts.
AGENTS.md reference: AGENTS.md:L19-L20
Useful? React with 👍 / 👎.
Comment on lines
+1491
to
+1493
| if (candidateChecksRemaining <= 0) | ||
| UvtLog.Warn($"[UV2 Postprocess] '{mesh.name}': {channelLabel} remap comparison limit reached; " + | ||
| "remaining vertices will keep zero."); |
There was a problem hiding this comment.
When a legitimate remap requires more than one million comparisons—for example, a few thousand vertices that miss their quantized buckets after a small position/order change—the fallback stops here and returns an array whose remaining entries are zero. ApplyUv2Entry then unconditionally passes that array to mesh.SetUVs, so an automatic reimport silently replaces part of the stored UV channel with (0,0); reaching the budget should make this replay fail without modifying the mesh.
Useful? React with 👍 / 👎.
Comment on lines
+92
to
+95
| if (TryConnectSeamVerts(neighbors, positions, normals, uv0, | ||
| vi, vj, posEpsSq, normThresh, uvEps, | ||
| crossHardEdges, crossUvSeams); | ||
| crossHardEdges, crossUvSeams)) | ||
| matched++; |
There was a problem hiding this comment.
On dense meshes containing several nearby but non-coincident vertex clusters in one spatial cell, TryConnectSeamVerts returns false for most candidates, so matched is never incremented and the new 256-candidate limit does not bound those comparisons. Since the cell is ten times wider than the position tolerance, a crafted or degenerate cell can still make every vertex scan nearly the whole group and retain the original quadratic Editor stall; track comparison attempts separately from successful position matches.
Useful? React with 👍 / 👎.
Comment on lines
+174
to
+175
| if (snapshotMesh != null && pngSnapshotsCaptured < MaxPngSnapshots && | ||
| IsPngSnapshotWithinLimits(snapshotMesh)) |
There was a problem hiding this comment.
Once a run records 32 PNG snapshots, or whenever a mesh exceeds the PNG safety limits, this condition leaves uv2Snap and trisSnap null; the later atlas-utilization calculation therefore records the default 0 for that mesh. BenchmarkSweep.Score averages this field with a weight of 100, so larger suites can select a different winner solely because later rows were denied diagnostic PNGs; compute the metric independently and apply the cap only to retained image data.
Useful? React with 👍 / 👎.
Comment on lines
+806
to
+807
| if (optColors != null && entry.optimizedColors != null && entry.optimizedColors.Length == optCount) | ||
| System.Array.Copy(entry.optimizedColors, optColors, optCount); |
There was a problem hiding this comment.
When AO creates vertex colors on an FBX that originally had no color stream, optimizedColors is saved but replay initializes optColors to null because rawColors is empty. This new copy is consequently skipped and the final mesh.SetColors is also skipped, so reimport silently drops the baked AO data; allocate optColors when a valid stored optimized-color array exists, even if the raw mesh lacks that attribute.
Useful? React with 👍 / 👎.
| } | ||
| } | ||
|
|
||
| if (CanApplyUv2(ctx.HasRepack, ctx.HasTransfer)) |
There was a problem hiding this comment.
A cancelled or failed ExecRepackCoreImpl still sets ctx.HasRepack = true unconditionally after processing its failed results, and it can also leave an older repackedMesh attached to an entry. Because this new condition exposes Apply based solely on that flag, a user can apply original or stale UV2 data immediately after the UI reported a repack failure; derive the state from successful current results and clear prior repack outputs before starting a new run.
Useful? React with 👍 / 👎.
…191 review) - build-native.yml: checkout with persist-credentials: false so the job's read-only intent isn't undermined by a token left in .git/config. - meta-check.yml / version-bump.yml: `jq empty` accepts a stream of several root values; slurp and assert a single object root instead. Verified: a two-document file passes `jq empty` and makes `jq -er .version` emit two lines. - Both workflows: strict SemVer regex — the previous one accepted leading zeros (01.2.3). - version-bump.yml: bound the patch component before $((PATCH + 1)), which wraps silently at the 64-bit boundary. Compares digit count because a numeric test on a 20-digit value errors out instead of comparing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…ge (PR #191 review) The reported path (.codex/agents/build.toml) does not exist on this branch, but the same leak is real for .claude/: `npm pack --dry-run` listed 42 .claude/skills/** files plus .vscode/settings.json and .prettierignore in the tarball. Ignore all of them (.codex/ included for the day it appears). Verified: packed file count 237 -> 195, no dotfiles remain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…ne (PR #191 review) CsvUtil.Escape kept CR/LF inside a quoted field (valid RFC 4180), but BenchmarkSweep.AggregateRun reads the report back with File.ReadAllLines and parses each physical line as one record — an asset name containing a newline therefore shifted every column after it. Flatten CR/LF/TAB to spaces before quoting; the formula-neutralising check still runs on the original string so a leading control character keeps its apostrophe. Tests: the three CR/LF/TAB escape cases now expect flattened output, plus a line-break flattening case and a BuildCsv -> ParseCsvRow round-trip that asserts the record survives a File.ReadAllLines-style split. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
review) A face with a repeated index collapses two of its three edges onto the same unordered pair, so DetectOverlap counted that pair twice per face. The inclusion-exclusion read side subtracts the pair count once, so the face's shared-face total came out too low and the face was reported as UV overlap. Emit each distinct pair once per triangle; vertex and triple counting are unchanged (they already skip repeated indices). Verified by replaying both code paths over the test inputs: face (0,7,7) sharing vertex 0 with three other faces in the same grid cell scored 3 of 4 before and 4 of 4 after, and both existing test cases keep their results. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…tes (PR #191 review) TryParseLodIndex + MaxLodLevels were copy-pasted in CleanupTool and ModelBuilderTool. Both now call one internal helper in MeshHygieneUtility (name hygiene already lives there), with the regex compiled once instead of re-parsed on every call. Behaviour is identical apart from a null-name guard. The cap was only honoured on one of the three ModelBuilderTool paths: - RebuildLodGroupFromNames already went through TryParseLodIndex — unchanged. - NormalizeHierarchy numbered mesh children sequentially with no ceiling, so a root with nine mesh children got a _LOD8 name that RebuildLodGroupFromNames then silently rejected. Stop numbering at the ceiling and warn instead of inventing unusable suffixes. - AddLodLevel appended levels without a ceiling — refuse past the eighth. GetLodIndexFromName is left alone: it only builds an inspector label and never indexes a LOD array. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
… remaps (PR #191 review) Confirmed both reports by reading the allocation site: 1. optColors was allocated only when the RAW FBX mesh carried a color channel (mesh.colors32 with rawCount entries), so entry.optimizedColors — the authoritative copy, since merged and orphan vertices cannot be rebuilt from the remap — was silently dropped for any mesh whose colors were produced after import (baked vertex AO on a color-less FBX). Allocate when either source exists; the remap fallback now runs only when the sidecar has no optimized colors AND the raw mesh has some. The legacy path is unaffected: optimized colors require ground truth, which that path does not have. 2. When the remap comparison budget ran out mid-fallback, the half-filled array (zeros for everything after the cut) was handed back and written to the mesh unconditionally, flattening part of the UV channel to (0,0) on every auto-reimport. Abort the entry instead and leave the mesh untouched, matching the stale-remap abort. The primary channel also gets the length check the auxiliary channel already had. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…view) Render already rejected too-few and too-many indices but accepted a trailing partial triangle, which every consumer then truncated with tris.Length / 3 — the diagnostic PNG would quietly not match its input. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…d correction (PR #191 review) - BlurAO's seam pass only counted candidates that actually connected, so a vertex whose grid neighbourhood holds thousands of near-but-not-matching candidates still scanned all of them — the quadratic worst case the cap was supposed to remove. Add a second cap on candidates examined (4096) that bounds all three cell loops and the inner scan; the 256 matched cap is unchanged, and it still trips first on the coincident-vertex case the existing test covers. - FaceAreaCorrection applied whatever correction/totalWeight the cancelled Parallel.For had already accumulated: loopState.Stop() only stops new iterations. Record the cancellation and return the untouched AO copy; normal completion is unchanged. - VertexAOBakerBlurTests: drop the ElapsedMilliseconds < 20000 assertion (it measures the CI runner, not the budget) and the now-unused System.Diagnostics import. [Timeout(30000)] still guards the test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…mp mesh (PR #191 review) AcquireNativeSession threw InvalidOperationException straight through the public RepackSingle / RepackMulti / RepackMultiAsync surface, while every caller (LightmapTransferTool's per-mesh loop, the repack tests) consumes the RepackResult.error contract — so a concurrent repack surfaced as an unhandled exception instead of a per-mesh failure. Return the busy state as RepackResult.error; RepackMultiCore stamps it on every mesh since nothing was packed. The release side is untouched: acquisition still happens outside the try, so a failed claim can never release someone else's session. RepackUv now destroys its temporary Instantiate copy in a finally. The failed -result path already destroyed it, but an exception escaping RepackSingle — including the one above — leaked the mesh into the editor session. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…et (PR #191 review) RecordMesh read UV2 + triangles only while the 32-PNG cap had room, but that same data feeds atlasUtilization — a scored metric that BenchmarkSweep weights x100. Past the 33rd recorded mesh (or for any mesh over the PNG size limits) the metric silently recorded 0, so a large suite could crown a different sweep winner purely from recording order. Read the data for every row (still bounded by the existing mesh-size sanity limits) and apply the 32-snapshot cap only to what is retained for the PNG dump. The pngSnapshotsSkipped counter keeps its original meaning. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…es (PR #191 review) ExecRepackCoreImpl set ctx.HasRepack = true after the result loop regardless of outcome, and left each entry's previous repackedMesh in place when the new run failed. Since #170 the Apply UV2 section is drawn purely off that flag, so after a failed or cancelled repack the user could apply a stale result — or, with no prior repack, the original UV2 (GetResultMesh falls back to originalMesh). Now each run clears its entries' repack output up front (destroying the old mesh, which the success path used to overwrite and leak), and HasRepack is derived from whether any entry currently holds a repacked mesh. Deriving it rather than assigning false keeps per-mesh grouping correct: that path calls this method once per group, and a later failing group must not erase an earlier group's success. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…review) ConvexDecomp_Compute checked the geometry arguments and clamped maxRecursionDepth, but cast maxHulls, resolution, maxVertsPerHull and minEdgeLength straight to uint32_t and fillMode straight to the FillMode enum. A negative int wraps to a huge unsigned value in V-HACD (a negative resolution becomes a multi-billion-voxel grid) and an out-of-range fillMode matches no enum case (confirmed 0..2 in third_party/VHACD.h). Reject all of them with the existing nullptr failure result, before CreateVHACD allocates anything. The editor-side ranges all remain valid. Checked with g++ -fsyntax-only -std=c++17 -I third_party (clean). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…view) The xatlas sources under Native~/third_party/xatlas carried no provenance, so nothing recorded which upstream revision the shipped binaries were built from. Add a VERSION file naming the upstream repo, commit f700c77, the license and the snapshot/verification date, plus the rule that local changes go in the bridge rather than the snapshot. xatlas.h / xatlas.cpp are untouched, and Native~ is tilde-hidden so the file needs no .meta. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…eview) Confirmed the #135 whitelist only constrains the option NAMES (%~2 / %~4). The values — %~1, %~3 and %~5 — went through untouched, and cmd.exe substitutes an argument into the python line before parsing that line, so a value carrying a double quote could close the quoting and run whatever followed it. Each forwarded value is now matched against a strict charset (letters, digits, _ - . ~ : \ / and space) and rejected with exit /b 3 otherwise. The test runs through delayed expansion, which substitutes the value after the line is parsed, so the value under test cannot itself be read as syntax. No cmd.exe in this environment, so the batch flow was desk-checked and the charset decisions were verified with the equivalent POSIX class: the example invocation, Windows drive paths and paths with spaces are accepted; quotes, & | < > ^ % ! ( ) ; and apostrophes are rejected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
NonNegativeColorKey was duplicated verbatim in UvCanvasView (2D canvas) and ShellColorModelPreview (3D model preview) — the two systems must agree on how a shell hash maps to a palette slot, and a copy each is how they drift. One internal helper in Editor/UvHashUtil.cs (with .meta, fresh GUID) now serves both; the int.MinValue fold is documented in one place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…eader The vendored xatlas directory is on the native include path; on case-insensitive filesystems (macOS/Windows) #include <version> resolved to third_party/xatlas/VERSION and broke the build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Owner
Author
|
Раунд ревью обработан: все 22 замечания (15 CodeRabbit + SemVer/nitpicks + 3 уникальных от Codex) проверены по коду и закрыты 15 коммитами ( Ключевое из раунда:
По замечанию Codex о пересборке Также напоминание: C#-код не компилировался в CI этого репозитория (нет Unity-джоба) — перед merge стоит открыть проект в Unity и прогнать EditMode-тесты. Generated by Claude Code |
The LOD/group FBX export path created two more kinds of temporary meshes that were never released: the per-entry `exportMesh` clones (Instantiate of the result mesh, for both the replace-in-clone and the add-missing-LOD branches) and the `stripped` collision meshes rebuilt for every `_COL` node. DestroyImmediate on tempRoot only frees GameObjects, so both leaked on every export. Route them through the same sink + DestroyTempMeshes pattern introduced in 78a6e7a: the per-group list (renamed `bakedMeshes` -> `tempMeshes`, since it now also carries export clones and stripped collision meshes) collects each mesh right after it is created, and the existing finally block releases them after ModelExporter.ExportObjects has read their vertex data. Only meshes created here are destroyed. The export clones live solely on tempRoot and in a local dictionary; `stripped` copies srcCol's vertex and index data instead of aliasing it, so srcCol (which may be an FBX sub-asset) is untouched. Nothing downstream holds them: renameMap is string->string, RelinkSceneMeshReferences reloads meshes from the reimported FBX, and the sidecar path builds its own clone from resultMesh. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…(leak cleanup) CollisionMeshTool.GetCollisionMeshesFromSidecar allocates a fresh Mesh per hull from the sidecar's serialized position/index arrays. ExportFbx attached them to _COL nodes on tempRoot and then replaced them with the stripped copies, so they were unreachable but never released — a leak on every export of an FBX that has sidecar collision data. Append them to the same per-group tempMeshes sink at the point where they are attached, so the existing DestroyTempMeshes call in the finally block frees them after ModelExporter.ExportObjects. Collecting at the sidecar loop (not the strip loop) is what makes this safe: every mesh on both of that method's return paths comes from the single `new Mesh()` construction site, so nothing shared or asset-backed can enter the sink. The strip loop's srcCol is deliberately still not collected — for collision nodes that came from the source FBX rather than the sidecar it is a real FBX sub-asset. The same caller-owns-the-meshes contract is already assumed by VertexAOTool, which puts them in batch.temporaryMeshesToDestroy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
…abled BenchmarkRecorder.NewRun opened a recording session on every Run Full Pipeline / Repack / Transfer All click, so a production transfer run wrote <projectRoot>/BenchmarkReports/ plus a per-mesh <run>_png/ subfolder of CSV/JSON/PNG analysis artefacts that nobody asked for. Gate the session on MeshLabProjectSettings.showDebugUI — the flag that already hides Parameter Sweep, Log filters, UV0 Analysis & Fix, the Repack "Advanced (debug)" block, the Mesh Lab ▸ Export FBX Metrics menu items and the Sweep Test Suite create action. With the flag off NewRun returns the existing NoOpScope, Current stays null and no folder or file is produced; every consumer already guards on `Current != null` / `_bench is BenchmarkRecorder`, so nothing logs or throws. Sweep and the FBX metrics exporter are reachable only from debug-gated UI, so they keep working unchanged. Existing report folders on disk are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
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
2 participants
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.
Summary
CsvUtilutility used by BenchmarkRecorder, BenchmarkSweep, and FbxMetricsExporterChanged Zones
Editor/— Editor tools / UIPlugins//Native/— Native plugins.github/— CI / workflowsREADME.md,CHANGELOG.md)Checklist
.metafiles present for all new files/directoriesTest Plan
Review Notes
Native~/third_party/xatlas/(MIT licensed, properly attributed)s_nativeSessionInFlightcounter to prevent concurrent atlas access during async operationshttps://claude.ai/code/session_0195YC4HWo1rQqEdFBb8p8jE
Summary by CodeRabbit