Repository: pygfx/pygfx
Pull request #589 (2023-09-13 12:28:08)

Track tuples better
### Introduction

I ran into very strange behavior where the `draw_range` was changed, but did not affect the rendering, causing glitches in the mesh that was being modified.

### The cause

Took me a deep dive to find out that the problem is this:
```py
# At some point in time ...
buf.draw_range = (0, 400)

# At another point in time, in one event-handler cycle ...

buf.draw_range = (0, 380)  # at this point, the previous tuple is de-allocated, and a new one is allocated
...
buf.draw_range = (0, 410)  # same here
```

What I observed was that, in some cases, the last two changes do not register a change of `draw_range` because the `id` of the value did not change. So it seems like when that last tuple is allocated, it may re-use the memory where the original tuple was stored, causing it to have the same id, so it appears as though the object is set back to its original.

### Solution

This PR changes the trackable code so it can deal with tuples. This is a nice improvement anyhow, because we use tuples in quite a few places. It also means that it compares the actual value, resulting in expected behavior when a property is set to the same value.

  


Patch:
diff --git a/pygfx/utils/trackable.py b/pygfx/utils/trackable.py
index 87d0cfe..093dfa3 100644
--- a/pygfx/utils/trackable.py
+++ b/pygfx/utils/trackable.py
@@ -74,6 +74,8 @@ simple_value_types = None.__class__, bool, int, float, str
 def get_comp_value(value):
     if isinstance(value, simple_value_types):
         return value
+    elif isinstance(value, tuple):
+        return tuple(get_comp_value(v) for v in value)
     else:
         return f"id:{id(value)}"
 


Test patch:
diff --git a/tests/utils/test_trackable.py b/tests/utils/test_trackable.py
index 9853770..dabd9f3 100644
--- a/tests/utils/test_trackable.py
+++ b/tests/utils/test_trackable.py
@@ -133,6 +133,91 @@ def test_changes_on_sub_sub_sub():
     make_changes_on_sub(root, root.sub2.sub1, t1, t2)
 
 
+def test_list_values():
+    rt = MyRootTrackable()
+
+    v1 = [1, 2]
+    v2 = [3, 4]
+    v3 = [1, 2]
+
+    with rt.track_usage("foo"):
+        rt.foo
+    assert rt.pop_changed() == set()
+
+    # Changing the value marks a change
+    rt.foo = v1
+    assert rt.pop_changed() == {"foo"}
+    rt.foo = v2
+    assert rt.pop_changed() == {"foo"}
+
+    # Even if the value is the same - it tracks the object!
+    rt.foo = v1
+    assert rt.pop_changed() == {"foo"}
+    rt.foo = v3
+    assert rt.pop_changed() == {"foo"}
+
+    # This means that ...
+    v3.append(8)
+    assert rt.foo == v3  # changed in-place
+    rt.foo = v3  # so this does not do much
+    assert rt.pop_changed() == set()
+
+    # It can even mean that the second assert below fails, because when
+    # that last list is allocated, it may use the memory previously
+    # occupied by [0, 1], so it has the same id ... This is hard to
+    # test reliably, but it *can* happen.
+    rt.foo = [0, 1]
+    assert rt.pop_changed() == {"foo"}
+    rt.foo = [0, 2]  # on this line, the previous list is freed
+    rt.foo = [0, 3]  # a new list object is allocated
+    # assert rt.pop_changed() == {"foo"}
+
+
+def test_tuple_values():
+    rt = MyRootTrackable()
+
+    offset = 0
+
+    v1 = (1, offset + 2)
+    v2 = (3, offset + 4)
+    v3 = (1, offset + 2)
+
+    assert v1 is not v3
+
+    with rt.track_usage("foo"):
+        rt.foo
+    assert rt.pop_changed() == set()
+
+    # Changing the value marks a change
+    rt.foo = v1
+    assert rt.pop_changed() == {"foo"}
+    rt.foo = v2
+    assert rt.pop_changed() == {"foo"}
+
+    # If the object is different, but the value is the same, it checks the value!
+    rt.foo = v1
+    assert rt.pop_changed() == {"foo"}
+    rt.foo = v3
+    assert rt.pop_changed() == set()
+
+    # This means that everything works as expected
+    v3 += (8,)
+    assert rt.foo != v3
+    rt.foo = v3
+    assert rt.pop_changed() == {"foo"}
+
+    # So does this
+    rt.foo = (0, offset + 1)
+    assert rt.pop_changed() == {"foo"}
+    rt.foo = (0, offset + 2)
+    rt.foo = (0, 0 + 3)
+    assert rt.pop_changed() == {"foo"}
+    rt.foo = (0, offset + 4)
+    rt.foo = (0, offset + 2)
+    rt.foo = (0, offset + 3)
+    assert rt.pop_changed() == set()
+
+
 def make_changes_on_sub(root, parent, t1, t2):
     parent.sub1 = t1