Add multi-case sweep, visual-defect metrics, and transfer diagnostics - #118
Open
SashaRX wants to merge 110 commits into
Open
Add multi-case sweep, visual-defect metrics, and transfer diagnostics#118SashaRX wants to merge 110 commits into
SashaRX wants to merge 110 commits into
Conversation
Adds `UvtLog.Category.TransferDiag` and emits a single concise summary line at the end of every `TransferCore` call: src/tgt shell counts, matched/rejected, ShellStatus + method histograms, fragmentsMerged, dedupConflicts, overlap/consistency fixes, mean/max 3D match distance, and topology iterations/fixed/capHit. Unblocks step 1 of the next-session checklist in Documentation~/TRANSFER_LOD_QUALITY_PLAN.md (identity sanity test, per-LOD ratio sweep) without trawling per-shell verbose logs. New category bit slots into the existing Log filters foldout via the auto-enumerated `s_logCategories` list — no UI change needed. Diagnostic-only: no behavioural change to the transfer pipeline.
GroupedShellTransfer.Phase 2b had a bypass that allowed multiple hint-matched targets to share a source even when their UV0 bboxes overlapped, on the assumption "previous LOD told us all these belong on this source, interp will be valid". Under standard UV0→UV2 interp that assumption is wrong for symmetric / tiled copies whose UV0 sub-regions coincide: both targets sample identical source triangles and bake identical UV2 onto the atlas. Carousel LOD2 TransferDiag output (new in fb3233f) confirmed the failure mode with six pairs of target shells producing byte-identical UV2 fingerprints. Remove the bypass and fall through to the existing eviction sort (hint-matched → non-merged → best avg3D). The strongest claimant keeps the source; the rest queue for FindBestSourceShell with `claimed` excluded, which on tiled LODs finds the unused source siblings (e.g. LOD0 src138/139/140 stay available once src135/136/137 are taken). EXPERIMENTS.md updated with the data and rationale.
… bypass Records the actual TransferDiag numbers from Carousel Full Pipeline before and after the 5340f67 fix. 10 duplicate UV2 fingerprint pairs (LOD2+LOD3) went to 0; LOD2 mean match distance dropped 39%, max 40%. Status A/D/P histograms unchanged. One small regression noted (new force3D UV2 overlap on LOD3 t66↔t40, +1 topo cap-hit on LOD2) — not blocking, the duplicate UV2 was the visible user-facing bug. Also lists what stayed broken so the next experiment has a target list (force3D overlaps on ARAP-reparameterized shells, topology cap-hit cycles, sliver/degenerate output from xform on ribbons).
…ed dedup bypass" This reverts commit 442f2bb.
This reverts commit 5340f67.
Existing sweep metrics (shellsRejected, overlapShellPairs, coverage) flagged the pipeline as "solid" across 60 cells × 4 models, but the user sees lightmap artefacts on Carousel that those metrics miss: symmetric copies sharing UV2 regions, force3D fallback overlaps, composite-broken single-source fallback, and grossly-wrong Phase 2 source picks. Add four counters that catch each: - uv2DuplicatePairs — pairs of target shells with byte-identical quantised UV2 fingerprint hashes (silent bleeding between distinct instances). Rejected/Unmatched excluded so empty-hash sharing doesn't inflate. Captured in the existing fingerprint loop. - compositeBrokenCount — Phase 3 composite-vs-best-source area check ratio >2× → fallback to single-source. Already logged; now counted. - severeMismatchCount — target shells whose chosen source is >10% of mesh diagonal away in 3D. Almost always a wrong-source assignment. - shellsOverlapFixed (pre-existing) — aliased as force3D overlap count in the sweep aggregator and HTML gallery so the row labels are clear. Sweep changes: - BenchmarkRecorder writes all 4 to CSV + JSON. - BenchmarkSweep.RunSummary carries them; AggregateRun sums across target LODs; Score() applies penalties (-50 dup, -30 force3D, -10 composite, -20 severe) chosen so each defect class has comparable weight to existing slivers/overlaps. - summary.csv, winner.json, index.html show the new columns. - TransferDiag log line gets a `DUP=/COMP=/SEVERE=` segment. - TRANSFER_BENCHMARK.md documents the new metrics + Go/Stop thresholds. No algorithm change. Lets the next refactor PR be validated by sweep delta instead of eyeballing screenshots.
Existing Run Sweep gates on whatever LODGroup the operator dragged into the tool — to cover the four canonical models (Carousel/Playground/ WateringCan/Wooden_Box_Long) the operator had to manually swap the ObjectField and rerun four times. Easy to forget; easy to skip a model that the next refactor regresses. New Run Multi-Case button (Setup → Parameter Sweep, right of Run Sweep) iterates every TestSuiteAsset.cases[]: 1. LoadAssetAtPath<GameObject> on the case's fbxAsset → InstantiatePrefab into the scene, marked HideFlags.DontSave so the spawn doesn't trip the dirty-scene flag. 2. Resolve LODGroup via lodGroupPath if set, else GetComponentInChildren. 3. ctx.Refresh + OnRefresh → tool now points at the spawned model. 4. ExecSweep with a per-case sweepDir = BenchmarkReports/sweep_<ts>_<label>/ so each model gets its own summary.csv / winner.json / index.html. 5. DestroyImmediate in finally; ctx.Refresh(null) just before destruction so straggling UI repaints don't dereference a half-dead LODGroup. ExecSweep now takes an optional sweepDirOverride parameter — the no-arg overload preserves the existing single-model behaviour. UI state (original LODGroup wiring) is restored on exit, including the cancelled / threw paths. TRANSFER_BENCHMARK.md gets a new "Multi-case sweep" section plus a pandas snippet for cross-model aggregation. No algorithm change. Step 2 of the sweep-first refactor plan.
Owner
Author
|
@codex review |
![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: 14145e2f84
ℹ️ 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".
P1 — RestoreWorkingMeshes before LODGroup switch (LightmapTransferTool.cs:1718) ExecMultiCaseSweep was calling ctx.Refresh on each spawned case without first restoring the operator's original working meshes. If repacked/transferred meshes were live on the original LODGroup, MeshEntries got wiped while the temp meshes stayed assigned in-scene and the FBX baseline references were lost — reloading the original group at the end would then treat the temps as the new baseline. Call ResetWorkingCopies() once at the start of the multi-case loop (only when origLodGroup is non-null) so the AGENTS.md LODGroup lifecycle invariant is honoured. P2 — Per-case sweep dirs collision-proof (LightmapTransferTool.cs:1816) Two cases with labels that sanitise to the same slug (e.g. "Chair A" and "Chair/A" both collapsing to "Chair_A") shared a directory and overwrote each other's summary.csv / winner.json / index.html. Prefix the case index in the dir name: sweep_<ts>_<ci>_<label>/. P2 — uv2DuplicatePairs counted combinatorially (GroupedShellTransfer.cs:3603) The old loop incremented by 1 for every extra shell after the first hash hit — a group of 3 shells reported 2 pairs instead of 3, a group of 4 reported 3 instead of 6, under-penalising visibly broken runs in sweep scoring. Now count group sizes then sum k*(k-1)/2 per group. P2 — Duplicate-pair hash on the FINAL UV2 (GroupedShellTransfer.cs:3603) EnforceShellTopologyOnUv2 can shift verts after the fingerprint loop, so the duplicate count was computed on stale UV2. The original fingerprint log line stays pre-topology (useful for cross-branch diff of raw Phase 3 output); the new post-topology block recomputes hashes for duplicate counting on the bytes that actually get written. P2 — severeMismatchCount after final reassignment (GroupedShellTransfer.cs:3666) Moved out of the middle of Phase 2b dedup to the same post-topology block. All sweep-scoring counters now share one consistent snapshot taken after Phase 2 and Phase 3 finish, so a late merged-shell reassignment can't make the metric reflect a stale assignment. No behavioural change to the transfer algorithm itself — only metric accuracy and lifecycle safety. Sweep delta after these fixes should show the same shells flagged but with corrected counts.
New SweepMatrix axes: - internalOversampleVariants (default [4]) — xatlas internal pack multiplier. internalRes = resolution × oversample. Lets us A/B the density-spread win from EXPERIMENTS.md a218a2b. - symSplitThresholdModeVariants (default [LegacyFixed]) — toggles SymmetrySplitShells.CurrentThresholdMode per cell, so the Adaptive proposal (EXPERIMENTS.md 2026-04-15) can be validated against LegacyFixed without rebuilding. Both axes thread through CellConfig → BenchmarkRecorder CSV/JSON → BenchmarkSweep summary/winner/HTML → recovery regex (so mid-crash recovery on the new label format still works). HTML thead/tbody column indices shifted to accommodate. Provenance manifest (manifest.json next to summary.csv): - package.{name, version, gitSha, gitBranch, gitDirty} — UPM PackageInfo + git rev-parse (best-effort; runs in pkg resolvedPath when available so the SHA reflects the package commit, not the consuming project) - unity.{version, platform}, host.{user, machine, os, processor} - sweep.{cellCount, caseCount, sweepLabel, matrix, scoringWeights} — literal mirror of SweepMatrix + snapshot of Score() weights so a metric delta six months later isn't silently caused by a constant change Auto-archive: - ZipFile.CreateFromDirectory copies the just-written sweep_<ts>/ into BenchmarkReports/Archive/<sweepDirName>.zip with CompressionLevel.Optimal, non-destructive (source dir stays). Failure logs a warn but never propagates so a successful sweep isn't killed by a zip hiccup. - Operator can point Drive/Dropbox sync at BenchmarkReports/Archive/ for chronological off-machine history. Multi-case sweep gets manifest + zip per case automatically (each case calls ExecSweep which now does both). TRANSFER_BENCHMARK.md documents the 7-axis matrix, label format, manifest schema, and Archive/ sync workflow.
UnityEditor exposes its own legacy PackageInfo (Asset Store metadata) alongside UnityEditor.PackageManager.PackageInfo (UPM); UnityEngine has a CompressionLevel enum for textures while System.IO.Compression has the one ZipFile needs. With both root namespaces imported the compiler can't pick. Add explicit using-aliases so the file compiles with no source-level qualification churn.
… transfer
Read-only Editor menu item `Mesh Lab → Diag → Hierarchical Containment
Probe`. For every face of every non-deepest LOD of the selected LODGroup
it finds the nearest deepest-LOD face by 3D centroid distance, computes
the angle between normals, and records the area ratio. Output:
- BenchmarkReports/hierdiag_<ts>_<lodGroupName>.csv — one row per
fine-LOD face: groupKey, lodIndex, faceIndex, parentFaceIndex,
centroidDistance, angleDeg, areaRatio, fineArea, parentArea.
- Console summary (under UvtLog.Category.Benchmark) with:
- Coverage % at θ ∈ {15, 30, 45, 60, 90}° + retained-area note
- Area ratio percentiles (p50 / p90 / p99 / max) + count of
fine-larger-than-parent (promotion candidates)
- Per-LOD breakdown (face count, mean θ, θ<30° %, θ<60° %)
Purpose: validate the inverse-hierarchical pipeline design BEFORE
writing HierarchicalRepack.cs + InverseTransfer.cs. Sweep data on the
existing pipeline showed parameter tuning can't move the visual-defect
counters (uv2DuplicatePairs / compositeBrokenCount / severeMismatch);
the proposed architecture rewrite is the next step but needs empirical
confirmation that containment + normal-sign correspondence is enough
on real assets. The probe answers that on Carousel / Wooden_Box_Long /
Playground / Gazebo before any algorithm code is written.
No changes to existing pipeline, asmdef, or UI. Brute-force O(N×M)
nearest-face match — sub-second on <10k-face meshes. Re-runs are
deterministic (same face order, same matches).
GO/STOP criteria for proceeding to the full implementation are in
/root/.claude/plans/logical-growing-chipmunk.md.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04573133f4
ℹ️ 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".
P1 — Multi-case mesh leak (LightmapTransferTool.cs:1897) After ExecSweep returns for a case, MeshEntries still hold the last cell's repackedMesh/transferredMesh refs (Object.Instantiate clones that don't belong to the spawned GameObject hierarchy and won't be collected when the GO is destroyed). Previous flow called ctx.Refresh(null) first → those refs dropped on the floor → DestroyImmediate(spawned) collected only the GameObject, not the temp meshes. Repeated multi-case sweeps accumulated unreleased Mesh objects until editor OOM. Fix: call ResetWorkingCopies() BEFORE ctx.Refresh(null) — matches the AGENTS.md LODGroup-lifecycle invariant. Same pattern already used at the multi-case entry where original LODGroup is reset. P2 — Cell count UI under-reports new axes (LightmapTransferTool.cs:493) Run Sweep / Run Multi-Case button labels showed only 5-axis product; internalOversampleVariants and symSplitThresholdModeVariants were ignored in the UI math but multiplied in ExecSweep's cartesian loop. Operators with oversample=[1,2,4] would see "Run Sweep (5)" and actually launch 15 cells. Now the label uses all 7 axis lengths. P2 — ARAP recommendation confounded by new axes (BenchmarkSweep.cs:803) ComparePair used by BuildRecommendation searched for an ARAP-toggle partner of the winner. It matched on res/pad/bdr/stretch but ignored internalOversample and symSplitMode. With those axes now active the "ARAP helped" sentence in winner.json/index.html could be computed from a run differing on three knobs, producing a wrong recommendation. Fix: require exact match on internalOversample AND symSplitMode in addition to res/pad/bdr. No behavioural changes to algorithm or scoring — only correctness on metadata reporting and resource cleanup.
v1 probe data on 4 canonical models showed coverage at θ<45° between 44% (Wooden_Box_Long) and 79% (Playground) — well below the GO threshold of 90%. But the failure was the probe, not the architecture: v1 matched each fine face to a single nearest deepest-LOD TRIANGLE by centroid. On a tessellated deepest LOD (box's front face split into 2 tris) that's a tessellation artifact, not a real correspondence failure: a single LOD0 front-face would land on one of the two box half-tris with arbitrary normal pick and bogus area ratio (ratio>1 just because LOD0 face is bigger than one half). v2 does what HierarchicalRepack would actually do at runtime: 1. Extract 3D shells from the deepest LOD via union-find on face adjacency (shared-edge) with normal threshold ≤30° — same convention as xatlas's hard-edge analysis. For a clean box that's 6 shells (one per side); for a cylinder ~1 shell (long strip); for irregular geometry it falls through to per-face shells. 2. For each shell: area-weighted dominant normal + centroid + total area (the same data HierarchicalRepack would use as atlas regions). 3. For each fine face: K=10 nearest shells by centroid, then pick the one with smallest angle to its dominant normal. Reports angle and area ratio against the full shell, not a single triangle. Both modes are reported side-by-side (CSV has both columns, console summary prints FACE-LEVEL and SHELL-LEVEL blocks). The shell-level numbers are the ones the GO/STOP criteria in /root/.claude/plans/logical-growing-chipmunk.md actually map to. K=10 was chosen empirically: 1st-nearest by centroid is unreliable on curved deepest LODs (a wedge-corner triangle can be closer to a front-face shell's centroid than to its own ceiling shell). Brute-force O(N×M) cost is preserved — sub-millisecond per fine face on probed meshes. No algorithm change yet — still read-only diagnostic. Re-run the menu item on the 4 models; the SHELL-LEVEL summary line is the go/no-go signal.
v2 shell-matching closed the tessellation gap but two of the proposed promotion criteria (angle, area ratio) miss a class of false matches: geometry that is parallel to a parent shell's plane but offset along its normal — e.g. a bolt sticking 30 cm out of a wall. Angle ≈ 0°, ratio « 1, but projecting the bolt's UV into the wall's atlas region would put its lightmap data on top of the wall's, smearing both. v3 measures `shellPerpDistanceNorm = |dot(fine.centroid - shell.centroid, shell.dominantNormal)| / meshDiagonal` per face — perpendicular offset from the fine centroid to the parent shell's mean plane, normalized by the deepest-LOD AABB diagonal so the threshold is scale-invariant (5% means 10 cm on a 2 m box, 1 m on a 20 m room). Console summary now reports the distribution at the three buckets that correspond to the discussed promotion criteria: <1% mesh diag — glued to parent (X-brace flush against wall) <5% — attached, within typical thickness (post depth, brace) <15% — close but separable >=15% — floating, promotion candidate Plus a combined verdict line using the proposed default thresholds promote IF (θ > 60° OR ratio > 1.5 OR perpNorm > 5%) so the operator can read the predicted promotion fraction directly from the summary. CSV gains one column `shellPerpDistanceNorm`. Per-LOD breakdown includes mean perp + <1%/<5% percentages. Still read-only diagnostic, no algorithm change. Re-run the menu item on the 4 models; the PROMOTION line is the go/no-go signal for threshold tuning before HierarchicalRepack.cs is written.
Phase A of the new inverse-hierarchical UV2 pipeline. Read-only — does
NOT write mesh.uv2 yet (that's InverseTransfer in PR-3). Produces the
per-LOD-per-face → LightingDomain assignment + the packed atlas layout
that InverseTransfer will later project into.
Pipeline:
1. Pick deepest LOD as base; pick LOD0..deepest-1 as fine layers.
2. ExtractShells on deepest LOD via union-find on face adjacency +
normal threshold ≤30° (matches probe v2/v3 + xatlas hard-edge
convention). Each shell = one base lighting domain with a 3D plane
(area-weighted centroid + dominant normal) + orthonormal basis.
3. For each fine-LOD face: probe v3 classification (K=10 nearest
shells, best-angle pick, three-criterion promotion rule with
defaults θ>60° | ratio>1.5 | perp>5% × meshDiagonal).
4. Cluster promoted faces per fine LOD via face-adjacency union-find
so the packer doesn't get thousands of single-tri charts.
5. Materialise LightingDomain[]: one per base shell + one per promoted
cluster, each with plane + basis + uv2Rect (filled by packer).
6. PackAtlasNaive — horizontal-strip packer (placeholder). Deliberately
not xatlas yet: PR-2 validates the data flow end-to-end without
touching the existing repack pipeline. PR-2.5 swaps in xatlas via
the XatlasNative wrapper, contract unchanged.
Public surface (all in HierarchicalRepack):
- Options (with Default factory matching probe v3 defaults)
- LightingDomain (rect + plane + basis + diagnostic counts)
- Result (domains[] + faceToDomain[lod][face] + atlas size + error)
- Build(LODGroup, Options) → Result
Editor entry: Mesh Lab → Diag → Hierarchical Atlas Dry-Run
- Runs Build on the selected LODGroup
- Logs domains count, atlas size, per-LOD assignment audit,
top-K largest domains
- Writes hierrepack_<ts>_<lgName>.csv with one row per domain
(rect + plane + basis extents) for visual inspection / pandas
Single-renderer-per-LOD only in PR-2 (multi-renderer LODs warn and pick
first). Multi-renderer support deferred to PR-3 or later.
No algorithm coupling with existing pipeline (XatlasRepack,
GroupedShellTransfer untouched). HierDiag's local shell extraction
duplicated here deliberately to keep PR-2 a single file; refactor into
HierarchicalShellExtractor is a separate follow-up.
Acceptance for PR-2 (visual check):
- Atlas dimensions match expected ~1024×N (N grows with promotion)
- Per-LOD audit shows ~85-95% faces → base shells, ~5-15% → promoted
(matches probe v3 predictions)
- Top-K largest domains include the obvious LOD3 sides (front/back/
top/bottom panels of box-like meshes) not random tiny shells
Three fixes to HierarchicalRepack on top of PR-2:
1. BuildCanonicalIndices: dedup mesh.vertices by quantized world-space
position (cell = meshDiag × 1e-5) before adjacency. Unity splits
vertices on UV/normal seams, so two physically adjacent triangles
often had no shared edge under raw vertex indices and ended up in
separate single-tri shells (Carousel: 308 base shells for 1046
deepest faces; Wooden_Box_Long: 100 base shells for 196 faces).
2. Degenerate-tri filter: faces with cross-product magnitude < 1e-12
keep area=0 and are excluded from edge construction + shell
formation. Their faceToDomain stays -1 so they consume no atlas
slot. Wooden_Box_Long top-5 domains were degenerate triangles
filling 30%+ of atlas footprint — now gone.
3. Shell-level classification (Overlay / Promote / Skip): each fine
LOD now extracts its own shells, then classifies whole shells
instead of individual faces.
Overlay — fine shell normal within overlayAngleDeg of parent
base shell, lies in parent's plane (perpNorm), and
fits within parent's planar extent (with slack) →
reuses parent's atlas rect. Wall+sign, box+decal.
Skip — area below skipAreaFrac × totalDeepArea AND face
count ≤ skipMaxFaceCount → faceToDomain = -1, no
atlas slot. Handles, fasteners, geometric noise.
Promote — own direction / non-trivial area → own atlas domain.
Replaces the per-face Promote/Keep logic (which produced 415
promoted clusters on Carousel by treating each fine face in
isolation) with one decision per fine shell. Result: dramatically
fewer atlas slots, and the overlay path lets details inherit the
parent surface's lightmap directly.
New Options thresholds (defaults):
overlayAngleDeg = 25 (stricter than promoteAngleDeg=60)
overlayPerpNorm = 0.02 (tighter than promotePerpNorm=0.05)
overlayExtentSlack = 0.10 (10% overhang allowed)
skipAreaFrac = 0.001 (0.1% of total deepest area)
skipMaxFaceCount = 4
New Result counters: overlaidFineFaces, skippedFineFaces,
degenerateFineFaces. Dry-run dialog + console summary report all
three so the next probe pass can be evaluated against the same
Carousel / Wooden_Box_Long / Gazebo / Playground sweep.
PR-2.6 (next) swaps the naive strip packer for xatlas via the
existing XatlasNative wrapper.
BuildCanonicalIndices was returning a per-vertex canonical map (length = mesh.vertices.Length), but ExtractShells indexed it as canonicalTris[f*3+k] expecting per-triangle-corner layout (length = mesh.triangles.Length). On any mesh where tris.Length > vertices.Length (i.e. almost every mesh with shared vertices — a cube has 8 verts but 36 triangle indices) this threw IndexOutOfRangeException. Fix: rewrite tris through the canonical map and return the rewritten array directly. Callers now get an array they can index as before.
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 888ed22ca7
ℹ️ 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".
BuildCanonicalIndices packed three quantized world-coord components into a single long with 21 bits each (kx & 0x1FFFFF). For a mesh with diagonal 1m the cell size is 1e-5m, so any vertex with |world coord| > ~10m wraps around 2²¹ and collides with an unrelated vertex. Streamed or world-offset content (a 1m prop placed at world position 50,0,0) would canonicalize distant verts to the same cell ID, falsely fusing unrelated faces into the same shell and corrupting the per-shell classification. Fix: swap the packed long key for a (long, long, long) ValueTuple — no lossy truncation, dictionary handles equality + hashing natively.
GroupedShellTransfer.cs (P2): uv2 duplicate-pair hash now sorts the quantized UV positions lexicographically before feeding them into FNV. Previously the hash was computed in vertexIndices iteration order, and vertexIndices was populated from HashSet<int> upstream, so two shells with the same UV2 layout could hash differently purely from hash-set iteration order and miss the duplicate count. HierarchicalDiag.cs (P2): probe now canonicalizes vertex indices via position-grid dedup (cell = meshDiag × 1e-5) before building shell adjacency, mirroring the same step in HierarchicalRepack (PR-2.5). Without this the probe's shell count diverged from the repack pipeline's once PR-2.5 added dedup there, breaking the GO/STOP signal. LightmapTransferTool.cs (P1): multi-case sweep cleanup guard now uses Transform.IsChildOf instead of GameObject equality, so the temp-mesh cleanup also runs when the resolved LODGroup sits on a descendant of the spawned prefab root (a common hierarchy layout). The equality check missed that case, leaking the last cell's repackedMesh / transferredMesh clones into the editor process across multi-case runs. LightmapTransferTool.cs (P2): sweep cell metadata (CellConfig.internal Oversample) now stores the clamped value (max(1, raw)) that actually ran, matching ctx.InternalOversample. Storing the raw suite input made summary/winner artefacts misrepresent the run configuration when a suite contained 0 or negative entries.
Previously DryRun() only looked at Selection.activeGameObject and bailed unless exactly one scene object resolved a LODGroup ancestor. Now CollectSelectedLodGroups() iterates Selection.gameObjects and resolves each: - Scene GameObject → GetComponentInParent<LODGroup> - Project-window prefab asset → LoadAssetAtPath + GetComponentInChildren Each unique LODGroup gets its own Build + CSV in BenchmarkReports/. A cancellable progress bar shows progress on multi-select; the final dialog reports per-model success/failure with per-model CSV paths. Read-only — no mesh/prefab mutation, no Undo group needed.
A leftover '}' between CollectSelectedLodGroups() and LogDryRunSummary() closed the class scope prematurely, so LogDryRunSummary, WriteDryRunReport and Sanitize ended up outside the class. csc reported the unbalance at the namespace's final brace (CS1022).
UvShell.vertexIndices is HashSet<int>, not int[] — the capacity hint in the sort-then-hash refactor must use .Count.
The K-nearest-by-centroid prefilter was inherited from probe v3's per-FACE classifier and made sense there (a single triangle picks its parent from local candidates). For SHELL-level matching it produces a pathological failure: a small detail on the edge of a large flat surface (e.g. a 0.05m roof-border ornament on a 2m gazebo roof) has the roof's centroid 2m away, while a dozen tiny neighbouring shells sit 0.1m away. K=10 then samples only the close clutter — the roof never enters bestAngle computation — so overlay can never fire and the detail gets promoted instead. Confirmed on Gazebo via offline FBX parse: LOD2 has two huge roof shells (12 faces, area=25 each, normal=±Y, extent=2.13×0.80) plus 136 small decoration shells. LOD0's same roof shells are 144 faces with extent 2.30×0.80 — easily inside the 10% slack — but per-fine-shell classification only saw the clutter and promoted those big roof faces. Fix: full O(N base × M fine) scan for min-angle parent. Worst test case is Carousel ~270 × ~600 = 160k float dots, microseconds. opts.shellSearchK is left in the Options struct for the per-face probe but is no longer read by HierarchicalRepack. Expected effect on baseline (PR-2.5 measured): Gazebo overlay 17% → ~40-50% Carousel overlay 21% → ~35-45% Wooden overlay 21% → ~30-40% Playground overlay 37% → ~50-55%
…mpotency) Each full-pipeline run mutated e.originalMesh in place (meshopt dedup → UV weld → sym-split) and never reset it. The per-stage clone guard only fires when originalMesh == fbxMesh, so on the SECOND run originalMesh was already the prior run's mutated working copy: meshopt re-deduped, weld re-merged, and sym-split re-cut an already-cut shell. Identical settings therefore produced a different (and degrading) result on every re-run — e.g. CafeTable_Leg went 466→434→377→480 on run 1, then started from 480 on run 2 with a different fold count and 125% atlas utilization. ResetWorkingMeshesToFbx() rewinds every entry's originalMesh back to the imported fbxMesh, destroys the stale working clone, drops derived repacked/transferred meshes and clears the per-step flags + caches before stage 1. A full-pipeline run is now a pure function of (fbxMesh, settings). Also makes each benchmark-sweep cell independent — previously cells accumulated mutations across the matrix.
DetectFoldCount looks at 3D rotational symmetry and happily reports N=11 for a cylinder that has been cleanly unwrapped side-by-side in UV — at which point ApplyNFoldSplit shreds the chart into 11 sawtooth sectors even though xatlas can already pack the original chart as-is. This is the visible "WELD breaks the unwrap" regression: weld correctly fuses the false seam, then SymSplit cuts the now-continuous ring. SplitWithParams (the prescribed-by-source-LOD path) already gates the N-fold branch on HasUv0Overlap (cs:322, 346) — only stacked instances that xatlas literally can't pack get cut. The primary Split path was missing the same gate, hence the asymmetry. Apply the same condition here: when N>=3 is detected but the shell's UV centroids don't share grid cells, demote to N=1 and let the chart pass through unchanged (falls through to binary detection, which is its own thing).
…alse-positive) The HasUv0Overlap-based gate (cbefdb6) didn't fire on CafeTable_Leg: a clean cylinder rectangle wrap with ~125 thin triangles trivially puts multiple face centroids in the same 0.01×0.01 UV grid cell, so the spatial-hash density check trips even on perfectly non-overlapping unwraps. Log confirms the gate-skip verbose line never printed; N-fold ran and produced the 11-sector sawtooth anyway. Replace the gate with UvCoverageRatio = sum(|tri UV area|) / bbox UV area. This is a pure ratio so it's robust to mesh scale and triangle count: - clean rectangle wrap → ~1.0 - irregular non-overlapping chart → < 1.0 - N stacked instances → ~N Threshold 1.5 cleanly separates the cases. HasUv0Overlap stays where it is (still used by SplitWithParams's descriptor-matching loops where the asymmetry doesn't bite).
19dd113 added UvCoverageRatio but the gate still didn't fire on CafeTable_Leg — the Verbose "skipping cut" line never appeared in the log, which means coverage was actually >= 1.5 (or we couldn't see why). Two issues compounded: 1) The bug log shows N=11 detected with only 5/50 = 10% votes — a very weak rotational-symmetry signal. A real N-fold-stacked chart would see most sampled faces voting; 10% is almost always a sparse coincidence on a non-stacked mesh. Vote ratio is a free signal that DetectFoldCount already has and was throwing away. 2) The gate was silent on >= 1.5 cases, so we couldn't tell whether coverage was right at the threshold or way above it. Plumb bestVotes/sampleCount out of DetectFoldCount as voteRatio and require BOTH coverage>=1.5 AND voteRatio>=0.30 for the N-fold cut to run. Either signal failing demotes to N=1 (falls through to binary detection, unchanged). Log both values + the gate decision at Info level so we can see what's happening at next bench run.
Walks each (li_proxy, li_proxy-1) transition starting from the deepest
LOD. Poisson-samples the deeper LOD's surface tagged with the deeper
LOD's shell id (not UV-derived — Stage D doesn't care about UV yet),
closest-point-projects the samples onto the finer LOD's geometry, and
tallies per finer SHELL which deeper shell contributed the most hits.
Per finer shell:
matchedFrac = bestProxyHits / totalShellHits
totalHits >= 4 AND matchedFrac >= 0.5 AND parent group resolves
→ inherit the parent's groupId (lighting domain spans LOD levels)
otherwise
→ start a fresh group with canonicalLod = finer LOD (the shell is
detail that doesn't exist on the deeper LOD — windows, fasteners,
trim — and becomes its own canonical chart for Stage E)
Membership only — perLodShellToGroup fills in, r.groups grows, no UV is
written. Stage E will pick each group's canonical chart and pack the
final atlas.
Diagnostic PNGs lod{N}_groups.png — same iso view as lod{N}_shells.png
but coloured by groupId. LabelColor is a pure hash of the id so a group
that spans multiple LODs gets the SAME colour on every PNG, which is the
visual cue that the cascade converged.
Stage D's join/new decision was governed by two hardcoded consts
(kMatchFrac=0.5, kMinHits=4). Promote them to Options.cascadeMatchFrac /
cascadeMinHits (Default unchanged: 0.5 / 4, clamped to [0,1] / >=1) and
add a comparison sweep so the knee can be found per asset class.
BuildStageDSweep(lg, baseOpts, matchFracGrid, minHitsGrid, outputDir):
rebuilds the LODGroup once per grid cell on a fresh Result (zero
cross-cell contamination — the cascade-independent stages re-run per
cell, which is fine for an opt-in diagnostic on a small grid) and emits
• lod{N}_groups_mf{F}_mh{H}.png — per-cell group iso-view (LabelColor
hashes groupId, so a domain that spans LODs keeps one colour across
every PNG; that consistency is the visual signal the cascade
converged)
• stage_d_sweep.csv — one row per LOD transition per cell:
matchFrac,minHits,groupCount,seedGroups,liProxy,liFine,samples,hits,
missed,joined,fresh,reused
No auto-winner: Stage E (which would expose a lightmap-defect scalar)
isn't built yet, so there's nothing objective to optimise. The operator
compares the PNG grid + join/new ratios by eye. r.cascadeStats persists
the per-transition counters that were previously log-only.
Wiring: new BenchTechniques.stageDSweep flag (default off) +
cascadeMatchFracVariants {0.35,0.5,0.65} / cascadeMinHitsVariants
{2,4,8}. ExecBenchmark runs it after the hierarchicalRepack dry-run into
the same hier/ dir; the "all techniques disabled" guard and the
help/summary text learn about it. Build stays non-destructive (clones
for xatlas, read-only mesh access) so repeated cell rebuilds never touch
scene assets.
Sweep 2026-06-03 (4 cases × 9 cells) showed that varying matchFrac/minHits barely affects domain quality — the visual 3×3 grid is near-identical on major surfaces — and that group-count explosion (Carousel 269→922, Playground 318→1664) comes from Stage C micro-shells (thin trim, wire, fasteners) being misrouted into fresh groups by the minHits gate, not from bad matching. skipAreaFrac/skipMaxFaceCount were declared in Options.Default but never actually wired into Stage C/D, so micro-shells had no floor. Stage D voting loop now adds a tiny-shell branch: a finer shell with totalArea ≤ opts.skipAreaFrac × totalFineArea AND faceCount ≤ opts.skipMaxFaceCount is force-joined to its modal proxy parent even when matchedFrac/minHits would fail it, provided the parent group resolves. Tiny shells with no proxy match at all (bestProxy < 0 — detail the deeper LOD genuinely lacks) still open a fresh group but are tracked separately as tinyOrphan so the next sweep can size them. CascadeStat gains tinyJoined + tinyOrphan; stage_d_sweep.csv adds two columns; Stage D log line prints them; the existing skipAreaFrac/ skipMaxFaceCount doc comments are rewritten (the old text referenced a "promote cluster" stage that was deleted in the legacy purge). EXPERIMENTS.md gets a 2026-06-03 entry capturing the sweep findings, the change, and the open questions — most importantly that group-count is a misleading optimisation target until Stage E exposes a lightmap-defect scalar, so the sweep stays comparison-only.
The first sweep run wrote lod{N}_groups_mf{F}_mh{H}.png for every cell ×
every LOD (~36 PNGs/case on a 3×3 grid) — but the run itself showed they
are near-identical across cells on the major surfaces (the only variation
is sub-pixel trim), so they were just noise next to the real signal:
stage_d_sweep.csv plus the canonical lod{N}_groups.png the
hierarchicalRepack technique already writes at default thresholds.
BuildStageDSweep gains an emitPerCellPngs parameter (default false); the
per-cell WritePerLodGroupsPngs call is now gated on it. BenchTechniques
gains stageDSweepEmitPngs (default false) wired through ExecBenchmark, so
a deep visual dive is still one toggle away. The summary log line states
whether PNGs were emitted or suppressed.
Second sweep (bench_2026-06-03_01-02-41-045) with tiny-merge active
across the same 4 cases × 9 cells. Key results captured:
- tiny-merge works: tinyJoined absorbs the minHits penalty (59-409
shells/case that would otherwise be fresh); groupCount dropped vs the
pre-merge sweep (Carousel 922→818, Playground 1664→1255, Gazebo
281→222, WoodenBox 171→105).
- Decisive finding: 73-98% of all remaining fresh groups are tinyOrphan
(tiny shells with bestProxy < 0 — the deeper LOD genuinely lacks the
geometry, so tiny-merge can't force-join them). tinyOrphan is
threshold-invariant by construction (exactly one distinct value across
all 9 cells per case: Gazebo 19, Carousel 378, Playground 83,
WoodenBox 86), so no mf/mh tuning can touch the dominant explosion
source. WoodenBox is the cleanest case: seed=6, final=105, 97 of 99
fresh are orphans.
Conclusion logged: the topological-neighbour fallback previously
deferred ("defer until the data shows it's needed") is now data-
justified as the next step — merge each tinyOrphan into its
largest-shared-boundary neighbour on the same finer LOD instead of
opening a fresh lighting domain.
First slice of Stage E — the packed shared domain atlas that turns the cascade's 3D grouping into a real 2D layout (and unblocks the objective overlap/density metric the sweep analysis showed we lack). PackDomainCharts: for each lighting-domain group, take its canonical (deepest-member) shell, project every face corner onto the shell's own plane (basisU/basisV + half-extents from Stage C → local [0,1]), and feed the whole set to xatlas as a UV mesh with faceMaterial = groupId so each group becomes its own chart. ComputeCharts + PackCharts lay the charts out without overlap; we read back the placed UV (already normalised [0,1] in this native build, confirmed against proxy_uv2_auto.png) and each group's chart bounding rect. Output is the SHARED layout: Result.domainAtlasRects[groupId] is where that lighting domain lives in the atlas — slice E2 will map every LOD's member shells into their group's rect so the domain occupies the same region across LODs. No per-LOD UV2 / mesh writing here. domainAtlasUv / domainAtlasTris back a new domains_atlas.png diagnostic, wired into BuildAndWriteForCase next to the existing Stage C/D PNGs. Sequence (AddUvMesh → ComputeCharts → PackCharts) mirrors XatlasRepack.RepackSingle, which relies on faceMaterial chart boundaries and preserves the supplied UVs. Note: Unity compile not run (no toolchain in this environment); verified statically against the xatlas/Shell3D/Options signatures and brace balance. Benchmark run + visual check of domains_atlas.png is the next manual step in Unity.
…ng found bench_2026-06-03_01-48-52-098 (4 cases). domains_atlas.png generated for all cases; Stage E ran. Works: packing correct on all 4 (UV in [0,1], atlas well-filled, no gross inter-chart overlap); big lighting domains are clean rectangular charts; orphan crumbs are individually tiny in atlas area, confirming the tinyOrphan-fallback deferral was right. Found: single-plane projection folds wrap-around shells (cylinders, rings, tubes, arcs — Carousel rim, Gazebo balusters, Playground slide tubes). Visible as sine-wave / dense-stripe / bowtie / rosette charts with self-overlapping UV. Root cause: dominantNormal is the normalised area-weighted face-normal sum, which cancels toward zero on wrap-around shells, yielding a garbage projection basis. Small in count/area (thin curved trim, often the same tinyOrphans) but a clear known-bad. Cheap detection metric: coherence = |accumNormal| / totalArea (~1 flat, ->0 wrap-around).
The E1 atlas showed single-plane projection folds wrap-around shells (cylinders, rings, tubes, arcs): their area-weighted dominantNormal cancels to ~zero, so the projection basis is garbage and the chart folds onto itself (sine-wave / bowtie / rosette charts with self-overlapping UV — visible on the Carousel rim, Gazebo balusters, Playground tubes). PackDomainCharts now emits each canonical shell's faces using the mesh's authored UV0 as the chart UV — a real unwrap that stitches the shell without folding — and still stitches per lighting domain via faceMaterial = groupId so each group packs as one chart region. Planar projection is kept only as the fallback for shells whose mesh carries no UV0. UV0 is cached per LOD alongside worldVerts/rawTris.
… rect
Previous Stage E (E1 + the UV0 "stitch" attempt) only packed each
group's canonical shell and argued over its parameterisation — which
ignored the whole point of the cascade: cross-LOD consistency. A group's
canonical shell (its deepest member — the one that matched nothing
deeper, i.e. the repacked seed) owns an atlas rect; EVERY other member,
on every LOD, must land in that SAME rect so one lightmap bake is valid
across all LODs. Unmatched shells are the canonical of their own fresh
group and use their repacked slot.
- Reverted the UV0 detour in PackDomainCharts: canonical charts are
planar again. With rotateCharts:0 the placed rect is the input [0,1]
box uniformly scaled/translated, so the placement is reproducible by a
plain [0,1]->rect map and members align with the canonical.
- New BuildCascadedUv2: every shell on every LOD projects its verts onto
its group's canonical plane (basis/centroid/extent) -> local [0,1] ->
into domainAtlasRects[gid]. Writes finalUv2/finalTris/
finalSourceVertexIdx per LOD (one output vert per face corner; Stage F
copies attributes from the source vertex each points at).
- lod{N}_final_uv2.png renders each LOD's cascaded uv2 so the same domain
occupying the same atlas region across LODs is visible.
Folding of curved canonicals now produces a distorted but cross-LOD
CONSISTENT projection (canonical and members fold identically); a
barycentric pull on the canonical mesh is a later quality improvement,
not a cascade blocker.
Unity compile not run (no toolchain here); verified statically.
bench_2026-06-03_02-59-26-442. lod{N}_final_uv2.png generated for every
LOD of every case. WoodenBox LOD3->LOD2->LOD0: the big lighting domains
(floor, walls) occupy the SAME atlas rect on every LOD — LOD3 is ~6
charts in specific rects, LOD2 keeps those rects and subdivides, LOD0
keeps them and adds hundreds of trim charts in the remaining space. The
domain stays anchored across LODs, so one lightmap bake is valid
everywhere — the cascade's goal.
Remaining known defect (not a blocker): curved/degenerate canonical
shells still fold under planar projection ("Union Jack" striped charts
with internal UV overlap), consistent across LODs but distorted within
the chart. Fix candidates: barycentric pull on the canonical mesh, or
splitting curved shells into developable patches.
…form texel The per-shell [0,1] extent-normalisation was the real bug: it (a) stretched non-square shells (a long thin strip forced into a square) and (b) destroyed texel density — a 2m wall and a 2cm screw both mapped to [0,1], so the screw got ~100x the texels per world unit. Fatal for lightmapping. Recipe (per user): classical -> align uv -> identical texel. Implemented: - Real proportions: the canonical shell projects onto its plane in WORLD UNITS (inU = dot(d,basisU), inV = dot(d,basisV)), no normalise-to-square. - Identical texel density: xatlas packs at a FIXED texelsPerUnit (atlasRes * sqrt(packEff/totalCanonArea)) instead of auto-fit, so every chart has the same texels/unit. - Alignment: least-squares-fit the affine inU->atlasU / inV->atlasV from the canonical's placed UV (r.domainPlacements[groupId]). The canonical reproduces its own placement and every finer member of the group reuses the SAME affine, so all members land in the same atlas region at the same density. Replaces the [0,1]->rect map. BuildCascadedUv2 now applies the group affine to each shell's canonical-plane coordinate; no extent normalisation, no clamping. Remaining: "classical" is still planar projection here — exact for flat shells (a plane unwraps trivially) but curved canonicals (rim/tubes) still fold. Full fix is a real xatlas unwrap of curved shells (Stage B already computes per-LOD classical UV). Distortion + texel density are now correct for flat domains. Unity compile not run (no toolchain); verified statically.
Even after switching to real proportions, Stage E was still DERIVING UV
from scratch via planar projection (dot(d,basis)) — throwing away the
shell's own unwrap and folding curved shells. The shell's authored UV0 is
its real parameterisation; preserve it and just place it.
PackDomainCharts: the xatlas input is now the canonical shell's PRESERVED
UV0 — only recentred (minus its island centroid) and uniformly scaled by
S = sqrt(area3D / areaUV0), which normalises texel density (UV-area ->
3D-area) without touching the shape. Fixed texelsPerUnit -> identical
texel density across domains. The least-squares affine is fit on the
scaled UV0 -> atlas; domainPlacements[gid] now carries {uvc, scale,
su,ou,sv,ov}.
BuildCascadedUv2: every shell on every LOD takes its OWN authored UV0 and
applies its group's placement: in = (uv0 - uvc)*scale; uv = affine(in).
Members of a domain share UV0 layout across LODs, so the same UV0 coord
maps to the same atlas texel on every LOD -> matched shells co-locate at
uniform density with the real unwrap intact. Planar dot(d,basis) is gone
from Stage E entirely.
Face emit loops validate all three corners against uv0 length before
emitting, so a clipped triangle never desyncs faceMaterial from the index
buffer. Assumes LODs share a domain's UV0 layout (standard for LOD
chains); if not, matched-shell alignment will show on lod{N}_final_uv2.png.
Unity compile not run (no toolchain); verified statically.
The UV0 face-validation trio fa/fb/fc shadowed the method-level int fc = faceMatList.Count, which CS0136 forbids. Renamed to e0/e1/e2.
bench_2026-06-03_08-41-51-481 (compiles after the CS0136 fix). Across 4 cases: - Folding gone: domains_atlas.png charts are real UV0 islands with true proportions (long thin = gazebo slats, fans = carousel canopy wedges, squares = panels). No more sine-wave / bowtie / Union-Jack charts. - Cross-LOD consistency holds: WoodenBox LOD3 = 6 big domains in a 2x3 block lower-left; LOD0 = the SAME squares in the SAME spots plus thin trim around them. One domain -> one atlas region on every LOD. - Shell unwrap preserved (UV0, not re-projected); texel density uniform (S-normalise + fixed texelsPerUnit). Recipe "preserve shell -> project -> align -> identical texel" is implemented and visually verified. The shared-UV0-layout assumption held on the test suite. Minor non-blockers: ~50% pack fill (packEff=0.5); very thin trim strips (real geometry); Carousel dense (hundreds of thin details).
… drop faces Two reliability fixes for the cascade's final-mesh path: 1. PackDomainCharts: a canonical chart that is a perfectly straight axis-aligned strip in UV0 has zero variance on one axis, which made the per-axis LSQ denominator vanish and the whole group's placement invalid. Borrow the resolved axis's magnitude for the degenerate axis (xatlas applies a uniform scale, rotateCharts:0); when both axes collapse, fall back to the designed texelsPerUnit/resolution scale anchored at the mean placed UV. 2. BuildCascadedUv2: shells whose group has no valid placement were silently skipped, which deleted real geometry from the final meshes (holes after Apply). Emit them with uv2 (0,0) instead — a bad bake on those faces, never a missing triangle — and warn with per-LOD unplaced shell/face counts.
…ss-LOD containment
ComputeStageEMetrics rasterises each LOD's final cascaded UV2 at atlas
resolution (texel-centre ownership) and measures, per LOD:
- overlapTexels / overlapShellPairs: texels claimed by 2+ triangles
that are neither the same face nor same-shell seam-adjacent. Cross-
shell conflicts always count — UV0 mirror-reuse between shells is
exactly the defect to catch; touching islands only contribute ~1px
border noise vs full-area genuine overlaps.
- invertedFaces / degenUvFaces / oobVerts: winding flips, zero-area
UV islands, verts outside [0,1].
- tpuMean/P1/P99/Spread: area-weighted texels-per-world-unit spread —
the preserve-UV0 + fixed texelsPerUnit recipe should hold ~1x.
- xLodContainedPct / misalignedGroups: cross-LOD containment — texels
of groups whose canonical lives on another LOD must land inside the
canonical's 3x3-dilated footprint. This measures the cascade's
contract (one bake valid across LODs) instead of assuming the
shared-UV0-layout precondition holds on a given asset.
Stage E2 now records per-emitted-face shell/group ids to back the
seam-vs-overlap distinction. Bench writes stage_e_metrics.csv +
lod{N}_overlap.png (grey covered, red overlap); the Apply menu dialog
reports the headline (overlap px, unplaced faces, misaligned domains).
stage_d_sweep.csv gains cell-level e3OverlapTexels / e3OverlapPct / e3TpuSpreadMax / e3XLodMinPct / e3MisalignedGroups / e3UnplacedFaces columns (repeated on each transition row, like groupCount). The sweep finally has an objective lightmap-defect scalar to minimise instead of comparative join/new ratios.
…tions Multi-agent audit (4/9 finders completed before spend limit; verify pass did not run). Cross-cutting themes confirmed by direct spot-check: silent zero-UV2 fallbacks reported as success, divergent deepest-LOD pickers, non-scaling absolute thresholds, unused-BVH brute force, stale counts after post-passes. Findings marked VERIFIED were checked against code; the rest are reviewer-proposed and need confirmation.
Codebase-specific plan to isolate where transfer corrupts UV2: complexity ladder F0-F6 (cube -> symmetric -> curved -> tiling -> 2-LOD -> double-sided -> full suite), pass/fail thresholds from stage_e_metrics.csv (overlap, tpuSpread, xLodContainedPct, unplaced/inverted), per-stage A->F artifact gates, audit-driven regression tests (silent zero-UV2, scale invariance, double-sided merge), threshold sweep now scored by E3 scalar, triage tree, and a Definition of Done.
- Added local agent/session state and generated experiment output directories to .gitignore. - Updated .npmignore to exclude Native~ directory instead of Native. - Modified AGENTS.md, CLAUDE.md, and README.md to reflect changes from 'Native/' to 'Native~/'. - Adjusted CODEOWNERS and PULL_REQUEST_TEMPLATE.md for the new directory structure. - Updated version-bump.yml to monitor changes in Native~ directory. - Renamed Tools/gen.bat to Tools~/gen.bat for consistency with the new structure. - Removed Native.meta file as part of the restructuring.
WalkthroughИерархический UV2-пайплайн получил стадии построения, диагностики и применения к LODGroup. Benchmark расширен визуальными метриками, sweep-осями, provenance-архивами и HTML/CSV/JSON-отчётами. Также обновлены package-пути, native-сборка, shader-имена и FBX define. ChangesUV2 transfer и hierarchical pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Editor/BenchmarkRecorder.cs (1)
330-346: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winФиксированный
uv2_pngвнутри общего каталога перезаписывает PNG между ячейками sweep.В sweep
BenchmarkRecorder.OutputDirectoryOverrideвыставлен в один и тот жеsweepDirдля всех ячеек (LightmapTransferTool.ExecSweep), поэтому каждая сессия пишет вsweepDir/uv2_png, а имя файла (<renderer>_LOD{n}_uv2.png) от ячейки не зависит → каждая следующая ячейка затирает предыдущую.BenchmarkSweep.BuildThumbsCellищет ровно этот каталог, так что вindex.htmlвсе строки покажут миниатюры последней ячейки, и сравнение конфигураций по картинкам перестаёт работать. ПрежнийfileBase + "_png"такой коллизии не давал; проблему длинного пути решает короткий уникальный суффикс, а не общий каталог.🐛 Вариант: короткий, но уникальный подкаталог на сессию
- int pngCount = 0; - string pngDir = Path.Combine(dir, "uv2_png"); - Directory.CreateDirectory(pngDir); + int pngCount = 0; + // Короткий, но уникальный на сессию каталог: общий "uv2_png" + // затирается следующей ячейкой sweep, а полный fileBase + // упирается в MAX_PATH. + string pngDir = Path.Combine(dir, "png_" + stamp); + Directory.CreateDirectory(pngDir);Соответствующий fallback-поиск нужно добавить и в
BenchmarkSweep.BuildThumbsCell.🤖 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/BenchmarkRecorder.cs` around lines 330 - 346, Update the PNG output directory logic in the recorder method containing the shown records loop to use a short but session-unique subdirectory instead of the shared uv2_png directory, preserving the shorter-path constraint and preventing sweep cells from overwriting one another. Update BenchmarkSweep.BuildThumbsCell to locate thumbnails using the same unique directory, including the corresponding fallback search, while retaining compatibility with existing output directories where required.
🧹 Nitpick comments (8)
Documentation~/HIERARCHICAL_CASCADE_PLAN.md (1)
20-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueУкажите язык для fenced-блока.
Блок с
LightingDomainGroupбез языка — markdownlint MD040. Достаточно пометить его какcsharp(ниже в файле аналогичный блок уже помечен).🤖 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 `@Documentation`~/HIERARCHICAL_CASCADE_PLAN.md around lines 20 - 27, Mark the fenced code block containing LightingDomainGroup with the csharp language identifier, matching the language annotation used by the analogous block elsewhere in the document.Source: Linters/SAST tools
Editor/HierarchicalRepack.cs (2)
2748-2751: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
BuildFaceDataздесь вызывается только радиrt.Метод дополнительно трансформирует все вершины в мир и строит
canonicalTris(словарь на весь меш) — всё это сразу отбрасывается. Достаточноvar rt = mesh.triangles;. То же самое вPackDomainCharts.EnsureLodGeometry.🤖 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/HierarchicalRepack.cs` around lines 2748 - 2751, Replace the BuildFaceData call in the surrounding mesh-processing flow with direct retrieval of the mesh triangle indices via mesh.triangles, since only rt is used. Apply the same change in PackDomainCharts.EnsureLodGeometry, preserving the existing UV validation and subsequent processing.
2416-2434: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
worldVertsByLodбольше не используется.После перехода на preserve-UV0 (Stage E fix 2) планарная проекция убрана, и заполненный
wvнигде не читается —BuildFaceDataздесь вызывается только радиrawTris. Можно убрать поле и не платить за world-transform всех вершин каждого LOD.🤖 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/HierarchicalRepack.cs` around lines 2416 - 2434, Remove the unused worldVertsByLod array and stop capturing or assigning the wv output in EnsureLodGeometry. Update the BuildFaceData call to discard its world-vertex output while preserving rawTrisByLod and uv0ByLod population.Documentation~/TRANSFER_AUDIT_2026-07-18.md (1)
91-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueПункт T5 про
StageEMetricsуже закрыт кодом этого PR.В
Editor/HierarchicalRepack.csмутации локальной копии записываются обратно (r.stageEMetrics[li] = m;в конце основного цикла иr.stageEMetrics[li] = mm;после cross-LOD блока). Стоит пометить пункт как проверенный/закрытый, чтобы «Needs review» не тянуло за собой лишнюю итерацию аудита.🤖 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 `@Documentation`~/TRANSFER_AUDIT_2026-07-18.md around lines 91 - 93, Update the StageEMetrics audit entry in TRANSFER_AUDIT_2026-07-18.md to mark T5 as verified/closed rather than “Needs review,” noting that HierarchicalRepack writes the mutated struct back after both the main loop and cross-LOD block.Editor/HierarchicalDiag.cs (2)
298-312: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDoc-комментарий и сигнатура
ExtractShellsрассинхронизированы;vertexCountиshellOfне используются.
<paramref name="faceToShell"/>ссылается на параметр, которого в сигнатуре нет (компилятор выдаст CS1734 при генерации XML-документации), параметрvertexCountнигде не читается, а локальныйshellOfзаполняется (строка 382) и после этого не используется. Либо верните маппинг face→shell наружу (как вHierarchicalRepack.ExtractShells), либо уберите лишнее.♻️ Минимальная чистка сигнатуры и комментария
- /// face is returned in <paramref name="faceToShell"/>. /// <paramref name="canonicalTris"/> must reference position-deduplicated /// vertex IDs (see <see cref="BuildCanonicalTris"/>) so seam-split /// vertices in Unity meshes don't fragment a single physical surface. /// </summary> - static ShellData[] ExtractShells(FaceData[] faces, int[] canonicalTris, int vertexCount) + static ShellData[] ExtractShells(FaceData[] faces, int[] canonicalTris)И соответственно вызов на строке 152:
- var deepShells = ExtractShells(deepFaces, deepCanonical, deepMesh.vertexCount); + var deepShells = ExtractShells(deepFaces, deepCanonical);🤖 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/HierarchicalDiag.cs` around lines 298 - 312, Синхронизируйте `ExtractShells` с фактическим API: удалите из XML-комментария ссылку на отсутствующий `faceToShell`, уберите неиспользуемые `vertexCount` и локальный `shellOf`, а также обновите вызов `ExtractShells` на месте его использования, чтобы передавались только необходимые аргументы.
525-528: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueПерегрузка
WriteReport(lgName, records)не вызывается.
ProbeLodGroupвсегда идёт через трёхаргументную версию. Если она не нужна как публичная точка входа — удалите, чтобы не тянуть мёртвый код.🤖 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/HierarchicalDiag.cs` around lines 525 - 528, Remove the unused two-argument WriteReport(string lgName, List<FaceProbeRecord> records) overload in Editor/HierarchicalDiag.cs, since ProbeLodGroup always calls the three-argument overload and no standalone entry point requires this wrapper.Editor/Uv0Analyzer.cs (1)
800-852: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
bool[N,N]+ перебор всех пар шеллов квадратичны по числу шеллов.На плотных ассетах
UvShellExtractor.Extractлегко даёт тысячи шеллов: при N=20000 матрица занимает ~400 МБ, а вложенный цикл — 200 млн итераций, и всё это выполняется на каждый вызовUvEdgeWeld(то есть на каждый меш каждого LOD). Разреженное множество заблокированных пар и ранний выход по непересекающимся bbox дадут ту же семантику без квадратичной памяти.♻️ Вариант с разреженным множеством
- blockedShellPair = new bool[uvShells.Count, uvShells.Count]; + var blockedPairs = new HashSet<long>(); @@ - if (smallerArea > 0f && interArea / smallerArea >= kInstanceOverlapFrac) - { - blockedShellPair[a, b] = true; - blockedShellPair[b, a] = true; - blocked++; - } + if (smallerArea > 0f && interArea / smallerArea >= kInstanceOverlapFrac) + { + blockedPairs.Add(((long)a << 32) | (uint)b); + blocked++; + }Проверку в guard заменить на
blockedPairs.Contains(Key(min(sA,sB), max(sA,sB))). Дополнительно стоит отсортировать шеллы поboundsMin.xи прерывать внутренний цикл, когдаbb.boundsMin.x >= ba.boundsMax.x.🤖 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/Uv0Analyzer.cs` around lines 800 - 852, Replace the dense blockedShellPair bool[,] and all-pairs loop in the UvShellExtractor processing with a sparse set of blocked shell-index pairs. Sort shells by boundsMin.x, stop each inner scan when the next shell’s boundsMin.x reaches ba.boundsMax.x, and preserve the existing overlap-fraction test when recording pairs. Update downstream blocked-pair checks to use a normalized min/max pair key with Contains, preserving the current blocking semantics.Editor/SymmetrySplitShells.cs (1)
843-848: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueУдалите перегрузку
DetectFoldCountбезvoteRatio— во всём репозитории остался только вызов варианта сout float voteRatio, поэтому этот overload больше не нужен.🤖 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/SymmetrySplitShells.cs` around lines 843 - 848, Remove the `DetectFoldCount` overload that omits the `out float voteRatio` parameter. Keep the `DetectFoldCount` variant that returns `voteRatio` and preserve all existing callers using that signature.
🤖 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 `@Documentation`~/HIERARCHICAL_CASCADE_PLAN.md:
- Around line 45-55: Обновите статусы Stage C–F в таблице и раздел «Current
state» в HIERARCHICAL_CASCADE_PLAN.md, чтобы они отражали уже реализованные
методы ExtractPerLodShellsAndSeedGroups, CascadeGroupShells, PackDomainCharts,
BuildCascadedUv2, ComputeStageEMetrics и HierarchicalApply. Пометьте
соответствующие стадии как выполненные согласно текущему коду PR и удалите
устаревшее описание Stage C как следующего шага; не изменяйте технические
критерии или содержание незавершённых работ.
In `@Documentation`~/TRANSFER_BENCHMARK.md:
- Around line 178-184: Update the multi-case sweep documentation to use the
exact UI label from LightmapTransferTool.DrawSetupDebugSection: “Run Benchmark
({caseCount} cases)”, replacing “Run Multi-Case (N × M)”. Also add an explicit
language identifier to the matrix code block near the referenced section to
satisfy markdownlint MD040.
In `@Editor/BenchmarkSweep.cs`:
- Around line 909-916: Update the CSV clustering logic in the recovery flow of
Editor/BenchmarkSweep.cs so files are grouped only within the same containing
directory, not solely by modification-time gaps. Track each file’s directory and
force a cluster boundary when it changes, while preserving the existing
kRecoveryGapSeconds boundary and summary.csv/winner.json generation for each
valid cluster.
In `@Editor/HierarchicalApply.cs`:
- Around line 82-87: Update the WARNING condition in the e3Note construction so
it also triggers when overlapPx is non-zero, alongside unplaced or misaligned
defects. Keep the existing overlap, unplaced, and misaligned values in the
message and preserve the current clean-message behavior when all three defect
measures are zero.
In `@Editor/HierarchicalDiag.cs`:
- Around line 104-142: Синхронизируйте выбор deepest LOD в диагностике с логикой
HierarchicalRepack.Build: не фиксируйте deepestIdx на последнем уровне, а
найдите последний LOD с валидным MeshFilter.sharedMesh для соответствующей
группы и используйте его как fallback, чтобы группы с пустым последним LOD не
пропускались. Сохраните groupsSkipped только для групп без валидного меша на
всех уровнях.
- Around line 482-490: Update WriteReport so CSV serialization of faceAreaRatio
and shellAreaRatio uses the existing finite-number formatting helper Num(...)
instead of directly calling ToString("R", inv). Preserve numeric output for
finite ratios while emitting an empty field for float.PositiveInfinity or other
non-finite values.
- Around line 721-725: Update CsvField() to neutralize values beginning with
“=”, “+”, “-”, or “@” before applying CSV quoting, such as by prepending a safe
text prefix. Preserve existing handling for null/empty values and values
requiring comma, quote, or newline escaping, and ensure the protection applies
to every string passed through CsvField().
In `@Editor/HierarchicalRepack.cs`:
- Around line 2237-2256: Устраните полный перебор граней в
ProjectVertexToDeepMesh: постройте пространственный индекс (uniform grid или
BVH) для fine-LOD и используйте его для поиска ближайших кандидатов при проекции
samples. Создайте индекс один раз и переиспользуйте его в BuildStageDSweep и
ProjectProxySamplesOntoFineLods, сохранив текущую проверку distAbsThreshold и
подсчёт попаданий.
- Around line 2553-2562: Update the xatlas packing call in XatlasRepack to avoid
enabling brute-force unconditionally. Reuse the existing
ResolvePackBruteForce/cost-budget heuristic, or equivalent packedGroups-based
threshold, and pass the resulting decision to bruteForce so large multi-domain
assets remain bounded while small packs retain the current optimization.
- Around line 2159-2178: Update the LOD cascade loop around the liProxy
transition so missing shells, renderers, or meshes do not silently break parent
resolution: at minimum emit a Warn identifying the skipped liProxy→liFine
transition and its missing prerequisite, while preserving the existing continue
behavior unless a valid deeper-LOD fallback is implemented. Ensure skipped
transitions are distinguishable from genuine geometry mismatches.
- Around line 3188-3192: Add an explicit final-mesh cleanup path: update
BuildFinalMeshes documentation and introduce DisposeFinalMeshes(Result) in
Editor/HierarchicalRepack.cs to DestroyImmediate every non-null r.finalMeshes
entry when no longer needed; in Editor/HierarchicalApply.cs lines 121-145,
destroy clone before continuing when rs[0] or mf is null, while leaving clone ==
null unchanged.
- Around line 4-18: Update the file-header comment to match the current
implementation: state that Build() executes the implemented Stage 1, B, 2, 3, C,
D, E1, E2, and E3 phases, and remove the claim that Stages C–E are pending. Also
document that finalUv2 is populated by BuildCascadedUv2 rather than remaining
null until a future Stage E.
- Around line 2032-2049: Unify renderer selection with Build() by adding a
shared helper that returns the first renderer with a valid
MeshFilter.sharedMesh, plus a deepest-LOD helper based on it. Replace the
rs[0]-only checks and mesh extraction in the shown Stage C/D/E flow and in
CascadeGroupShells, PackDomainCharts, BuildCascadedUv2, and
ComputeStageEMetrics; use the helper’s renderer and mesh consistently so valid
entries such as renderers[1] are processed.
- Around line 2879-2890: Уменьшите пиковое потребление памяти в Stage E: не
храните растровые буферы `ownerFaceByLod` и `r.stageEOverlapPx` для всех LOD до
завершения метода. Ограничьте разрешение метрических растров безопасным
максимумом (например, 1024) и/или измените `WriteStageEOverlapPngs`, чтобы PNG
кодировались сразу после построения каждого растра и пиксели не сохранялись в
`Result`; сохраните необходимую cross-LOD проверку без длительного удержания
всех буферов.
- Around line 2789-2806: Добавьте в BuildFinalMeshes дедупликацию при
формировании triOut, uvOut и srcOut: используйте словарь по ключу (srcIdx,
quantized uv2), чтобы совпадающие углы переиспользовали существующий индекс
вершины, а не добавляли новую запись. Сохраняйте отдельные вершины при различии
исходного индекса или квантованного UV2, и добавляйте в triOut найденный либо
созданный индекс.
In `@Editor/SymmetrySplitShells.cs`:
- Around line 1209-1213: Remove the duplicate opening <summary> tag in the XML
documentation comment for the shell UV-area ratio, leaving one opening tag
paired with the existing closing tag.
In `@Editor/Tools/LightmapTransferTool.cs`:
- Around line 2249-2294: Update ResetWorkingMeshesToFbx to restore each entry’s
MeshFilter.sharedMesh to e.fbxMesh before destroying e.originalMesh, matching
ResetWorkingCopies and preventing destroyed meshes from remaining referenced.
Also clear accumulatedMatchHints alongside accumulatedOverlapHints so reset
state is independent of prior runs.
In `@Editor/UvtLog.cs`:
- Around line 28-30: Обновите логику чтения EnabledCategories для сохранённых
значений EditorPrefs по ключу MaskPrefKey: автоматически добавляйте в маску
новые неизвестные биты, включая TransferDiag, вместо применения сохранённой
старой маски без них. Не изменяйте поведение для уже известных категорий и
сохраните значение Category.All как дефолт при отсутствии ключа.
In `@Editor/XatlasRepack.cs`:
- Around line 140-144: Исправьте настройку blockAlign в xatlasPackCharts с
учётом internalOversample: не используйте фиксированное выравнивание 4×4 во
внутренних координатах как защиту блока BC/ETC. Реализуйте snap в
пользовательском разрешении либо пересчитайте размер блока в
internal-координатах через internalOversample, сохранив корректное выравнивание
границ после нормализации UV.
---
Outside diff comments:
In `@Editor/BenchmarkRecorder.cs`:
- Around line 330-346: Update the PNG output directory logic in the recorder
method containing the shown records loop to use a short but session-unique
subdirectory instead of the shared uv2_png directory, preserving the
shorter-path constraint and preventing sweep cells from overwriting one another.
Update BenchmarkSweep.BuildThumbsCell to locate thumbnails using the same unique
directory, including the corresponding fallback search, while retaining
compatibility with existing output directories where required.
---
Nitpick comments:
In `@Documentation`~/HIERARCHICAL_CASCADE_PLAN.md:
- Around line 20-27: Mark the fenced code block containing LightingDomainGroup
with the csharp language identifier, matching the language annotation used by
the analogous block elsewhere in the document.
In `@Documentation`~/TRANSFER_AUDIT_2026-07-18.md:
- Around line 91-93: Update the StageEMetrics audit entry in
TRANSFER_AUDIT_2026-07-18.md to mark T5 as verified/closed rather than “Needs
review,” noting that HierarchicalRepack writes the mutated struct back after
both the main loop and cross-LOD block.
In `@Editor/HierarchicalDiag.cs`:
- Around line 298-312: Синхронизируйте `ExtractShells` с фактическим API:
удалите из XML-комментария ссылку на отсутствующий `faceToShell`, уберите
неиспользуемые `vertexCount` и локальный `shellOf`, а также обновите вызов
`ExtractShells` на месте его использования, чтобы передавались только
необходимые аргументы.
- Around line 525-528: Remove the unused two-argument WriteReport(string lgName,
List<FaceProbeRecord> records) overload in Editor/HierarchicalDiag.cs, since
ProbeLodGroup always calls the three-argument overload and no standalone entry
point requires this wrapper.
In `@Editor/HierarchicalRepack.cs`:
- Around line 2748-2751: Replace the BuildFaceData call in the surrounding
mesh-processing flow with direct retrieval of the mesh triangle indices via
mesh.triangles, since only rt is used. Apply the same change in
PackDomainCharts.EnsureLodGeometry, preserving the existing UV validation and
subsequent processing.
- Around line 2416-2434: Remove the unused worldVertsByLod array and stop
capturing or assigning the wv output in EnsureLodGeometry. Update the
BuildFaceData call to discard its world-vertex output while preserving
rawTrisByLod and uv0ByLod population.
In `@Editor/SymmetrySplitShells.cs`:
- Around line 843-848: Remove the `DetectFoldCount` overload that omits the `out
float voteRatio` parameter. Keep the `DetectFoldCount` variant that returns
`voteRatio` and preserve all existing callers using that signature.
In `@Editor/Uv0Analyzer.cs`:
- Around line 800-852: Replace the dense blockedShellPair bool[,] and all-pairs
loop in the UvShellExtractor processing with a sparse set of blocked shell-index
pairs. Sort shells by boundsMin.x, stop each inner scan when the next shell’s
boundsMin.x reaches ba.boundsMax.x, and preserve the existing overlap-fraction
test when recording pairs. Update downstream blocked-pair checks to use a
normalized min/max pair key with Contains, preserving the current blocking
semantics.
🪄 Autofix (Beta)
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: 79d3afac-bda9-488f-895a-a526bd239843
⛔ Files ignored due to path filters (3)
Plugins/macOS/libxatlas-unity.dylibis excluded by!**/*.dylibPlugins/x86_64/libxatlas-unity.sois excluded by!**/*.soPlugins/x86_64/xatlas-unity.dllis excluded by!**/*.dll
📒 Files selected for processing (33)
.github/CODEOWNERS.github/PULL_REQUEST_TEMPLATE.md.github/workflows/version-bump.yml.gitignore.npmignoreAGENTS.mdCHANGELOG.mdCLAUDE.mdDocumentation~/EXPERIMENTS.mdDocumentation~/HIERARCHICAL_CASCADE_PLAN.mdDocumentation~/TRANSFER_AUDIT_2026-07-18.mdDocumentation~/TRANSFER_BENCHMARK.mdDocumentation~/TRANSFER_TEST_PLAN.mdEditor/BenchmarkRecorder.csEditor/BenchmarkSweep.csEditor/GroupedShellTransfer.csEditor/HierarchicalApply.csEditor/HierarchicalApply.cs.metaEditor/HierarchicalDiag.csEditor/HierarchicalDiag.cs.metaEditor/HierarchicalRepack.csEditor/HierarchicalRepack.cs.metaEditor/Settings/TestSuiteAsset.csEditor/SymmetrySplitShells.csEditor/Tools/LightmapTransferTool.csEditor/Uv0Analyzer.csEditor/UvtLog.csEditor/XatlasNative.csEditor/XatlasRepack.csNative.metaNative~/xatlas-unity-bridge.cppREADME.mdTools~/gen.bat
💤 Files with no reviewable changes (1)
- Native.meta
Comment on lines
+45
to
+55
| ## Stage map (v2) | ||
|
|
||
| | # | Stage | Что делает | Diagnostic | Pass criteria | Status | | ||
| |---|---|---|---|---|---| | ||
| | A | Poisson coverage | `GenerateProxySamples`: убран adaptive median filter | `proxy_samples.png` | Pink dots по ВСЕМ чартам | ✅ `3fb4c01` | | ||
| | B | Per-LOD classical unwrap | xatlas (sym-split+ARAP+pack) на каждый non-deepest LOD. Diagnostic + источник shell-форм для финал-пака. **Texel НЕ выравниваем тут — это забота финал-пака.** | `lodN_classical_uv2.png` | Каждый LOD пакуется чисто, без инверсий | ✅ `3bcfdfb` | | ||
| | — | **Legacy purge** | Удалён весь PR-2 single-proxy classifier + PR-3 single-proxy projector. -1042 строки. Build() остался только: Stage 1 (proxy unwrap variants) + Stage B (per-LOD classical) + Stage 2 (Poisson) + Stage 3 (sample→fineLOD). Apply menu graceful no-op до Stage E. | — | Brace balance 0, CI зелёный, бенч работает (без `final_uv2.png`) | ✅ `c9948e6` | | ||
| | **C** | **Per-LOD 3D shell extract + group seed** ⬅ **СЕЙЧАС** | См. ниже | См. ниже | См. ниже | ⬜ next | | ||
| | D | Cascade grouping (deep→fine) | Цикл li = deepest-1 … 0. На каждом шаге: Poisson на LOD[li+1] (immediate deeper), project на LOD[li], per-shell vote за deeper-shell → group. matched-fraction ≥ порог → join группы; иначе → новая группа. **Только membership, НЕ UV.** | `lodN_groups.png` (iso, faces по groupId — ОДИН цвет across LODs = один домен) | Соответствующие шелы разных LOD'ов = один цвет. Unmatched = новые цвета. Никакого noise-разброса | ⬜ | | ||
| | E | Final pack + per-LOD uv2 | 1) xatlas pack всех canonical-чартов (геометрия canonical членов) ОДНИМ вызовом, unified texelsPerUnit+padding+blockAlign. 2) Per group: построить target (worldVerts + placed uv2 canonical члена). 3) Finer члены: ortho-project на canonical target → barycentric → uv. 4) Seam-dup по groupId. Записать `finalUv2[li]`. | `lodN_final_uv2.png` (ВСЕ LOD в ОДНОМ атласе), `atlas.png` | Все LOD в общем атласе. Группа = один чарт. Никаких sentinel. Атлас покрывает весь контент | ⬜ | | ||
| | F | Apply + Bake | `BuildFinalMeshes`+`Apply` (готовы) на каскадный результат. Bake в Unity. | Apply menu, Ctrl+Z, bake | Bake совпадает между LOD0..LOD3 в shared domain | ⬜ | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Статусы стадий в плане расходятся с кодом этого же PR.
Таблица помечает Stage C как «⬜ next», а D/E/F как незавершённые, но Editor/HierarchicalRepack.cs в этом PR уже содержит ExtractPerLodShellsAndSeedGroups (Stage C), CascadeGroupShells (D), PackDomainCharts + BuildCascadedUv2 (E1/E2), ComputeStageEMetrics (E3) и HierarchicalApply (F). Обновите колонку Status и раздел «Current state», иначе документ вводит в заблуждение относительно того, что уже реализовано.
🤖 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 `@Documentation`~/HIERARCHICAL_CASCADE_PLAN.md around lines 45 - 55, Обновите
статусы Stage C–F в таблице и раздел «Current state» в
HIERARCHICAL_CASCADE_PLAN.md, чтобы они отражали уже реализованные методы
ExtractPerLodShellsAndSeedGroups, CascadeGroupShells, PackDomainCharts,
BuildCascadedUv2, ComputeStageEMetrics и HierarchicalApply. Пометьте
соответствующие стадии как выполненные согласно текущему коду PR и удалите
устаревшее описание Stage C как следующего шага; не изменяйте технические
критерии или содержание незавершённых работ.
Comment on lines
+178
to
+184
| ### Multi-case sweep (all `TestSuiteAsset.cases[]` in one click) | ||
|
|
||
| For cross-model regression coverage — running the full sweep matrix on | ||
| every case in the suite without manually switching FBXes — use **Run | ||
| Multi-Case (N × M)**. Located in *Setup → Parameter Sweep*, right of the | ||
| single-model **Run Sweep** button. `N` is the number of `cases`, `M` is | ||
| the cell count. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Название кнопки в доке не совпадает с UI.
В LightmapTransferTool.DrawSetupDebugSection кнопка подписана Run Benchmark ({caseCount} cases), а не «Run Multi-Case (N × M)»; по описанию её не найти. Заодно у блока с матрицей (строка 224) не указан язык — markdownlint MD040.
🤖 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 `@Documentation`~/TRANSFER_BENCHMARK.md around lines 178 - 184, Update the
multi-case sweep documentation to use the exact UI label from
LightmapTransferTool.DrawSetupDebugSection: “Run Benchmark ({caseCount} cases)”,
replacing “Run Multi-Case (N × M)”. Also add an explicit language identifier to
the matrix code block near the referenced section to satisfy markdownlint MD040.
Source: Linters/SAST tools
Comment on lines
+909
to
+916
| // Per-cell CSVs used to live at BenchmarkReports/ top level, | ||
| // but after the OutputDirectoryOverride redirect they're | ||
| // emitted inside sweep_<stamp>/ (or bench_<stamp>/<case>/legacy/ | ||
| // for the unified benchmark). Walk recursively so recovery | ||
| // works whether the operator points at the top-level reports | ||
| // root, a specific sweep run, or a unified bench case. | ||
| string[] csvFiles; | ||
| try { csvFiles = Directory.GetFiles(benchmarkReportsRoot, "*.csv", SearchOption.TopDirectoryOnly); } | ||
| try { csvFiles = Directory.GetFiles(benchmarkReportsRoot, "*.csv", SearchOption.AllDirectories); } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Рекурсивный поиск + кластеризация только по mtime смешает ячейки разных моделей.
После перехода на SearchOption.AllDirectories в выборку попадут CSV из bench_<ts>/<case>/legacy/ всех кейсов. В multi-case прогоне кейсы идут подряд, поэтому разрыв mtime между последней ячейкой модели A и первой ячейкой модели B почти всегда меньше kRecoveryGapSeconds = 300, и они склеятся в один кластер — восстановленный summary.csv/winner.json сравнит ячейки разных моделей между собой.
🛠 Предложение: кластеризовать в пределах одного каталога
- matched.Sort((a, b) => a.mtime.CompareTo(b.mtime));
+ // Разные модели multi-case прогона лежат в разных каталогах, но
+ // пишутся подряд по времени — сначала группируем по каталогу,
+ // иначе кластер по mtime склеит ячейки разных моделей.
+ matched.Sort((a, b) =>
+ {
+ int c = string.CompareOrdinal(
+ Path.GetDirectoryName(a.path) ?? "",
+ Path.GetDirectoryName(b.path) ?? "");
+ return c != 0 ? c : a.mtime.CompareTo(b.mtime);
+ });и добавить в цикл кластеризации условие смены каталога как границу кластера.
🤖 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/BenchmarkSweep.cs` around lines 909 - 916, Update the CSV clustering
logic in the recovery flow of Editor/BenchmarkSweep.cs so files are grouped only
within the same containing directory, not solely by modification-time gaps.
Track each file’s directory and force a cluster boundary when it changes, while
preserving the existing kRecoveryGapSeconds boundary and summary.csv/winner.json
generation for each valid cluster.
Comment on lines
+82
to
+87
| e3Note = $"\nAtlas check: overlap {overlapPx} texel(s), " + | ||
| $"unplaced faces {unplaced}, misaligned domains {misaligned}." + | ||
| (unplaced > 0 || misaligned > 0 | ||
| ? "\nWARNING: non-zero defects — see Console for per-LOD detail." | ||
| : ""); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
overlapPx показывается, но не влияет на предупреждение.
По Documentation~/EXPERIMENTS.md (E3) e3OverlapTexels — главный скаляр дефектности («минимизировать e3OverlapTexels при e3XLodMinPct близком к 100»), однако WARNING поднимается только по unplaced/misaligned. Атлас с массовым overlap'ом пройдёт как «чистый».
🛡️ Предлагаемая правка
- (unplaced > 0 || misaligned > 0
+ (overlapPx > 0 || unplaced > 0 || misaligned > 0
? "\nWARNING: non-zero defects — see Console for per-LOD detail."
: "");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| e3Note = $"\nAtlas check: overlap {overlapPx} texel(s), " + | |
| $"unplaced faces {unplaced}, misaligned domains {misaligned}." + | |
| (unplaced > 0 || misaligned > 0 | |
| ? "\nWARNING: non-zero defects — see Console for per-LOD detail." | |
| : ""); | |
| } | |
| e3Note = $"\nAtlas check: overlap {overlapPx} texel(s), " + | |
| $"unplaced faces {unplaced}, misaligned domains {misaligned}." + | |
| (overlapPx > 0 || unplaced > 0 || misaligned > 0 | |
| ? "\nWARNING: non-zero defects — see Console for per-LOD detail." | |
| : ""); | |
| } |
🤖 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/HierarchicalApply.cs` around lines 82 - 87, Update the WARNING
condition in the e3Note construction so it also triggers when overlapPx is
non-zero, alongside unplaced or misaligned defects. Keep the existing overlap,
unplaced, and misaligned values in the message and preserve the current
clean-message behavior when all three defect measures are zero.
Comment on lines
+104
to
+142
| var lods = lg.GetLODs(); | ||
| if (lods.Length < 2) | ||
| throw new System.InvalidOperationException( | ||
| "LODGroup needs at least 2 LOD levels for hierarchical probe."); | ||
| int deepestIdx = lods.Length - 1; | ||
|
|
||
| // Group renderers across LODs by MeshGroupKey so each fine renderer | ||
| // is paired with its deepest-LOD counterpart even when there are | ||
| // many renderers per LOD level (e.g. multi-mesh LODGroups). | ||
| var groups = new Dictionary<string, Renderer[]>(); | ||
| for (int li = 0; li < lods.Length; li++) | ||
| { | ||
| var rends = lods[li].renderers; | ||
| if (rends == null) continue; | ||
| foreach (var r in rends) | ||
| { | ||
| if (r == null) continue; | ||
| string key = UvToolContext.ExtractGroupKey(r.name); | ||
| if (!groups.TryGetValue(key, out var arr)) | ||
| { | ||
| arr = new Renderer[lods.Length]; | ||
| groups[key] = arr; | ||
| } | ||
| arr[li] = r; | ||
| } | ||
| } | ||
|
|
||
| if (groups.Count == 0) | ||
| throw new System.InvalidOperationException( | ||
| "No renderers found under LODGroup."); | ||
|
|
||
| var allRecords = new List<FaceProbeRecord>(); | ||
| int groupsProbed = 0, groupsSkipped = 0; | ||
| foreach (var kv in groups) | ||
| { | ||
| var arr = kv.Value; | ||
| if (arr[deepestIdx] == null) { groupsSkipped++; continue; } | ||
| var deepMesh = arr[deepestIdx].GetComponent<MeshFilter>()?.sharedMesh; | ||
| if (deepMesh == null) { groupsSkipped++; continue; } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
deepestIdx фиксирован как lods.Length - 1 — расходится с выбором deepest в HierarchicalRepack.
HierarchicalRepack.Build (строки 568-574) спускается вниз, пока не найдёт LOD с валидным мешем, а здесь берётся строго последний уровень: если у последнего LOD нет рендерера/меша, все группы уходят в groupsSkipped, зонд пишет пустой CSV, и диагностика молча ничего не измеряет. Это ровно тема T2 из Documentation~/TRANSFER_AUDIT_2026-07-18.md. Стоит либо переиспользовать общий выбор deepest, либо явно откатываться на первый уровень, у которого есть меш.
🤖 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/HierarchicalDiag.cs` around lines 104 - 142, Синхронизируйте выбор
deepest LOD в диагностике с логикой HierarchicalRepack.Build: не фиксируйте
deepestIdx на последнем уровне, а найдите последний LOD с валидным
MeshFilter.sharedMesh для соответствующей группы и используйте его как fallback,
чтобы группы с пустым последним LOD не пропускались. Сохраните groupsSkipped
только для групп без валидного меша на всех уровнях.
Comment on lines
+3188
to
+3192
| /// duplicated). Output meshes land on <c>r.finalMeshes[li]</c>. | ||
| /// In-memory only — the menu-driven Apply step swaps them into | ||
| /// renderers via Undo; without Apply they're orphaned and the | ||
| /// next GC cycle reclaims them.</summary> | ||
| public static void BuildFinalMeshes(LODGroup lg, Result r) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Утечка клонированных мешей: у результатов BuildFinalMeshes нет владельца жизненного цикла. UnityEngine.Mesh не собирается GC по потере managed-ссылки — в редакторе он живёт до перезагрузки домена, поэтому каждый неприменённый клон остаётся в памяти (бенчмарк и каждая ячейка BuildStageDSweep создают их пачками).
Editor/HierarchicalRepack.cs#L3188-L3192: исправить комментарий про «next GC cycle reclaims them» и предусмотреть явное освобождение — например, методDisposeFinalMeshes(Result), уничтожающий все ненулевыеr.finalMeshes[li], вызываемый вызывающей стороной после того, как меши больше не нужны.Editor/HierarchicalApply.cs#L121-L145: в ветках пропуска (clone == nullне в счёт;rs[0] == nullиmf == null) вызыватьUnityEngine.Object.DestroyImmediate(clone)передcontinue, чтобы неприменённые клоны не оставались висеть.
Как per coding guidelines: «Destroy temporary repacked, transferred, and welded meshes when they are no longer needed».
📍 Affects 2 files
Editor/HierarchicalRepack.cs#L3188-L3192(this comment)Editor/HierarchicalApply.cs#L121-L145
🤖 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/HierarchicalRepack.cs` around lines 3188 - 3192, Add an explicit
final-mesh cleanup path: update BuildFinalMeshes documentation and introduce
DisposeFinalMeshes(Result) in Editor/HierarchicalRepack.cs to DestroyImmediate
every non-null r.finalMeshes entry when no longer needed; in
Editor/HierarchicalApply.cs lines 121-145, destroy clone before continuing when
rs[0] or mf is null, while leaving clone == null unchanged.
Source: Coding guidelines
Comment on lines
+1209
to
+1213
| /// <summary> | ||
| /// <summary> | ||
| /// Ratio of summed |triangle UV area| to UV bbox area for a shell. | ||
| /// A clean non-overlapping unwrap covers a fraction of its bbox so the | ||
| /// ratio is <= 1.0 (rectangle wrap ≈ 1.0, irregular shapes lower). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Дублированный <summary> ломает XML-документацию.
Открывающих тегов два, закрывающий один — компилятор выдаст предупреждение о некорректном XML-комментарии.
🛠 Исправление
- /// <summary>
/// <summary>
/// Ratio of summed |triangle UV area| to UV bbox area for a shell.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /// <summary> | |
| /// <summary> | |
| /// Ratio of summed |triangle UV area| to UV bbox area for a shell. | |
| /// A clean non-overlapping unwrap covers a fraction of its bbox so the | |
| /// ratio is <= 1.0 (rectangle wrap ≈ 1.0, irregular shapes lower). | |
| /// <summary> | |
| /// Ratio of summed |triangle UV area| to UV bbox area for a shell. | |
| /// A clean non-overlapping unwrap covers a fraction of its bbox so the | |
| /// ratio is <= 1.0 (rectangle wrap ≈ 1.0, irregular shapes lower). |
🤖 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/SymmetrySplitShells.cs` around lines 1209 - 1213, Remove the duplicate
opening <summary> tag in the XML documentation comment for the shell UV-area
ratio, leaving one opening tag paired with the existing closing tag.
Comment on lines
+2249
to
+2294
| void ResetWorkingMeshesToFbx() | ||
| { | ||
| if (ctx?.MeshEntries == null) return; | ||
| foreach (var e in ctx.MeshEntries) | ||
| { | ||
| if (e == null) continue; | ||
|
|
||
| // Drop derived meshes produced by an earlier run. | ||
| if (e.transferredMesh != null) | ||
| { | ||
| UnityEngine.Object.DestroyImmediate(e.transferredMesh); | ||
| e.transferredMesh = null; | ||
| } | ||
| if (e.repackedMesh != null) | ||
| { | ||
| UnityEngine.Object.DestroyImmediate(e.repackedMesh); | ||
| e.repackedMesh = null; | ||
| } | ||
| e.repackedAtlasWidth = 0; | ||
| e.repackedAtlasHeight = 0; | ||
| e.transferState = null; | ||
| e.shellTransferResult = null; | ||
| e.validationReport = null; | ||
|
|
||
| // Rewind the working mesh to the imported fbx asset. The stale | ||
| // working clone (weld / sym-split product) is destroyed — it is | ||
| // ours, never the asset (fbxMesh is owned by the AssetDatabase). | ||
| if (e.fbxMesh != null) | ||
| { | ||
| if (e.originalMesh != null && e.originalMesh != e.fbxMesh) | ||
| UnityEngine.Object.DestroyImmediate(e.originalMesh); | ||
| e.originalMesh = e.fbxMesh; | ||
| } | ||
|
|
||
| e.wasWelded = false; | ||
| e.wasEdgeWelded = false; | ||
| e.wasSymmetrySplit = false; | ||
| } | ||
|
|
||
| ctx.ClearAllCaches(); | ||
| accumulatedOverlapHints.Clear(); | ||
| shellTransformCache.Clear(); | ||
| ctx.HasRepack = false; | ||
| ctx.HasTransfer = false; | ||
| uv0Welded = false; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
ResetWorkingMeshesToFbx уничтожает рабочий меш, не сняв его с MeshFilter.
ResetWorkingCopies (строки 4855-4861) сначала возвращает e.meshFilter.sharedMesh = e.fbxMesh, и только потом делает DestroyImmediate. Здесь этого шага нет: если предыдущий прогон/Update refs/Apply оставил на MeshFilter рабочую копию, после DestroyImmediate(e.originalMesh) рендерер получит уничтоженный меш и останется с Missing-ссылкой, пока стадии не назначат новый (а при Repack/Transfer=off они этого не сделают).
Дополнительно: accumulatedMatchHints не очищается (в отличие от accumulatedOverlapHints), поэтому кросс-LOD подсказки предыдущего прогона протекают в новый и нарушают заявленную идемпотентность «чистая функция от (fbxMesh, settings)».
🐛 Предлагаемое исправление
foreach (var e in ctx.MeshEntries)
{
if (e == null) continue;
+ // Снять рабочую копию со сцены ДО уничтожения, иначе
+ // MeshFilter останется с destroyed-мешем.
+ if (e.meshFilter != null && e.fbxMesh != null)
+ e.meshFilter.sharedMesh = e.fbxMesh;
+
// Drop derived meshes produced by an earlier run.
if (e.transferredMesh != null)
@@
ctx.ClearAllCaches();
accumulatedOverlapHints.Clear();
+ accumulatedMatchHints.Clear();
shellTransformCache.Clear();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void ResetWorkingMeshesToFbx() | |
| { | |
| if (ctx?.MeshEntries == null) return; | |
| foreach (var e in ctx.MeshEntries) | |
| { | |
| if (e == null) continue; | |
| // Drop derived meshes produced by an earlier run. | |
| if (e.transferredMesh != null) | |
| { | |
| UnityEngine.Object.DestroyImmediate(e.transferredMesh); | |
| e.transferredMesh = null; | |
| } | |
| if (e.repackedMesh != null) | |
| { | |
| UnityEngine.Object.DestroyImmediate(e.repackedMesh); | |
| e.repackedMesh = null; | |
| } | |
| e.repackedAtlasWidth = 0; | |
| e.repackedAtlasHeight = 0; | |
| e.transferState = null; | |
| e.shellTransferResult = null; | |
| e.validationReport = null; | |
| // Rewind the working mesh to the imported fbx asset. The stale | |
| // working clone (weld / sym-split product) is destroyed — it is | |
| // ours, never the asset (fbxMesh is owned by the AssetDatabase). | |
| if (e.fbxMesh != null) | |
| { | |
| if (e.originalMesh != null && e.originalMesh != e.fbxMesh) | |
| UnityEngine.Object.DestroyImmediate(e.originalMesh); | |
| e.originalMesh = e.fbxMesh; | |
| } | |
| e.wasWelded = false; | |
| e.wasEdgeWelded = false; | |
| e.wasSymmetrySplit = false; | |
| } | |
| ctx.ClearAllCaches(); | |
| accumulatedOverlapHints.Clear(); | |
| shellTransformCache.Clear(); | |
| ctx.HasRepack = false; | |
| ctx.HasTransfer = false; | |
| uv0Welded = false; | |
| } | |
| void ResetWorkingMeshesToFbx() | |
| { | |
| if (ctx?.MeshEntries == null) return; | |
| foreach (var e in ctx.MeshEntries) | |
| { | |
| if (e == null) continue; | |
| // Снять рабочую копию со сцены ДО уничтожения, иначе | |
| // MeshFilter останется с destroyed-мешем. | |
| if (e.meshFilter != null && e.fbxMesh != null) | |
| e.meshFilter.sharedMesh = e.fbxMesh; | |
| // Drop derived meshes produced by an earlier run. | |
| if (e.transferredMesh != null) | |
| { | |
| UnityEngine.Object.DestroyImmediate(e.transferredMesh); | |
| e.transferredMesh = null; | |
| } | |
| if (e.repackedMesh != null) | |
| { | |
| UnityEngine.Object.DestroyImmediate(e.repackedMesh); | |
| e.repackedMesh = null; | |
| } | |
| e.repackedAtlasWidth = 0; | |
| e.repackedAtlasHeight = 0; | |
| e.transferState = null; | |
| e.shellTransferResult = null; | |
| e.validationReport = null; | |
| // Rewind the working mesh to the imported fbx asset. The stale | |
| // working clone (weld / sym-split product) is destroyed — it is | |
| // ours, never the asset (fbxMesh is owned by the AssetDatabase). | |
| if (e.fbxMesh != null) | |
| { | |
| if (e.originalMesh != null && e.originalMesh != e.fbxMesh) | |
| UnityEngine.Object.DestroyImmediate(e.originalMesh); | |
| e.originalMesh = e.fbxMesh; | |
| } | |
| e.wasWelded = false; | |
| e.wasEdgeWelded = false; | |
| e.wasSymmetrySplit = false; | |
| } | |
| ctx.ClearAllCaches(); | |
| accumulatedOverlapHints.Clear(); | |
| accumulatedMatchHints.Clear(); | |
| shellTransformCache.Clear(); | |
| ctx.HasRepack = false; | |
| ctx.HasTransfer = false; | |
| uv0Welded = false; | |
| } |
🤖 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/LightmapTransferTool.cs` around lines 2249 - 2294, Update
ResetWorkingMeshesToFbx to restore each entry’s MeshFilter.sharedMesh to
e.fbxMesh before destroying e.originalMesh, matching ResetWorkingCopies and
preventing destroyed meshes from remaining referenced. Also clear
accumulatedMatchHints alongside accumulatedOverlapHints so reset state is
independent of prior runs.
Comment on lines
+28
to
+30
| TransferDiag = 1 << 10, | ||
|
|
||
| All = General | SymSplit | Repack | Match | Dedup | Overlap | Topology | Validation | Export | Benchmark | TransferDiag, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
У существующих пользователей новый бит TransferDiag окажется выключенным.
EnabledCategories читает EditorPrefs.GetInt(MaskPrefKey, (int)Category.All) — дефолт применяется только при отсутствии ключа. У всех, кто хоть раз трогал Log filters, сохранена старая маска без бита 10, поэтому новая диагностика будет молча подавлена, хотя changelog обещает, что категория «просто появится».
🛠 Вариант: считать неизвестные (новые) биты включёнными
if (!_cachedMask.HasValue)
- _cachedMask = EditorPrefs.GetInt(MaskPrefKey, (int)Category.All);
+ {
+ int stored = EditorPrefs.GetInt(MaskPrefKey, (int)Category.All);
+ // Биты категорий, добавленных после сохранения маски,
+ // включаем по умолчанию — иначе новая диагностика молча немая.
+ const int knownAtLastMigration = (1 << 10) - 1; // до TransferDiag
+ _cachedMask = stored | (~knownAtLastMigration & (int)Category.All);
+ }🤖 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/UvtLog.cs` around lines 28 - 30, Обновите логику чтения
EnabledCategories для сохранённых значений EditorPrefs по ключу MaskPrefKey:
автоматически добавляйте в маску новые неизвестные биты, включая TransferDiag,
вместо применения сохранённой старой маски без них. Не изменяйте поведение для
уже известных категорий и сохраните значение Category.All как дефолт при
отсутствии ключа.
Comment on lines
+140
to
+144
| // Snap charts to block boundaries (4×4 by default — DXT/BC | ||
| // tile size). Stops a chart's right/bottom edge from | ||
| // straddling a compression block edge, which otherwise eats | ||
| // 1 texel of leak budget on every compressed lightmap. | ||
| blockAlign = true, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant file and surrounding lines =="
git ls-files 'Editor/XatlasRepack.cs'
echo
wc -l Editor/XatlasRepack.cs
echo
sed -n '1,240p' Editor/XatlasRepack.cs
echo
echo "== Search for internalOversample and blockAlign usage =="
rg -n "internalOversample|blockAlign|PackOptions|xatlas" Editor/XatlasRepack.csRepository: SashaRX/UnityMeshLab
Length of output: 23906
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Search vendored xatlas references in repo =="
rg -n "blockAlign|internalOversample|PackOptions" . --glob '!**/node_modules/**' --glob '!**/.git/**'Repository: SashaRX/UnityMeshLab
Length of output: 8895
🌐 Web query:
xatlas PackOptions blockAlign 4x4 internalOversample documentation
💡 Result:
In the xatlas library, PackOptions is a structure used to configure the packing of texture charts into an atlas [1][2]. The blockAlign option, when set to true, forces charts to be aligned to 4x4 pixel blocks [1]. This can improve packing speed because it reduces the number of possible chart locations the algorithm needs to evaluate [1]. Regarding internalOversample, this specific parameter does not exist in the standard xatlas::PackOptions structure [1][3][4]. Xatlas documentation and source code focus on parameters such as attempts, resolution, texelsPerUnit, padding, and maxChartSize to control the packing process [1][4]. If you encountered a reference to internalOversample, it may be part of a custom modification, an external wrapper, or a different library entirely.
Citations:
- 1: https://github.com/MozillaReality/xatlas-web/blob/master/xatlas.h
- 2: https://github.com/jpcy/xatlas/blob/master/README.md
- 3: https://docs.rs/xatlas-rs/latest/xatlas_rs/struct.PackOptions.html
- 4: https://github.com/buckaroo-upgrade-bot/jpcy-xatlas/blob/master/xatlas.h
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Editor/XatlasRepack.cs around xatlasPackCharts call =="
sed -n '1100,1140p' Editor/XatlasRepack.cs
echo
echo "== Native~/xatlas-unity-bridge.cpp relevant section =="
sed -n '100,150p' Native~/xatlas-unity-bridge.cpp
echo
echo "== Editor/XatlasNative.cs signature =="
sed -n '1,120p' Editor/XatlasNative.csRepository: SashaRX/UnityMeshLab
Length of output: 6394
Не связывайте blockAlign с internalOversample через 4×4.
В Editor/XatlasRepack.cs (internalOversample = 4, вызов xatlasPackCharts) выравнивание xatlas идёт в internal-координатах, а потом UV нормализуются обратно. Поэтому blockAlign = true даёт примерно 1×1 пользовательский пиксель, а не защиту границ BC/ETC на 4×4. Нужен snap в пользовательском разрешении или пересчёт блока с учётом oversample.
🤖 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/XatlasRepack.cs` around lines 140 - 144, Исправьте настройку
blockAlign в xatlasPackCharts с учётом internalOversample: не используйте
фиксированное выравнивание 4×4 во внутренних координатах как защиту блока
BC/ETC. Реализуйте snap в пользовательском разрешении либо пересчитайте размер
блока в internal-координатах через internalOversample, сохранив корректное
выравнивание границ после нормализации UV.
- Updated package identifier from `com.sasharx.lightmap-uv-tool` to `com.sasharx.unitymeshlab` for consistency with the repository name. - Renamed root namespace from `LightmapUvTool` to `SashaRX.UnityMeshLab` across all Editor C# files, ensuring compliance with naming conventions. - Adjusted logging prefixes from `[LightmapUV]` to `[MeshLab]` in various files to reflect the new package identity. - Updated FBX exporter code guards from `#if LIGHTMAP_UV_TOOL_FBX_EXPORTER` to `#if UNITY_MESH_LAB_FBX_EXPORTER` for consistency. - Modified asset paths and shader names to align with the new package structure. - Removed outdated documentation and references related to the previous package identity.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Editor/Tools/CollisionMeshTool.cs (1)
467-474: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winP1: Создавайте весь каталог вывода рекурсивно.
После смены default path на
Assets/UnityMeshLab/Outputэтот код создаёт только конечную папку. ЕслиAssets/UnityMeshLabещё отсутствует,CreateFolderзавершится ошибкой, и сохранение generated mesh assets не сработает. Создайте родительские каталоги поэтапно передCreateAsset.🤖 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/CollisionMeshTool.cs` around lines 467 - 474, The output-directory setup around savePath must create all missing parent folders recursively before saving generated mesh assets. Update the folder-creation logic in the CollisionMeshTool save flow to walk each path segment and create missing directories under Assets, ensuring the final savePath is valid before CreateAsset runs.
🤖 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 @.claude/skills/repo-auditor/SKILL.md:
- Line 14: Update the repo-auditor skill’s discovery and structure checks to use
only an explicit allowlist manifest or caller-provided parameters. Remove
scanning, enumeration, or reading of other agent skills and configuration
directories, including .claude/, .codex/, and .gemini/; specifically revise the
discovery command referenced near the skill’s discovery instructions.
In `@Editor/Settings/MeshLabProjectSettings.cs`:
- Line 18: Добавьте миграцию при загрузке MeshLabProjectSettings, которая
заменяет старое значение savePath на новый Assets/UnityMeshLab/Output только для
существующих настроек с Assets/LightmapUvTool_Output; пользовательские значения
не изменяйте и сохраните обновлённые настройки.
In `@Editor/Tools/CleanupTool.cs`:
- Around line 591-592: Update the hidden-material predicate in the
embedded-material scan around isHidden to recognize both Hidden_UnityMeshLab and
Hidden/UnityMeshLab prefixes, matching the external remap scan. Reuse or extract
the shared predicate used by both scans so their detection behavior remains
consistent.
In `@Editor/UvtLog.cs`:
- Around line 33-35: Update the EditorPrefs access in UvtLog to migrate legacy
LightmapUvTool_* keys: when UnityMeshLab_LogLevel or
UnityMeshLab_LogCategoryMask is absent, read the corresponding old key, then
persist the recovered value under the new key while preserving existing defaults
when neither key exists.
---
Outside diff comments:
In `@Editor/Tools/CollisionMeshTool.cs`:
- Around line 467-474: The output-directory setup around savePath must create
all missing parent folders recursively before saving generated mesh assets.
Update the folder-creation logic in the CollisionMeshTool save flow to walk each
path segment and create missing directories under Assets, ensuring the final
savePath is valid before CreateAsset runs.
🪄 Autofix (Beta)
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: ef35e53d-8cb8-4488-9b53-7b4b93feffdf
⛔ Files ignored due to path filters (4)
Shaders/CheckerUV2.shaderis excluded by!**/*.shaderShaders/SpotProjection.shaderis excluded by!**/*.shaderShaders/TintedTexture.shaderis excluded by!**/*.shaderShaders/VertexAODepth.shaderis excluded by!**/*.shader
📒 Files selected for processing (32)
.claude/skills/_archive/migration-and-refactor-planner.pre-overhaul.md.claude/skills/_archive/repo-auditor.pre-overhaul.md.claude/skills/_archive/unity-assetdatabase-tools.pre-overhaul.md.claude/skills/_archive/unity-editor-tooling.pre-overhaul.md.claude/skills/_archive/unity-package-architect.pre-overhaul.md.claude/skills/_archive/unity-package-reviewer.pre-overhaul.md.claude/skills/_archive/unity-serialized-workflow.pre-overhaul.md.claude/skills/_archive/unity-undo-prefab-safety.pre-overhaul.md.claude/skills/_shared/naming-conventions.md.claude/skills/_shared/version-gates.md.claude/skills/repo-auditor/SKILL.md.claude/skills/repo-conventions/SKILL.md.claude/skills/skills-overhaul-plan.md.claude/skills/unity-package-architect/SKILL.md.github/PULL_REQUEST_TEMPLATE.mdAGENTS.mdCHANGELOG.mdCLAUDE.mdDocumentation~/EMBREE_INTEGRATION_PLAN.mdDocumentation~/REVIEW.mdDocumentation~/TRANSFER_BENCHMARK.mdEditor/CheckerTexturePreview.csEditor/Framework/UvCanvasView.csEditor/Framework/UvToolHub.csEditor/SashaRX.UnityMeshLab.Editor.asmdefEditor/Settings/MeshLabProjectSettings.csEditor/Tools/CleanupTool.csEditor/Tools/CollisionMeshTool.csEditor/Tools/LightmapTransferTool.csEditor/Uv2DataAsset.csEditor/UvTransferPipeline.csEditor/UvtLog.cs
💤 Files with no reviewable changes (9)
- .claude/skills/_archive/migration-and-refactor-planner.pre-overhaul.md
- .claude/skills/_archive/unity-package-architect.pre-overhaul.md
- .claude/skills/_archive/unity-undo-prefab-safety.pre-overhaul.md
- .claude/skills/_archive/unity-package-reviewer.pre-overhaul.md
- .claude/skills/_archive/unity-assetdatabase-tools.pre-overhaul.md
- .claude/skills/_archive/unity-serialized-workflow.pre-overhaul.md
- .claude/skills/_archive/repo-auditor.pre-overhaul.md
- .claude/skills/_archive/unity-editor-tooling.pre-overhaul.md
- .claude/skills/skills-overhaul-plan.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/PULL_REQUEST_TEMPLATE.md
- Documentation~/TRANSFER_BENCHMARK.md
- Editor/Tools/LightmapTransferTool.cs
| Covered here: | ||
|
|
||
| - Skills directory structure and coherence with `.claude/skills/skills-overhaul-plan.md` if present. | ||
| - Skills directory structure and internal reference coherence. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Information Disclosure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: Internal
P1: Не перечисляйте другие agent skills и конфигурационные каталоги.
Добавление этой проверки закрепляет доступ к .claude/skills, а discovery-команда на Line 101 перечисляет и читает SKILL.md других skills. Ограничьте аудит явным allowlist-манифестом или переданными параметрами; не сканируйте .claude/, .codex/ или .gemini/.
🧰 Tools
🪛 SkillSpector (2.3.11)
[error] 101: [AS1] Agent Config Directory Access: Skill reads from agent configuration directories (.claude/, .codex/, .gemini/). These directories may contain API keys, personal settings, and other credentials that the skill has no legitimate need to access.
Remediation: Remove all code or instructions that access agent configuration directories (.claude/, .codex/, .gemini/). If configuration values are needed, pass them explicitly as parameters or environment variables — never read the agent's own config files.
(Agent Snooping (AS1))
[warning] 55: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
[warning] 101: [AS3] Skill Enumeration: Skill enumerates or reads other installed skills. Access to other skills' SKILL.md files or the skills directory reveals prompt instructions, capabilities, and secrets that should be invisible to peer skills.
Remediation: Remove all code or instructions that list or read other skills' files or directories. Skills should operate independently; cross-skill access is a privilege escalation.
(Agent Snooping (AS3))
🤖 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 @.claude/skills/repo-auditor/SKILL.md at line 14, Update the repo-auditor
skill’s discovery and structure checks to use only an explicit allowlist
manifest or caller-provided parameters. Remove scanning, enumeration, or reading
of other agent skills and configuration directories, including .claude/,
.codex/, and .gemini/; specifically revise the discovery command referenced near
the skill’s discovery instructions.
Source: Linters/SAST tools
|
|
||
| // ── Output ── | ||
| public string savePath = "Assets/LightmapUvTool_Output"; | ||
| public string savePath = "Assets/UnityMeshLab/Output"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
P1 — Мигрируйте сохранённый savePath.
В Editor/Settings/MeshLabProjectSettings.cs:18 изменён только initializer; существующий ProjectSettings/MeshLabSettings.asset сохранит Assets/LightmapUvTool_Output, поэтому текущие проекты не перейдут на новый каталог. Добавьте миграцию при загрузке, заменяющую старое значение только если пользователь его не переопределял.
🤖 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/Settings/MeshLabProjectSettings.cs` at line 18, Добавьте миграцию при
загрузке MeshLabProjectSettings, которая заменяет старое значение savePath на
новый Assets/UnityMeshLab/Output только для существующих настроек с
Assets/LightmapUvTool_Output; пользовательские значения не изменяйте и сохраните
обновлённые настройки.
Source: Learnings
Comment on lines
+591
to
592
| bool isHidden = mat.name.StartsWith("Hidden_UnityMeshLab") | ||
| || mat.shader.name.StartsWith(CheckerTexturePreview.ToolShaderPrefix); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
P1: Учитывайте оба формата имени скрытого материала.
Внешний remap-скан на Lines 562-563 уже проверяет Hidden/UnityMeshLab, но embedded-material scan проверяет только Hidden_UnityMeshLab. Поэтому встроенные FBX-материалы с префиксом Hidden/UnityMeshLab будут пропущены. Используйте общий предикат для обоих сканов.
🤖 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 591 - 592, Update the
hidden-material predicate in the embedded-material scan around isHidden to
recognize both Hidden_UnityMeshLab and Hidden/UnityMeshLab prefixes, matching
the external remap scan. Reuse or extract the shared predicate used by both
scans so their detection behavior remains consistent.
Comment on lines
+33
to
+35
| const string LevelPrefKey = "UnityMeshLab_LogLevel"; | ||
| const string MaskPrefKey = "UnityMeshLab_LogCategoryMask"; | ||
| const string Prefix = "[MeshLab]"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
P2: Мигрируйте старые ключи EditorPrefs.
Переименование ключей без fallback или одноразовой миграции сбрасывает у существующих пользователей уровень логирования и маску категорий. Сначала прочитайте старые LightmapUvTool_* ключи при отсутствии новых, затем сохраните значения под UnityMeshLab_*.
🤖 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/UvtLog.cs` around lines 33 - 35, Update the EditorPrefs access in
UvtLog to migrate legacy LightmapUvTool_* keys: when UnityMeshLab_LogLevel or
UnityMeshLab_LogCategoryMask is absent, read the corresponding old key, then
persist the recovered value under the new key while preserving existing defaults
when neither key exists.
9 tasks
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
TestSuiteAsset.cases[], instantiates each FBX, runs the full sweep matrix, and writes per-model reports tosweep_<ts>_<model>/subdirectories.uv2DuplicatePairs,compositeBrokenCount, andsevereMismatchCounttoTransferResultto catch failure modes (silent lightmap bleeding, matching misses, wrong-source assignments) that existing metrics miss.TransferDiaglog category, showing shell status histogram, match distance stats, and visual-defect counts for easier sweep validation.BenchmarkSweep.Score()to penalize visual defects (-50for duplicate UV2,-30for force3D overlaps,-10for composite-broken,-20for severe mismatch).Changed Zones
Editor/— Editor tools / UIREADME.md,CHANGELOG.md)Checklist
finallyblocksctx.LodGrouprestored after multi-case loopTest Plan
Multi-case sweep:
TestSuiteAssetwith 2–3 cases (each with valid FBX + LODGroup).BenchmarkReports/sweep_<ts>_<model1>/,sweep_<ts>_<model2>/directories created with separatesummary.csv+winner.json.ctx.LodGrouprestored after loop completes or is cancelled.Visual-defect metrics:
uv2DuplicatePairs).uv2DuplicatePairs,compositeBrokenCount,severeMismatchCountpresent and non-zero where expected.Transfer diagnostic summary:
TransferDiaglog category in UvtLog settings.Review Notes
ExecMultiCaseSweep()usesPrefabUtility.InstantiatePrefab()+HideFlags.DontSaveto avoid scene-dirty leakage; spawned roots destroyed infinallyblocks.SanitizeForPath()helper collapses non-alphanumeric characters to_for safe directory names.TransferCore()and exposed in CSV/JSON for sweep scoring and cross-model pandas analysis.TransferDiagcategory added toUvtLog.Categoryenum; gated by both log level and category filter so users can toggle independently.https://claude.ai/code/session_01Bwxb62ZvuCSRyfjFEWTQ5V
Summary by CodeRabbit
Новые возможности
Изменения
Документация