Skip to content

Commit a77c3c8

Browse files
committed
fix(combobulate): make SetExternalRotation idempotent + diagnostic API
Root cause: every slider tick re-installed the composition expression animation chain on _root.TransformMatrix and on the property-set buffer. That re-install costs one composition tick before _root.TransformMatrix shows the new value, but the auto-refresh sampler reads the property set immediately and runs cull/sort with the new rotation. The result is a one-frame transform-vs-cull desync. Tiny slider steps mask it; large jumps make the prior-frame matrix grossly visible. Fix: SetExternalRotation now no-ops when the same ExpressionAnimation instance is reinstalled (in both Combobulate and CombobulateSceneVisual). The property set is the only thing that needs updating per tick; the expression already references it. Also adds diagnostic API for future investigation: InvalidateRenderCaches, GetRenderCacheSnapshot, GetActualChildrenOrder, GetLiveSpriteVisibility, plus GetState/ForceRebuild/SetToggles rover actions and a per-rotation log. Version 1.0.14.0 of sample MSIX.
1 parent 5f68260 commit a77c3c8

5 files changed

Lines changed: 192 additions & 1 deletion

File tree

src/Combobulate.Sample.WinUI3/MainWindow.Actions.cs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,24 @@ public sealed partial class MainWindow : zRover.Core.IActionableApp
4141
name: "ResetRotation",
4242
description: "Resets all three rotation axes to 0.",
4343
parameterSchema: @"{""type"":""object"",""properties"":{}}"),
44+
new ActionDescriptor(
45+
name: "GetState",
46+
description: "Returns the current viewer state (rotation X/Y/Z degrees, zoom, external-rotation toggle, auto-refresh toggle, current Source) PLUS a snapshot of the sprite renderer's per-frame cull/order caches (visible[], order[]), the live _root.Children stack, and the live SpriteVisual.IsVisible flags. Returned via the ActionResult.Consequences string list as 'key=value' lines so a caller can correlate a screenshot with the exact internal state that produced it.",
47+
parameterSchema: @"{""type"":""object"",""properties"":{}}"),
48+
new ActionDescriptor(
49+
name: "ForceRebuild",
50+
description: "Diagnostic: invalidates the sprite renderer's per-frame cull/order skip caches (_lastVisible, _lastOrder) and triggers a full rebuild against the current rotation. Use this when the view looks broken: if it heals after this call, the bug is in the skip caches; if not, the bug is elsewhere (rotation matrix, expression animation, painter algorithm).",
51+
parameterSchema: @"{""type"":""object"",""properties"":{}}"),
52+
new ActionDescriptor(
53+
name: "SetToggles",
54+
description: "Sets the ExternalRotation and AutoRefresh toggle switches. Either or both may be omitted to keep current. Use this to script the exact mode used to reproduce a rotation-update bug.",
55+
parameterSchema: @"{
56+
""type"": ""object"",
57+
""properties"": {
58+
""externalRotation"": { ""type"": ""boolean"" },
59+
""autoRefresh"": { ""type"": ""boolean"" }
60+
}
61+
}"),
4462
new ActionDescriptor(
4563
name: "ResetCube",
4664
description: "Reloads the built-in cube model.",
@@ -79,6 +97,14 @@ public Task<ActionResult> DispatchAsync(string actionName, string parametersJson
7997
case "LoadObjFromPath": return DispatchLoadObjFromPathAsync(parametersJson);
8098
case "SetRotation": return DispatchSetRotationAsync(parametersJson);
8199
case "ResetRotation": return RunOnUi(() => { PitchSlider.Value = 0; YawSlider.Value = 0; RollSlider.Value = 0; });
100+
case "GetState": return DispatchGetStateAsync();
101+
case "ForceRebuild": return RunOnUi(() =>
102+
{
103+
combobulate.InvalidateRenderCaches();
104+
var rot = new System.Numerics.Vector3((float)PitchSlider.Value, (float)YawSlider.Value, (float)RollSlider.Value);
105+
combobulate.RebuildForExternalRotation(rot);
106+
});
107+
case "SetToggles": return DispatchSetTogglesAsync(parametersJson);
82108
case "ResetCube": return RunOnUi(LoadCube);
83109
case "SetZoom": return DispatchSetZoomAsync(parametersJson);
84110
case "SetMaterial": return DispatchSetMaterialAsync(parametersJson);
@@ -160,6 +186,74 @@ private Task<ActionResult> DispatchSetMaterialAsync(string parametersJson)
160186
});
161187
}
162188

