Skip to content

TransferValidator: определение shell треугольника по majority-vote (3 вершины) - #18

Merged
SashaRX merged 1 commit into
masterfrom
codex/update-shell-computation-logic
Mar 3, 2026
Merged

TransferValidator: определение shell треугольника по majority-vote (3 вершины)#18
SashaRX merged 1 commit into
masterfrom
codex/update-shell-computation-logic

Conversation

@SashaRX

@SashaRX SashaRX commented Mar 3, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Исправить ошибочную привязку shell к треугольнику, когда учитывался только первый индекс вершины, что могло приводить к неверной агрегации stretch по шеллам.
  • Сделать логику выбора shell для треугольника более устойчивой к смешанным вершинам за счёт учёта всех трёх вершин и правила majority-vote.

Description

  • В Validate(...) (Phase 2) изменена логика назначения triShell[f]: теперь анализируются все три вершины (i0, i1, i2) вместо только i0.
  • Реализован majority-vote: если два или три значения shell совпадают и >= 0, используется этот shell; при отсутствии совпадений выполняется fallback на первый валидный shell среди трёх вершин; если валидных нет, присваивается -1.
  • Агрегация shellStretchLists и вычисление медиан/флагов outlier оставлены без изменений и применяются к новому triShell[f].
  • Обновлён сопроводительный комментарий рядом с кодом, он теперь соответствует реальной реализации majority-vote + fallback.

Testing

  • Выполнена автоматическая проверка изменений через git diff -- Editor/TransferValidator.cs, которая подтвердила ожидаемый патч (успешно).
  • Проверен статус репозитория командой git status --short после модификации (успешно).
  • Изменения добавлены и зафиксированы командой git add Editor/TransferValidator.cs && git commit -m "Update triangle shell selection to 3-vertex majority vote" (коммит создан успешно).

Codex Task

SashaRX pushed a commit that referenced this pull request Mar 3, 2026
…urce-only split)

Merged PRs:
- #18 codex/update-shell-computation-logic (majority vote)
- #17 codex/add-symmetry-split-support-to-sidecar-metadata (sidecar flag)
- #16 codex/refactor-execsymmetrysplit-to-handle-source-lod (source-only default)

Resolved merge conflict between PR #17 and #16 in ExecSymmetrySplit():
combined LOD filtering with wasSymmetrySplit tracking.

Removed REVIEW.md (deleted by all 3 PRs).

https://claude.ai/code/session_01198JqNgkdbzwRFc7nt8FqJ
@SashaRX
SashaRX merged commit f1f0b65 into master Mar 3, 2026
@SashaRX
SashaRX deleted the codex/update-shell-computation-logic branch March 4, 2026 00:09
SashaRX pushed a commit that referenced this pull request May 12, 2026
Vendors xatlas master @ today into Native~/third_party/xatlas/ and
patches the per-chart ceil(extents) rescale in PackCharts (xatlas.cpp
~line 8345 — "Scale charts to use the entire texel area"). Upstream
Issue #18 ("Add packer option to retain texture sizes") is closed
wontfix; the patch lives in our fork instead.

When PreserveChartScale is true:
  - chart extents still round up to integer pixel bbox (for packer slot)
  - but per-chart UV vertices keep their input scale (no rescale loop)
  - uniform per-shell texel density end-to-end (au2/a3 ratio = 1.0×)
  - trade-off: sub-pixel-thin shells take a full 1×1 slot with empty
    margin → some atlas utilization is lost

When false (= stock xatlas behaviour):
  - charts non-uniformly stretch to fill their integer bbox
  - tightest pack but per-chart density variance up to ~14× on real
    artist UVs (the bug we've been chasing)

Plumbing:
  - Native~/CMakeLists.txt: use local third_party/xatlas/ instead of
    FetchContent (so the patch survives CI builds)
  - xatlas-unity-bridge.cpp: xatlasPackCharts grows a new
    preserveChartScale int parameter, wired to a thread-safe setter
    in the patched xatlas
  - Editor/XatlasNative.cs: matching P/Invoke signature
  - RepackOptions.preserveChartScale (default true)
  - UvToolContext.PreserveChartScale (default true)
  - UI toggle "Preserve chart scale (forked xatlas)" in Pre-pack panel

The build-native.yml workflow already triggers on Native~/** changes,
will produce fresh DLLs for Windows/Linux/macOS and auto-commit them.
SashaRX pushed a commit that referenced this pull request May 13, 2026
Probe confirms patched DLL is loaded and preserveChartScale=1 is being
passed in, but density spread still measures ~14×. Either the
if(!s_preserveChartScale) gate isn't taken at runtime (some scope or
linkage issue I'm missing) or there's a second per-chart scale step
downstream that the upstream Issue #18 discussion didn't cover.

Adds two counters bumped from inside Stage B itself:
- s_stageBSeenCount      — entered the "if (extents>0)" block
- s_stageBRescaledCount  — actually ran the rescale loop

After PackCharts returns, C# reads them via xatlasGetStageBSeenCount /
xatlasGetStageBRescaledCount and logs:
  [xatlas-bridge] post-pack: flag=N stageB-seen=K stageB-rescaled=M

Expected on patched + flag=1:  seen=149 (number of charts), rescaled=0.
If rescaled>0 with flag=1     → the gate isn't taking the patched branch.
If rescaled=0 but density 14× → the rescale loop isn't the source; look
                                 for a second scale step.
SashaRX pushed a commit that referenced this pull request May 13, 2026
Approach swap: instead of patching xatlas's per-chart ceil(extents)
rescale (xatlas.cpp:8345-8362, upstream Issue #18 wontfix), prepare
the input so that ceil() becomes a no-op. SnapShellsToIntegerPixels
scales each shell per-axis around its UV centroid so the bbox extent
is already an integer number of atlas texels. Uniform per-shell
texel density survives the pack without forking xatlas.

Native:
- Drop vendored Native~/third_party/xatlas/ (~10K LOC).
- CMakeLists.txt back to FetchContent_Declare(xatlas) against upstream master.
- xatlas-unity-bridge.cpp: drop probe / Stage B counters / preserveChartScale.

Managed:
- XatlasRepack.SnapShellsToIntegerPixels — per-axis pre-pack snap, called
  from RepackSingle and RepackMulti after TexelDensityNormalizer.Normalize.
- RepackOptions.snapShellsToIntegerPixels (default true) replaces
  preserveChartScale.
- UvToolContext.SnapShellsToIntegerPixels + UI toggle in Pre-pack panel.

Codex review fixes:
- ExecSweep: force RepackResolutionMode = Manual for the sweep, restore
  on exit. Previously AutoFromTexelDensity overrode every cell's atlasRes
  and collapsed the resolution dimension of the sweep.
- BenchmarkRecorder.SetResolvedAtlasResolution — ExecRepackCore stamps
  the recorder with the resolution xatlas actually packed at, so the
  atlasRes column reflects post auto-compute, not the raw ctx setting.
- ExecFullPipeline / ExecTransferAll RecordMesh loops skip entries with
  include == false (user-deselected meshes were surfacing as failed rows
  in sweep aggregates).
- BenchmarkSweep totalMs uses pipelineMs (already wraps inner stages);
  falls back to repack+transfer+validate only for standalone runs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant