fix(chargen): Campaign CC gate round 1 closeout — Group 3: round review fixes (F4-F11, F14, F16)

The remaining code-bearing findings from the round review, F4-F16 minus
the doc-only items (batched separately):

- F4: three client-wide UiButton corpus sweeps (LabelBox path — exactly
  the 4 Town buttons, confined to chargen; conflicting custom-selection-
  pair + standard Normal/Highlight media — zero found, no gate
  tightening needed; per-state label-color map — 209 matches beyond
  chargen, confirming AP-222's mechanism has always been broadly active
  since it shipped generically in DatWidgetFactory).
- F5/F6: LayoutImporter's Batch C un-consumed-children carve-out now
  honors a child's own AuthoredInvisible flag (a narrow honor scoped to
  exactly that carve-out, not the general #408 client-wide one) — the
  chat transcript's new-text indicator (0x1000048C) was building as a
  visible phantom element retail never shows; verified both directions
  against the gold-frame pieces, which do not author Invisible.
- F7: BoundedProcessOutputCapture.AppendLine combines the line text and
  its trailing newline into one buffer and one file open/write/close
  instead of two.
- F9: corrected a stale comment in RuntimeSettingsTargets — #407 split
  DisplayModeCatalog's Resolutions/WindowedResolutions in two, so the
  fullscreen validator's own narrower list is now DELIBERATELY different
  from the Config dropdown's fuller offering, not the "must match" bug
  the comment described.
- F10: documented (not changed) why the LabelBox path's default 3px
  inset and the face-relative +4px gap in DatWidgetFactory.BuildButton
  are deliberately different numbers — neither carries a retail
  citation, and moving either to match the other would be an unfounded
  guess on a button that currently works correctly.
- F11: Heritage/Profession/Summary/Town description pages now compose
  DatRichText.Compose's result ONCE inside their already revision-gated
  Refresh, caching the built line list instead of re-wrapping on every
  draw call.
- F14: documented (not changed) why PrivateEntityViewportRenderer's
  _animatedIds set carrying a reserved-but-never-drawn backdrop id is
  harmless — BuildDrawEntities already excludes a null/empty backdrop
  from the actual draw list, so the id is never looked up.
- F16: the Summary preview now uses its own render-id pair
  (SummaryPreviewRenderId/SummaryPreviewBackdropRenderId, 0xDA11D035/
  0xDA11D036) instead of sharing the Appearance page's
  (0xDA11D032/0xDA11D034) — confirmed by tracing
  FixedEntityTextureOwnerLease through TextureCache to
  CompositeTextureArrayCache's shared owner tracker that both pages'
  previews share ONE process-wide TextureCache, so sharing render ids
  was a real cross-page texture-release collision (either page's own
  re-dress or disposal could release the OTHER page's still-active
  textures), not a theoretical one.

F3's own register bookkeeping (AP-229 addendum) and F12's register/AD
header-count corrections land in the docs-only commit alongside F15.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-16 15:34:07 +02:00
parent 0fed5fdd91
commit bd359d5181
17 changed files with 636 additions and 37 deletions

View file

@ -180,6 +180,8 @@ internal sealed class ChargenPreviewController :
private readonly IChargenClothingTableSource _clothingTables;
private readonly object _datLock;
private readonly bool _useZoomedOutEye;
private readonly uint _renderId;
private readonly uint _backdropRenderId;
private readonly Stopwatch _clock = Stopwatch.StartNew();
private ChargenPreviewAnimator? _animator;
@ -228,7 +230,19 @@ internal sealed class ChargenPreviewController :
IChargenPalSetSource palSets,
IChargenClothingTableSource clothingTables,
object datLock,
bool useZoomedOutEye = false)
bool useZoomedOutEye = false,
// F16 (Campaign CC gate round 1 closeout): the render-id pair this
// controller stamps on the entities it builds — MUST match the
// pair the sibling ChargenPreviewRenderer was constructed with (see
// that class's own renderId/backdropRenderId parameters), since
// both feed the SAME shared TextureCache owner-tracking key.
// Defaults to the Appearance page's pair; the composition root
// passes the Summary pair explicitly for its own instance — see
// ChargenPreviewEntityBuilder.SummaryPreviewRenderId's own doc for
// why sharing the default here would be a real collision, not
// merely untidy.
uint renderId = ChargenPreviewEntityBuilder.PreviewRenderId,
uint backdropRenderId = ChargenPreviewEntityBuilder.PreviewBackdropRenderId)
{
_renderer = renderer ?? throw new ArgumentNullException(nameof(renderer));
_camera = camera ?? throw new ArgumentNullException(nameof(camera));
@ -239,6 +253,8 @@ internal sealed class ChargenPreviewController :
_clothingTables = clothingTables ?? throw new ArgumentNullException(nameof(clothingTables));
_datLock = datLock ?? throw new ArgumentNullException(nameof(datLock));
_useZoomedOutEye = useZoomedOutEye;
_renderId = renderId;
_backdropRenderId = backdropRenderId;
_rotation = new ChargenPreviewRotationController();
// Seed the eye NOW, matching whatever the first Rebuild's own
// heritageOrGenderChanged branch below would otherwise defer until
@ -298,7 +314,7 @@ internal sealed class ChargenPreviewController :
Quaternion heading = MoveToMath.SetHeading(
Quaternion.Identity, _rotation.HeadingDegrees);
ChargenPreviewAnimatedBuild? build = ChargenPreviewEntityBuilder.TryBuildAnimated(
_dats, _animations, result, heritageId, heading, _datLock);
_dats, _animations, result, heritageId, heading, _datLock, _renderId);
if (build is null)
return false;
@ -335,7 +351,7 @@ internal sealed class ChargenPreviewController :
WorldEntity? backdrop =
options.TryGetHeritage(heritageId, out ChargenHeritageOptions? heritage)
? ChargenPreviewEntityBuilder.TryBuildBackdrop(
_dats, heritage!.EnvironmentSetupId, _datLock)
_dats, heritage!.EnvironmentSetupId, _datLock, _backdropRenderId)
: null;
_renderer.SetBackdrop(backdrop);
}

