Skip to content

Commit ee4259f

Browse files
committed
fix(combobulate): VisualCollection reorder + deferred Source resolve
VisualCollection.InsertAtTop throws E_INVALIDARG when the visual already has a parent. The v0.5.0 sprite-pool reorder loop assumed it would re-position existing children in place; instead the renderer must Remove(sprite) before InsertAtTop. Without this fix Combobulate crashes during the first painter sort that hits a non-empty visible set. OnSourceChanged now defers resolution failures to the next OnLoaded, so declarative XAML markup like Source="key" still works when the consumer registers the keyed cache from code-behind right after InitializeComponent. Same change in CombobulateSceneVisual. Sample manifest bumped to 1.0.9.0.
1 parent 205235f commit ee4259f

3 files changed

Lines changed: 45 additions & 9 deletions

File tree

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.8.0" />
12+
Version="1.0.9.0" />
1313

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

src/Combobulate/Combobulate.cs

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,20 @@ protected override void OnApplyTemplate()
216216
TryAttachVisuals();
217217
}
218218

219-
private void OnLoaded(object sender, RoutedEventArgs e) => TryAttachVisuals();
219+
private void OnLoaded(object sender, RoutedEventArgs e)
220+
{
221+
TryAttachVisuals();
222+
// If a Source set in XAML failed to resolve during construction (e.g. the
223+
// app registered the keyed cache entry from code-behind AFTER InitializeComponent
224+
// returned), retry now that we are loaded and any code-behind initialisation has
225+
// had a chance to run.
226+
if (_pendingSource != null)
227+
{
228+
var key = _pendingSource;
229+
_pendingSource = null;
230+
OnSourceChanged(key);
231+
}
232+
}
220233

221234
private void OnUnloaded(object sender, RoutedEventArgs e)
222235
{
@@ -480,14 +493,18 @@ private void OnSourceChanged(string? newValue)
480493
}
481494
catch (Exception)
482495
{
483-
// Surface failures by clearing the model. Callers can listen to Model changes
484-
// or check ObjCache directly for richer diagnostics.
496+
// Resolution can fail when XAML sets Source before code-behind has had a
497+
// chance to register the keyed cache entry (Source="key" attribute is
498+
// applied during InitializeComponent). Stash the value and retry from
499+
// OnLoaded once user code has run.
485500
_sourceKey = null;
486501
_sourceDirectory = null;
487502
_geometry = null;
503+
_pendingSource = newValue;
488504
Model = null;
489505
return;
490506
}
507+
_pendingSource = null;
491508

492509
_sourceKey = newValue;
493510
try
@@ -519,6 +536,7 @@ private void OnSourceChanged(string? newValue)
519536
private ObjGeometry? _geometry;
520537
private string? _sourceKey;
521538
private string? _sourceDirectory;
539+
private string? _pendingSource;
522540

523541
// --- Per-instance render-state cache, keyed off the active geometry. -----
524542
// Sprites are created once per quad and reused across rebuilds; rotation only
@@ -806,14 +824,17 @@ private void Rebuild(Matrix4x4 rotation)
806824
var order = TopologicalPainterSort(visibleIndices, visCount, cachedQuads, geometry.Predecessors, rotation, _slotScratch!);
807825

808826
// Reorder children only if the painter order actually changed. Sprites already
809-
// live under _root; InsertAtTop on an existing child is a sibling-reorder, not a
810-
// new attach. Walking visible quads back-to-front and InsertAtTop'ing each one
811-
// leaves the last-painted (frontmost) on top.
827+
// live under _root; VisualCollection.InsertAtTop throws E_INVALIDARG when the
828+
// child still has a parent (even if it's the same parent), so we must Remove
829+
// first. Walking visible quads back-to-front and re-attaching each one leaves
830+
// the last-painted (frontmost) on top.
812831
if (!OrderEquals(order, _lastOrder))
813832
{
814833
for (int i = 0; i < order.Length; i++)
815834
{
816-
_root.Children.InsertAtTop(pool[order[i]]!);
835+
var sprite = pool[order[i]]!;
836+
_root.Children.Remove(sprite);
837+
_root.Children.InsertAtTop(sprite);
817838
}
818839
_lastOrder = order;
819840
}

src/Combobulate/CombobulateSceneVisual.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ public sealed class CombobulateSceneVisual : Control
7777
private SceneVisual? _sceneVisual;
7878
private SceneNode? _modelNode;
7979
private ObjGeometry? _geometry;
80+
private string? _pendingSource;
8081

8182
public CombobulateSceneVisual()
8283
{
@@ -261,7 +262,16 @@ protected override void OnApplyTemplate()
261262
TryAttachVisuals();
262263
}
263264

264-
private void OnLoaded(object sender, RoutedEventArgs e) => TryAttachVisuals();
265+
private void OnLoaded(object sender, RoutedEventArgs e)
266+
{
267+
TryAttachVisuals();
268+
if (_pendingSource != null)
269+
{
270+
var key = _pendingSource;
271+
_pendingSource = null;
272+
OnSourceChanged(key);
273+
}
274+
}
265275

266276
private void OnUnloaded(object sender, RoutedEventArgs e)
267277
{
@@ -453,10 +463,15 @@ private void OnSourceChanged(string? newValue)
453463
}
454464
catch (Exception)
455465
{
466+
// See Combobulate.OnSourceChanged: defer retry until OnLoaded so XAML
467+
// Source="key" applied during InitializeComponent still works when the
468+
// app registers the keyed cache from code-behind right after.
456469
_geometry = null;
470+
_pendingSource = newValue;
457471
Model = null;
458472
return;
459473
}
474+
_pendingSource = null;
460475

461476
_geometry = geometry;
462477
if (!ReferenceEquals(Model, geometry.Model))

0 commit comments

Comments
 (0)