189+
private Task<ActionResult> DispatchSetTogglesAsync(string parametersJson)
190+
{
191+
JObject p;
192+
try { p = JObject.Parse(parametersJson); }
193+
catch { return Task.FromResult(ActionResult.Fail("validation_error", "params is not valid JSON.")); }
194+
195+
return RunOnUi(() =>
196+
{
197+
if (p["externalRotation"] != null && ExternalRotationToggle != null)
198+
ExternalRotationToggle.IsOn = p["externalRotation"]!.Value<bool>();
199+
if (p["autoRefresh"] != null && AutoRefreshToggle != null)
200+
AutoRefreshToggle.IsOn = p["autoRefresh"]!.Value<bool>();
201+
});
202+
}
203+
204+
private Task<ActionResult> DispatchGetStateAsync()
205+
{
206+
var tcs = new TaskCompletionSource<ActionResult>();
207+
var dq = this.DispatcherQueue;
208+
if (!dq.TryEnqueue(DispatcherQueuePriority.Normal, () =>
209+
{
210+
try
211+
{
212+
var (cacheVisible, cacheOrder) = combobulate.GetRenderCacheSnapshot();
213+
var actualOrder = combobulate.GetActualChildrenOrder();
214+
var liveVisible = combobulate.GetLiveSpriteVisibility();
215+
var lines = new List<string>
216+
{
217+
$"rotationX={PitchSlider.Value:F3}",
218+
$"rotationY={YawSlider.Value:F3}",
219+
$"rotationZ={RollSlider.Value:F3}",
220+
$"zoom={combobulate.ModelScale:F3}",
221+
$"externalRotation={(ExternalRotationToggle?.IsOn == true)}",
222+
$"autoRefresh={(AutoRefreshToggle?.IsOn == true)}",
223+
$"perspective={combobulate.EnablePerspective}",
224+
$"source={combobulate.Source ?? "(null)"}",
225+
$"cache.visible={string.Join(",", cacheVisible)}",
226+
$"cache.order={string.Join(",", cacheOrder)}",
227+
$"children.actual={string.Join(",", actualOrder)}",
228+
$"sprite.visible={string.Join(",", liveVisible)}",
229+
};
230+
tcs.SetResult(ActionResult.Ok(lines));
231+
}
232+
catch (Exception ex) { tcs.SetResult(ActionResult.Fail("execution_error", ex.Message)); }
233+
}))
234+
{
235+
tcs.SetResult(ActionResult.Fail("execution_error", "Failed to enqueue UI work."));
236+
}
237+
return tcs.Task;
238+
}
239+
240+
/// <summary>
241+
/// Continuous diagnostic log: writes the current rotation triple to the rover
242+
/// log every time ApplyRotation runs so a tail of the diagnostic log shows
243+
/// the live angle history.
244+
/// </summary>
245+
internal void LogCurrentRotation()
246+
{
247+
try
248+
{
249+
zRover.WinUI.RoverMcp.Log(
250+
"Combobulate",
251+
$"rotation x={PitchSlider.Value:F2} y={YawSlider.Value:F2} z={RollSlider.Value:F2} " +
252+
$"external={ExternalRotationToggle?.IsOn == true} auto={AutoRefreshToggle?.IsOn == true}");
253+
}
254+
catch { /* logger is best-effort */ }
255+
}
256+
163257
private Task<ActionResult> RunOnUi(Action action)
164258
{
165259
var tcs = new TaskCompletionSource<ActionResult>();

src/Combobulate.Sample.WinUI3/MainWindow.xaml.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,9 @@ private void ApplyRotation()
156156
combobulate.RotationZ = z;
157157
// combobulateSceneVisual mirrors combobulate's rotation via x:Bind.
158158
}
159+
#if DEBUG
160+
LogCurrentRotation();
161+
#endif
159162
}
160163

161164
private async void LoadObjButton_Click(object sender, RoutedEventArgs e)

src/Combobulate.Sample.WinUI3/Package.appxmanifest

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<Identity
1010
Name="Combobulate.Sample.WinUI3"
1111
Publisher="CN=Dev"
12-
Version="1.0.9.0" />
12+
Version="1.0.14.0" />
1313

1414
<mp:PhoneIdentity PhoneProductId="b0b0b0b0-b0b0-b0b0-b0b0-b0b0b0b0b001" PhonePublisherId="00000000-0000-0000-0000-000000000000"/>
1515

src/Combobulate/Combobulate.cs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,13 @@ private void TryAttachVisuals()
311311
public void SetExternalRotation(ExpressionAnimation rotationDegrees)
312312
{
313313
if (rotationDegrees is null) throw new ArgumentNullException(nameof(rotationDegrees));
314+
// Idempotent: re-installing the same expression every slider tick was
315+
// tearing down and rebuilding the composition animation chain on the
316+
// root, which causes a one-frame lag where _root.TransformMatrix is
317+
// still showing the previous matrix while the cull/sort run on the
318+
// freshly-pushed property-set value. The visible symptom is a quad
319+
// jump that looks "wrong" until the next slider tick recovers.
320+
if (ReferenceEquals(_externalRotationExpression, rotationDegrees)) return;
314321
_externalRotationExpression = rotationDegrees;
315322
TryStartExternalRotationAnimation();
316323
}
@@ -656,6 +663,75 @@ public void EnableAutoRefresh(Func<Vector3> rotationSampler)
656663
#endif
657664
}
658665

666+
/// <summary>
667+
/// Diagnostic helper: forces the next rebuild to re-emit IsVisible flags and
668+
/// child-order updates for every quad, regardless of the per-frame skip
669+
/// optimisations (<c>_lastVisible</c> / <c>_lastOrder</c>). If a known-broken
670+
/// view heals after calling this, the bug lives in the skip caches.
671+
/// </summary>
672+
public void InvalidateRenderCaches()
673+
{
674+
if (_lastVisible != null)
675+
for (int i = 0; i < _lastVisible.Length; i++) _lastVisible[i] = false;
676+
_lastOrder = Array.Empty<int>();
677+
}
678+
679+
/// <summary>
680+
/// Diagnostic snapshot of the per-frame skip caches. Returns the visible
681+
/// flag for every quad (in cached-quad index order) and the painter order
682+
/// (sequence of cached-quad indices, back to front). UI thread only.
683+
/// </summary>
684+
public (bool[] visible, int[] order) GetRenderCacheSnapshot()
685+
{
686+
var v = _lastVisible == null ? Array.Empty<bool>() : (bool[])_lastVisible.Clone();
687+
var o = (int[])_lastOrder.Clone();
688+
return (v, o);
689+
}
690+
691+
/// <summary>
692+
/// Diagnostic: returns the actual <c>_root.Children</c> stack as cached-quad
693+
/// indices, bottom to top. This is the ground truth of what compositor will
694+
/// render and includes hidden sprites still parented in the collection.
695+
/// Compare against <see cref="GetRenderCacheSnapshot"/>.order to confirm the
696+
/// visible subset's relative ordering matches what the painter sort decided.
697+
/// </summary>
698+
public int[] GetActualChildrenOrder()
699+
{
700+
if (_root == null || _spritePool == null) return Array.Empty<int>();
701+
var children = _root.Children;
702+
var pool = _spritePool;
703+
var result = new int[children.Count];
704+
int k = 0;
705+
foreach (var child in children)
706+
{
707+
int idx = -1;
708+
for (int i = 0; i < pool.Length; i++)
709+
{
710+
if (ReferenceEquals(pool[i], child)) { idx = i; break; }
711+
}
712+
result[k++] = idx;
713+
}
714+
return result;
715+
}
716+
717+
/// <summary>
718+
/// Diagnostic: returns the LIVE <c>SpriteVisual.IsVisible</c> for every pooled
719+
/// sprite (in cached-quad index order). Compare against
720+
/// <see cref="GetRenderCacheSnapshot"/>.visible to detect a divergence between
721+
/// what the cull code thinks it pushed and what the sprite property actually holds.
722+
/// </summary>
723+
public bool[] GetLiveSpriteVisibility()
724+
{
725+
if (_spritePool == null) return Array.Empty<bool>();
726+
var result = new bool[_spritePool.Length];
727+
for (int i = 0; i < _spritePool.Length; i++)
728+
{
729+
var s = _spritePool[i];
730+
result[i] = s != null && s.IsVisible;
731+
}
732+
return result;
733+
}
734+
659735
/// <summary>Stops the auto-refresh loop installed by <see cref="EnableAutoRefresh"/>.</summary>
660736
public void DisableAutoRefresh()
661737
{

src/Combobulate/CombobulateSceneVisual.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,10 @@ private void UpdateRootLayout()
381381
public void SetExternalRotation(ExpressionAnimation rotationDegrees)
382382
{
383383
if (rotationDegrees is null) throw new ArgumentNullException(nameof(rotationDegrees));
384+
// See Combobulate.SetExternalRotation: re-installing the same expression
385+
// every slider tick re-creates the composition animation chain and
386+
// produces a one-frame transform lag. Idempotent install fixes that.
387+
if (ReferenceEquals(_externalRotationExpression, rotationDegrees)) return;
384388
_externalRotationExpression = rotationDegrees;
385389
TryStartExternalRotationAnimation();
386390
}
@@ -515,6 +519,20 @@ private void DisposeSceneTree()
515519
public void RebuildForExternalRotation(Vector3 rotationDegrees)
516520
{
517521
RebuildMesh(rotationDegrees);
522+
// The mesh now has the full rotation baked into its vertices. If
523+
// SetExternalRotation also installed a 2D affine TransformMatrix
524+
// animation against the rasterised surface, that animation would
525+
// double-apply on top of the baked rotation (visually: the right-
526+
// hand SceneVisual gets rotated/skewed twice). Stop the surface
527+
// animation and reset to identity so the baked mesh is the sole
528+
// source of truth. The expression buffer ("R") is left running
529+
// so callers can still drive the next rebake through the same
530+
// property set without re-installing.
531+
if (_surfaceSprite != null && _externalRotationExpression != null)
532+
{
533+
_surfaceSprite.StopAnimation("TransformMatrix");
534+
_surfaceSprite.TransformMatrix = Matrix4x4.Identity;
535+
}
518536
}
519537

520538
private void RebuildMesh() => RebuildMesh(rotationOverride: null);

0 commit comments

Comments
 (0)