From c541cdcbd65b3c4ce775e7988859699c1473df5d Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Thu, 7 Sep 2017 01:04:40 +0200 Subject: [PATCH] Bugfix: redrawing issues when multiple layers are affected This is how to reproduce the bug: have a layout with two layers. Select two shapes of different layers and delete them. One layer is not updated and only after zooming/panning the shape will disappear on this layer. --- src/db/db/dbLayoutStateModel.cc | 43 +++- src/db/db/dbLayoutStateModel.h | 25 +-- src/db/unit_tests/dbLayout.cc | 262 +++++++++++++++++++++++ src/laybasic/laybasic/layLayoutCanvas.cc | 6 +- src/laybasic/laybasic/layLayoutView.cc | 2 +- 5 files changed, 313 insertions(+), 25 deletions(-) diff --git a/src/db/db/dbLayoutStateModel.cc b/src/db/db/dbLayoutStateModel.cc index ff02ebbd7..9465b7d70 100644 --- a/src/db/db/dbLayoutStateModel.cc +++ b/src/db/db/dbLayoutStateModel.cc @@ -23,17 +23,19 @@ #include "dbLayoutStateModel.h" +#include + namespace db { LayoutStateModel::LayoutStateModel (bool busy) - : m_hier_dirty (false), m_bboxes_dirty (false), m_busy (busy) + : m_hier_dirty (false), m_all_bboxes_dirty (false), m_busy (busy) { // .. nothing yet .. } LayoutStateModel::LayoutStateModel (const LayoutStateModel &d) - : m_hier_dirty (d.m_hier_dirty), m_bboxes_dirty (d.m_bboxes_dirty), m_busy (d.m_busy) + : m_hier_dirty (d.m_hier_dirty), m_bboxes_dirty (d.m_bboxes_dirty), m_all_bboxes_dirty (d.m_all_bboxes_dirty), m_busy (d.m_busy) { // .. nothing yet .. } @@ -43,6 +45,7 @@ LayoutStateModel::operator= (const LayoutStateModel &d) { m_hier_dirty = d.m_hier_dirty; m_bboxes_dirty = d.m_bboxes_dirty; + m_all_bboxes_dirty = d.m_all_bboxes_dirty; m_busy = d.m_busy; return *this; } @@ -65,5 +68,41 @@ LayoutStateModel::do_invalidate_bboxes (unsigned int index) bboxes_changed_any_event (); } +void +LayoutStateModel::invalidate_bboxes (unsigned int index) +{ + if (index == std::numeric_limits::max ()) { + if (! m_all_bboxes_dirty || m_busy) { + do_invalidate_bboxes (index); // must be called before the bboxes are invalidated (stopping of redraw thread requires this) + m_all_bboxes_dirty = true; + } + } else { + if (index >= (unsigned int) m_bboxes_dirty.size ()) { + m_bboxes_dirty.resize (index + 1, false); + } + if ((! m_all_bboxes_dirty && ! m_bboxes_dirty [index]) || m_busy) { + do_invalidate_bboxes (index); // must be called before the bboxes are invalidated (stopping of redraw thread requires this) + m_bboxes_dirty [index] = true; + } + } +} + +bool +LayoutStateModel::bboxes_dirty () const +{ + return ! m_bboxes_dirty.empty () || m_all_bboxes_dirty; +} + +void +LayoutStateModel::update () +{ + if (bboxes_dirty () || m_hier_dirty) { + do_update (); + m_bboxes_dirty.clear (); + m_all_bboxes_dirty = false; + m_hier_dirty = false; + } +} + } diff --git a/src/db/db/dbLayoutStateModel.h b/src/db/db/dbLayoutStateModel.h index 3eea4cf7c..c49a5b5fa 100644 --- a/src/db/db/dbLayoutStateModel.h +++ b/src/db/db/dbLayoutStateModel.h @@ -100,13 +100,7 @@ public: * If the index is std::numeric_limits::max, this method * applies to all layers. */ - void invalidate_bboxes (unsigned int index) - { - if (! m_bboxes_dirty || m_busy) { - do_invalidate_bboxes (index); // must be called before the bboxes are invalidated (stopping of redraw thread requires this) - m_bboxes_dirty = true; - } - } + void invalidate_bboxes (unsigned int index); /** * @brief Signal that the database unit has changed @@ -121,14 +115,7 @@ public: * * This method will call do_update if necessary and reset the invalid flags. */ - void update () - { - if (m_bboxes_dirty || m_hier_dirty) { - do_update (); - m_bboxes_dirty = false; - m_hier_dirty = false; - } - } + void update (); /** * @brief The "dirty hierarchy" attribute @@ -145,10 +132,7 @@ public: * * This attribute is true, if the bounding boxes have changed since the last "update" call */ - bool bboxes_dirty () const - { - return m_bboxes_dirty; - } + bool bboxes_dirty () const; /** * @brief Sets or resets busy mode @@ -211,7 +195,8 @@ public: private: bool m_hier_dirty; - bool m_bboxes_dirty; + std::vector m_bboxes_dirty; + bool m_all_bboxes_dirty; bool m_busy; void do_invalidate_hier (); diff --git a/src/db/unit_tests/dbLayout.cc b/src/db/unit_tests/dbLayout.cc index dea946924..389f026eb 100644 --- a/src/db/unit_tests/dbLayout.cc +++ b/src/db/unit_tests/dbLayout.cc @@ -202,3 +202,265 @@ TEST(1) EXPECT_EQ (ex, true); } +namespace +{ + +struct EventListener + : public tl::Object +{ + EventListener () + : flags (0), + bboxes_dirty (false), + bboxes_all_dirty (false), + hier_dirty (false), + dbu_dirty (true), + cell_name_dirty (true), + property_ids_dirty (true), + layer_properties_dirty (true) + { } + + void reset () { *this = EventListener (); } + + void bboxes_changed (unsigned int i) + { + if (i < 31) { + flags |= (1 << i); + } else { + bboxes_all_dirty = true; + } + } + + void bboxes_any_changed () { bboxes_dirty = true; } + void hier_changed () { hier_dirty = true; } + void dbu_changed () { dbu_dirty = true; } + void cell_name_changed () { cell_name_dirty = true; } + void property_ids_changed () { property_ids_dirty = true; } + void layer_properties_changed () { layer_properties_dirty = true; } + + unsigned int flags; + bool bboxes_dirty, bboxes_all_dirty, hier_dirty; + bool dbu_dirty, cell_name_dirty, property_ids_dirty, layer_properties_dirty; +}; + +} + +TEST(2) +{ + // LayoutStateModel hierarchy events + + db::Layout g; + EventListener el; + + g.hier_changed_event.add (&el, &EventListener::hier_changed); + g.bboxes_changed_any_event.add (&el, &EventListener::bboxes_any_changed); + g.bboxes_changed_event.add (&el, &EventListener::bboxes_changed); + + db::cell_index_type ci; + db::Cell *top; + + ci = g.add_cell ("TOP"); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, false); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, true); + + el.reset (); + top = &g.cell (ci); + ci = g.add_cell ("A"); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, false); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, false); // needs g.update() before being issues again + + el.reset (); + top->insert (db::CellInstArray (ci, db::Trans ())); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, true); + EXPECT_EQ (el.bboxes_all_dirty, true); + EXPECT_EQ (el.hier_dirty, false); // needs g.update() before being issues again + + g.clear (); + g.update (); + el.reset (); + + ci = g.add_cell ("TOP"); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, false); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, true); + + el.reset (); + g.update (); + top = &g.cell (ci); + ci = g.add_cell ("A"); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, false); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, true); // OK - see above + + el.reset (); + g.update (); + top->insert (db::CellInstArray (ci, db::Trans ())); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, true); + EXPECT_EQ (el.bboxes_all_dirty, true); + EXPECT_EQ (el.hier_dirty, true); // OK - see above + + // busy mode will make events issued always + g.clear (); + g.set_busy (true); + el.reset (); + + ci = g.add_cell ("TOP"); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, false); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, true); + + el.reset (); + top = &g.cell (ci); + ci = g.add_cell ("A"); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, false); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, true); // OK - see above + + el.reset (); + top->insert (db::CellInstArray (ci, db::Trans ())); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, true); + EXPECT_EQ (el.bboxes_all_dirty, true); + EXPECT_EQ (el.hier_dirty, true); // OK - see above + +} + +TEST(3) +{ + // LayoutStateModel bbox events + + db::Layout g; + EventListener el; + + g.insert_layer (0); + g.insert_layer (1); + + g.hier_changed_event.add (&el, &EventListener::hier_changed); + g.bboxes_changed_any_event.add (&el, &EventListener::bboxes_any_changed); + g.bboxes_changed_event.add (&el, &EventListener::bboxes_changed); + + db::cell_index_type ci; + db::Cell *top; + + ci = g.add_cell ("TOP"); + top = &g.cell (ci); + + EXPECT_EQ (el.flags, (unsigned int) 0); + EXPECT_EQ (el.bboxes_dirty, false); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, true); + + el.reset (); + g.update (); + + top->shapes (0).insert (db::Box (0, 0, 10, 20)); + top->shapes (1).insert (db::Box (0, 0, 10, 20)); + + EXPECT_EQ (el.flags, (unsigned int) 3); + EXPECT_EQ (el.bboxes_dirty, true); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, false); + + el.reset (); + + top->shapes (0).insert (db::Box (0, 0, 10, 20)); + + EXPECT_EQ (el.flags, (unsigned int) 0); // g.update () is missing -> no new events + EXPECT_EQ (el.bboxes_dirty, false); // g.update () is missing -> no new events + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, false); + + el.reset (); + g.update (); + + top->shapes (0).insert (db::Box (0, 0, 10, 20)); + + EXPECT_EQ (el.flags, (unsigned int) 1); // voila + EXPECT_EQ (el.bboxes_dirty, true); // :-) + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, false); + + top->shapes (1).insert (db::Box (0, 0, 10, 20)); + + EXPECT_EQ (el.flags, (unsigned int) 3); // and yet another one + EXPECT_EQ (el.bboxes_dirty, true); + EXPECT_EQ (el.bboxes_all_dirty, false); + EXPECT_EQ (el.hier_dirty, false); +} + +TEST(4) +{ + // Other events + + db::Layout g; + EventListener el; + + g.insert_layer (0); + g.insert_layer (1); + db::cell_index_type top = g.add_cell ("TOP"); + + g.dbu_changed_event.add (&el, &EventListener::dbu_changed); + g.cell_name_changed_event.add (&el, &EventListener::cell_name_changed); + g.prop_ids_changed_event.add (&el, &EventListener::property_ids_changed); + g.layer_properties_changed_event.add (&el, &EventListener::layer_properties_changed); + + EXPECT_EQ (el.dbu_dirty, false); + EXPECT_EQ (el.cell_name_dirty, false); + EXPECT_EQ (el.property_ids_dirty, false); + EXPECT_EQ (el.layer_properties_dirty, false); + + g.set_properties (0, db::LayerProperties (1, 0)); + EXPECT_EQ (el.layer_properties_dirty, true); + el.reset (); + g.set_properties (0, db::LayerProperties (1, 0)); + EXPECT_EQ (el.layer_properties_dirty, false); // no change + g.set_properties (0, db::LayerProperties (1, 1)); + EXPECT_EQ (el.layer_properties_dirty, true); // but this is + + g.dbu (1.0); + EXPECT_EQ (el.dbu_dirty, true); + el.reset (); + g.dbu (1.0); + EXPECT_EQ (el.dbu_dirty, false); // no change + g.dbu (0.5); + EXPECT_EQ (el.dbu_dirty, true); // but this is + + g.rename_cell (top, "TIP"); + EXPECT_EQ (el.cell_name_dirty, true); + el.reset (); + g.rename_cell (top, "TIP"); + EXPECT_EQ (el.cell_name_dirty, false); // no change + g.rename_cell (top, "TAP"); + EXPECT_EQ (el.cell_name_dirty, true); // but this is + + db::properties_id_type prop_id; + + db::PropertiesRepository::properties_set ps; + ps.insert (std::make_pair (g.properties_repository ().prop_name_id (tl::Variant (1)), tl::Variant ("XYZ"))); + prop_id = g.properties_repository ().properties_id (ps); + EXPECT_EQ (el.property_ids_dirty, true); + el.reset (); + + ps.clear (); + ps.insert (std::make_pair (g.properties_repository ().prop_name_id (tl::Variant (1)), tl::Variant ("XXX"))); + prop_id = g.properties_repository ().properties_id (ps); + EXPECT_EQ (el.property_ids_dirty, true); +} diff --git a/src/laybasic/laybasic/layLayoutCanvas.cc b/src/laybasic/laybasic/layLayoutCanvas.cc index 13b7ac339..f104dac58 100644 --- a/src/laybasic/laybasic/layLayoutCanvas.cc +++ b/src/laybasic/laybasic/layLayoutCanvas.cc @@ -40,6 +40,7 @@ #include "layBitmapsToImage.h" #include +#include namespace lay { @@ -1149,7 +1150,9 @@ LayoutCanvas::redraw_selected (const std::vector &layers) } m_need_redraw = true; - m_need_redraw_layer = layers; + m_need_redraw_layer.insert (m_need_redraw_layer.end (), layers.begin (), layers.end ()); + std::sort (m_need_redraw_layer.begin (), m_need_redraw_layer.end ()); + m_need_redraw_layer.erase (std::unique (m_need_redraw_layer.begin (), m_need_redraw_layer.end ()), m_need_redraw_layer.end ()); m_redraw_force_update = true; update (); // produces a paintEvent() @@ -1166,7 +1169,6 @@ LayoutCanvas::change_visibility (const std::vector &visible) if (! m_need_redraw) { m_redraw_clearing = false; - m_need_redraw_layer.clear (); } m_need_redraw = true; diff --git a/src/laybasic/laybasic/layLayoutView.cc b/src/laybasic/laybasic/layLayoutView.cc index bf2285c7b..2a5a65ffc 100644 --- a/src/laybasic/laybasic/layLayoutView.cc +++ b/src/laybasic/laybasic/layLayoutView.cc @@ -2126,7 +2126,7 @@ LayoutView::signal_bboxes_from_layer_changed (unsigned int cv_index, unsigned in // redraw only the layers required for redrawing for (std::vector::const_iterator l = mp_canvas->get_redraw_layers ().begin (); l != mp_canvas->get_redraw_layers ().end (); ++l) { - if (l->cellview_index == int (cv_index) && (layer_index == std::numeric_limits::max () || l->layer_index == int (layer_index))) { + if (l->cellview_index == int (cv_index) && l->layer_index == int (layer_index)) { redraw_layer ((unsigned int) (l - mp_canvas->get_redraw_layers ().begin ())); } }