View file

@ -117,6 +117,37 @@ internal static class ChargenPreviewEntityBuilder
/// member (<c>gmCG3DView::m_pbgObject</c>).</summary>
public const uint PreviewBackdropRenderId = 0xDA11_D034u;
/// <summary>
/// F16 (Campaign CC gate round 1 closeout, 2026-08-16): the Summary
/// page's OWN preview render-local id — DISTINCT from
/// <see cref="PreviewRenderId"/>. Both the Appearance and Summary pages
/// construct their own <c>ChargenPreviewRenderer</c>, but they share
/// ONE process-wide <c>TextureCache</c> (<c>Wb.IEntityTextureLifetime</c>)
/// via <c>LivePresentationComposition</c>'s <c>foundation.TextureCache</c>
/// — confirmed by tracing <c>FixedEntityTextureOwnerLease.Replace</c> →
/// <c>TextureCache.ReleaseOwner</c> → <c>CompositeTextureArrayCache.ReleaseOwner</c>
/// → its own <c>_owners</c> tracker, keyed ONLY by the raw
/// <c>ownerLocalId</c> uint with no per-renderer namespace. Both pages
/// are mounted as PERMANENT siblings (register AP-229) and can be
/// simultaneously live, so two <c>PrivateEntityViewportRenderer</c>
/// instances sharing <see cref="PreviewRenderId"/> would share this
/// SAME owner bucket: either page re-dressing its own entity (a
/// <c>FixedEntityTextureOwnerLease.Replace</c> call) or being disposed
/// would call <c>ReleaseOwner(PreviewRenderId)</c> and release textures
/// the OTHER page's preview is still actively drawing with — a real
/// cross-page texture-corruption path, not a theoretical one. Reserved
/// in the SAME 0xDA11D0xx synthetic family, next free slot after the
/// Appearance page's own pair.
/// </summary>
public const uint SummaryPreviewRenderId = 0xDA11_D035u;
/// <summary>F16: the Summary page's own backdrop render-local id,
/// paired with <see cref="SummaryPreviewRenderId"/> exactly as
/// <see cref="PreviewBackdropRenderId"/> pairs with
/// <see cref="PreviewRenderId"/> — see that constant's own doc for why a
/// distinct id is required, not merely tidy.</summary>
public const uint SummaryPreviewBackdropRenderId = 0xDA11_D036u;
/// <summary>
/// Retail's held-pose (REST) animation DID enum key, resolved through
/// master map slot 7 exactly like <c>RetailPaperdollPoseApplicator.ResolvePoseDid</c>
@ -188,10 +219,11 @@ internal static class ChargenPreviewEntityBuilder
ChargenAppearanceResult appearance,
uint heritageId,
Quaternion heading,
object datLock)
object datLock,
uint renderId = PreviewRenderId)
{
ChargenPreviewAnimatedBuild? build = TryBuildAnimated(
dats, animations, appearance, heritageId, heading, datLock);
dats, animations, appearance, heritageId, heading, datLock, renderId);
if (build is null)
return null;
@ -214,7 +246,13 @@ internal static class ChargenPreviewEntityBuilder
ChargenAppearanceResult appearance,
uint heritageId,
Quaternion heading,
object datLock)
object datLock,
// F16 (Campaign CC gate round 1 closeout): the Appearance and
// Summary pages both call this method through their own
// ChargenPreviewController, but must NOT stamp the same Id on
// both entities — see SummaryPreviewRenderId's own doc for the
// full TextureCache collision trace this id also feeds.
uint renderId = PreviewRenderId)
{
ArgumentNullException.ThrowIfNull(dats);
ArgumentNullException.ThrowIfNull(animations);
@ -292,7 +330,7 @@ internal static class ChargenPreviewEntityBuilder
var entity = new WorldEntity
{
Id = PreviewRenderId,
Id = renderId,
ServerGuid = PreviewServerGuid,
SourceGfxObjOrSetupId = setupId,
Position = Vector3.Zero,
@ -363,7 +401,11 @@ internal static class ChargenPreviewEntityBuilder
public static WorldEntity? TryBuildBackdrop(
IDatReaderWriter dats,
uint environmentSetupId,
object datLock)
object datLock,
// F16 (Campaign CC gate round 1 closeout): see TryBuildAnimated's
// own renderId parameter doc — same Appearance-vs-Summary
// distinction, applied to the backdrop entity.
uint renderId = PreviewBackdropRenderId)
{
ArgumentNullException.ThrowIfNull(dats);
ArgumentNullException.ThrowIfNull(datLock);
@ -389,7 +431,7 @@ internal static class ChargenPreviewEntityBuilder
return new WorldEntity
{
Id = PreviewBackdropRenderId,
Id = renderId,
ServerGuid = PreviewBackdropServerGuid,
SourceGfxObjOrSetupId = environmentSetupId,
Position = Vector3.Zero,

View file

@ -80,7 +80,17 @@ internal sealed class ChargenPreviewRenderer :
IEntityTextureLifetime textureLifetime,
IWbMeshAdapter meshAdapter,
uint heritageId = 0u,
ChargenPreviewCamera? camera = null)
ChargenPreviewCamera? camera = null,
// F16 (Campaign CC gate round 1 closeout): the Appearance and
// Summary pages each construct their OWN ChargenPreviewRenderer but
// share ONE process-wide TextureCache — see
// ChargenPreviewEntityBuilder.SummaryPreviewRenderId's own doc for
// the full collision trace. Defaulting to the Appearance page's
// pair keeps every pre-existing call site byte-identical; the
// composition root passes the Summary pair explicitly for its own
// instance.
uint renderId = ChargenPreviewEntityBuilder.PreviewRenderId,
uint backdropRenderId = ChargenPreviewEntityBuilder.PreviewBackdropRenderId)
{
// CC6b-MOUNT: when a caller supplies its own camera instance (the
// page-mount composition, which needs a SETTABLE camera for
@ -98,13 +108,13 @@ internal sealed class ChargenPreviewRenderer :
lightUbo,
textureLifetime,
meshAdapter,
ChargenPreviewEntityBuilder.PreviewRenderId,
renderId,
_camera,
"chargen preview",
// Batch D (GF-7/GF-14): reserves the second draw-entity slot for
// the heritage's environment Setup — see PrivateEntityViewportRenderer's
// own doc comment on backdropRenderId.
ChargenPreviewEntityBuilder.PreviewBackdropRenderId);
backdropRenderId);
}
public bool TextureIsBottomUp => _renderer.TextureIsBottomUp;

View file

@ -140,6 +140,21 @@ internal sealed class PrivateEntityViewportRenderer :
? new EntitySlot(_meshAdapter, textureLifetimeChecked, backdropId, _diagnosticName + " backdrop")
: null;
// F14 (Campaign CC gate round 1 closeout): this set is built ONCE
// here, from the RESERVED backdropRenderId (a renderer either has a
// backdrop slot or it doesn't — see _backdropSlot's own doc), not
// from whether a backdrop ENTITY is currently set via
// SetBackdrop/BuildDrawEntities. That is deliberately harmless, not
// an oversight: BuildDrawEntities below already degrades to
// [main] alone whenever the backdrop slot is null or has no
// meshes, so animatedEntityIds carrying a backdrop id with no
// matching entry in THIS frame's actual draw-entities list is a
// pure dead lookup (WbDrawDispatcher.Draw only ever consults this
// set against ids it is ACTUALLY drawing) — never a wrong-entity
// animation flag, never extra per-frame work beyond one inert
// HashSet entry. Recomputing per-frame would add real complexity
// (a second HashSet allocation or a mutable-set sync path) for a
// case that is already correct by construction.
_animatedIds = backdropRenderId is uint animatedBackdropId
? [renderId, animatedBackdropId]
: [renderId];