From 0187abfafcc560f58a5a132c10ba64b454e376cd Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Sat, 30 Aug 2025 19:37:12 +0200 Subject: [PATCH] [consider merging] Bugfix: internal error when trying to replace a shape in a standalone Shapes collection while preserving properties --- src/db/db/dbShapes.cc | 24 +++++++++-- src/db/unit_tests/dbShapesTests.cc | 68 ++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/src/db/db/dbShapes.cc b/src/db/db/dbShapes.cc index ff2c1bfff..8d396730b 100644 --- a/src/db/db/dbShapes.cc +++ b/src/db/db/dbShapes.cc @@ -1441,8 +1441,10 @@ Shapes::replace_member_with_props (typename db::object_tag tag, const shape_ if (! layout ()) { if (needs_translate (tag)) { + return reinsert_member_with_props (tag, ref, sh); - } else { + + } else if (! ref.has_prop_id ()) { // simple replace case @@ -1459,7 +1461,21 @@ Shapes::replace_member_with_props (typename db::object_tag tag, const shape_ db::layer_op::queue_or_append (manager (), this, true /*insert*/, sh); } - return ref; + } else { + + if (manager () && manager ()->transacting ()) { + check_is_editable_for_undo_redo (); + db::layer_op, db::stable_layer_tag>::queue_or_append (manager (), this, false /*not insert*/, *ref.basic_ptr (typename db::object_with_properties::tag ())); + } + + invalidate_state (); // HINT: must come before the change is done! + + db::object_with_properties swp (sh, ref.prop_id ()); + get_layer, db::stable_layer_tag> ().replace (ref.basic_iter (typename db::object_with_properties::tag ()), swp); + + if (manager () && manager ()->transacting ()) { + db::layer_op, db::stable_layer_tag>::queue_or_append (manager (), this, true /*insert*/, swp); + } } @@ -1514,9 +1530,9 @@ Shapes::replace_member_with_props (typename db::object_tag tag, const shape_ } - return ref; - } + + return ref; } // explicit instantiations diff --git a/src/db/unit_tests/dbShapesTests.cc b/src/db/unit_tests/dbShapesTests.cc index fd584449e..7fb53898c 100644 --- a/src/db/unit_tests/dbShapesTests.cc +++ b/src/db/unit_tests/dbShapesTests.cc @@ -2404,6 +2404,10 @@ TEST(12A) db::Cell &topcell = layout.cell (*layout.begin_top_down ()); + // standalone copy + db::Shapes copy (true); + copy = topcell.shapes (lindex); + db::Shapes::shape_iterator shape = topcell.shapes (lindex).begin (db::Shapes::shape_iterator::All); while (! shape.at_end ()) { topcell.shapes (lindex).replace (*shape, db::Box (shape->box ().transformed (db::Trans (1)))); @@ -2436,6 +2440,70 @@ TEST(12A) "box (-1050,150;-150,2150) #112\n" ); + shape = topcell.shapes (lindex).begin (db::Shapes::shape_iterator::All); + while (! shape.at_end ()) { + topcell.shapes (lindex).replace (*shape, db::Box (shape->box ().transformed (db::Trans (1)))); + ++shape; + } + + EXPECT_EQ (shapes_to_string (_this, topcell.shapes (lindex)), + "box (-2000,-1000;0,-100) #0\n" + "box (-2100,-1100;-100,-200) #0\n" + "box (-2150,-1050;-150,-150) #0\n" + "box (-2000,-1000;0,-100) #110\n" + "box (-2100,-1100;-100,-200) #111\n" + "box (-2150,-1050;-150,-150) #112\n" + ); + + // on standalone shapes + + shape = copy.begin (db::Shapes::shape_iterator::All); + while (! shape.at_end ()) { + copy.replace (*shape, db::Box (shape->box ().transformed (db::Trans (1)))); + ++shape; + } + + EXPECT_EQ (shapes_to_string (_this, copy), + "box (-1000,0;-100,2000) #0\n" + "box (-1100,100;-200,2100) #0\n" + "box (-1050,150;-150,2150) #0\n" + "box (-1000,0;-100,2000) #10\n" + "box (-1100,100;-200,2100) #11\n" + "box (-1050,150;-150,2150) #12\n" + ); + + shape = copy.begin (db::Shapes::shape_iterator::All); + while (! shape.at_end ()) { + if (shape->has_prop_id ()) { + copy.replace_prop_id (*shape, shape->prop_id () + 100); + } + ++shape; + } + + EXPECT_EQ (shapes_to_string (_this, copy), + "box (-1000,0;-100,2000) #0\n" + "box (-1100,100;-200,2100) #0\n" + "box (-1050,150;-150,2150) #0\n" + "box (-1000,0;-100,2000) #110\n" + "box (-1100,100;-200,2100) #111\n" + "box (-1050,150;-150,2150) #112\n" + ); + + shape = copy.begin (db::Shapes::shape_iterator::All); + while (! shape.at_end ()) { + copy.replace (*shape, db::Box (shape->box ().transformed (db::Trans (1)))); + ++shape; + } + + EXPECT_EQ (shapes_to_string (_this, copy), + "box (-2000,-1000;0,-100) #0\n" + "box (-2100,-1100;-100,-200) #0\n" + "box (-2150,-1050;-150,-150) #0\n" + "box (-2000,-1000;0,-100) #110\n" + "box (-2100,-1100;-100,-200) #111\n" + "box (-2150,-1050;-150,-150) #112\n" + ); + } }