[Bugfix] Only UICollection.append maintains the parent link; insert/extend/setitem/slice do not #377

New issue

Closed

opened 2026-07-14 11:44:01 +00:00 by john · 0 comments

john commented

2026-07-14 11:44:01 +00:00

Owner

Found while implementing #373, which needs a trustworthy "is this drawable in a children collection?" signal.

UIDrawable::parent / parent_scene (#122/#183) are supposed to be the inverse of collection membership. Only append() actually maintains that invariant. Every other mutator gets it wrong, in two distinct ways.

1. Scene collections: only `append` sets `parent_scene`

append() branches on self->scene_name:

if (!self->scene_name.empty()) drawable->setParentScene(self->scene_name);
else                           drawable->setParent(self->owner.lock());

insert(), extend(), and setitem() skip the branch and call setParent(self->owner.lock()) unconditionally. A scene collection has no owner drawable, so owner.lock() is null — and setParent(nullptr) also clears parent_scene. The drawable lands in the scene's UI vector with no parent at all:

scene.children.insert(0, b)   # b IS rendered, b IS in scene.children
b.parent                      # -> None          (should be the Scene)

Reproduced on fix/bugfix-batch-july:

append   -> a.parent: <Scene 's'>   | in scene: True
insert   -> b.parent: None          | in scene: True
extend   -> c.parent: None          | in scene: True
setitem  -> d.parent: None          | in scene: True

2. Slice ops ignore the parent link entirely

ass_subscript() erases and inserts straight into the vector, never calling setParent/removeFromParent:

f.children.append(a); f.children.append(b)
del f.children[0:2]
len(f.children)   # 0
a.parent is f     # True  <-- stale: a is not in f.children anymore

f.children[0:1] = [d]
d.parent          # None   <-- never linked
c.parent          # f      <-- replaced element never unlinked

So after a slice op, x.parent.children may not contain x, and f.children may hold elements whose .parent is someone else. A stale parent is a live weak_ptr to a frame the child is no longer in, which is also what get_global_position() walks.

Fix

Factor the append branch into link_child(self, drawable) / unlink_child(drawable) helpers and route every mutator through them: setitem (both the replace and the delete arm), insert, extend, remove, pop, and both arms of ass_subscript.

Why this blocks #373

#373's owner-held strong ref pins the Python wrapper of a subclassed drawable exactly while it is a member of a children collection. The parent link is the natural representation of that membership — but a pin hung off a link that four of six mutators don't maintain would fail to pin on insert/extend/slice-assign, and fail to release on slice-delete (leaking the wrapper and the C++ object forever).

Test

tests/regression/ — assert the x in collection <-> x.parent is collection_owner invariant holds after every mutator, for both a Frame-owned and a Scene-owned collection.

Found while implementing #373, which needs a trustworthy "is this drawable in a children collection?" signal. `UIDrawable::parent` / `parent_scene` (#122/#183) are supposed to be the inverse of collection membership. Only `append()` actually maintains that invariant. Every other mutator gets it wrong, in two distinct ways. ## 1. Scene collections: only `append` sets `parent_scene` `append()` branches on `self->scene_name`: ```cpp if (!self->scene_name.empty()) drawable->setParentScene(self->scene_name); else drawable->setParent(self->owner.lock()); ``` `insert()`, `extend()`, and `setitem()` skip the branch and call `setParent(self->owner.lock())` unconditionally. A scene collection has **no owner drawable**, so `owner.lock()` is null — and `setParent(nullptr)` also clears `parent_scene`. The drawable lands in the scene's UI vector with no parent at all: ```python scene.children.insert(0, b) # b IS rendered, b IS in scene.children b.parent # -> None (should be the Scene) ``` Reproduced on `fix/bugfix-batch-july`: ``` append -> a.parent: <Scene 's'> | in scene: True insert -> b.parent: None | in scene: True extend -> c.parent: None | in scene: True setitem -> d.parent: None | in scene: True ``` ## 2. Slice ops ignore the parent link entirely `ass_subscript()` erases and inserts straight into the vector, never calling `setParent`/`removeFromParent`: ```python f.children.append(a); f.children.append(b) del f.children[0:2] len(f.children) # 0 a.parent is f # True <-- stale: a is not in f.children anymore f.children[0:1] = [d] d.parent # None <-- never linked c.parent # f <-- replaced element never unlinked ``` So after a slice op, `x.parent.children` may not contain `x`, and `f.children` may hold elements whose `.parent` is someone else. A stale `parent` is a live `weak_ptr` to a frame the child is no longer in, which is also what `get_global_position()` walks. ## Fix Factor the append branch into `link_child(self, drawable)` / `unlink_child(drawable)` helpers and route **every** mutator through them: `setitem` (both the replace and the delete arm), `insert`, `extend`, `remove`, `pop`, and both arms of `ass_subscript`. ## Why this blocks #373 #373's owner-held strong ref pins the Python wrapper of a subclassed drawable exactly while it is a member of a children collection. The parent link is the natural representation of that membership — but a pin hung off a link that four of six mutators don't maintain would fail to pin on `insert`/`extend`/slice-assign, and fail to *release* on slice-delete (leaking the wrapper and the C++ object forever). ## Test `tests/regression/` — assert the `x in collection` <-> `x.parent is collection_owner` invariant holds after every mutator, for both a Frame-owned and a Scene-owned collection.