luke
/
klayout
mirror of https://github.com/KLayout/klayout.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixed #464 (problems building a layer node tree with 'add_child') 

The reason was a synchronization issue.

Actually "LayerPropertiesNodeRef" is not a reference, but
a mirror copy of the LayerPropertiesNode the reference points
to. Changes to the original must be synchronized into the
reference and back.

This concept is a tribute to the original implementation and
the node reference was a convenience add-on to the iterator-
based API.

Better solution in general is to replace the LayerPropertiesNodeRef
concept with a real reference.
This commit is contained in:
Matthias Koefferlein 2020-01-04 19:39:09 +01:00
parent 85c033db64
commit 0285a5195c
3 changed files with 242 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
src/laybasic/laybasic/layLayerProperties.cc
View File

@ -78,7 +78,8 @@ LayerProperties::brighter (color_t in, int x)
}
LayerProperties::LayerProperties ()
: m_frame_color (0),
: m_gen_id (0),
m_frame_color (0),
m_frame_color_real (0),
m_fill_color (0),
m_fill_color_real (0),
@ -117,6 +118,7 @@ LayerProperties::LayerProperties ()
LayerProperties::LayerProperties (const LayerProperties &d)
: gsi::ObjectBase (d),
m_gen_id (0),
m_frame_color (0),
m_frame_color_real (0),
m_fill_color (0),
@ -159,6 +161,7 @@ LayerProperties::operator= (const LayerProperties &d)
{
if (&d != this) {
refresh ();
d.ensure_realized ();
int flags = 0;
@ -333,13 +336,21 @@ LayerProperties::merge_source (const LayerProperties *d) const
void
LayerProperties::ensure_realized () const
{
ensure_source_realized ();
ensure_visual_realized ();
refresh ();
if (m_realize_needed_source) {
realize_source ();
m_realize_needed_source = false;
}
if (m_realize_needed_visual) {
realize_visual ();
m_realize_needed_visual = false;
}
}
void
LayerProperties::ensure_source_realized () const
{
refresh ();
if (m_realize_needed_source) {
realize_source ();
m_realize_needed_source = false;
@ -349,6 +360,7 @@ LayerProperties::ensure_source_realized () const
void
LayerProperties::ensure_visual_realized () const
{
refresh ();
if (m_realize_needed_visual) {
realize_visual ();
m_realize_needed_visual = false;
@ -470,6 +482,8 @@ private:
std::string
LayerProperties::display_string (const lay::LayoutView *view, bool real, bool always_show_source) const
{
refresh ();
try {
std::string ret;
@ -516,6 +530,7 @@ LayerProperties::display_string (const lay::LayoutView *view, bool real, bool al
void
LayerProperties::set_xfill (bool xf)
{
refresh ();
if (xf != m_xfill) {
m_xfill = xf;
need_realize (nr_visual);
@ -531,6 +546,7 @@ LayerProperties::set_source (const std::string &s)
void
LayerProperties::set_source (const lay::ParsedLayerSource &s)
{
refresh ();
if (m_source != s) {
m_source = s;
need_realize (nr_source);
@ -554,9 +570,19 @@ LayerProperties::realize_source () const
do_realize (0);
}
void
LayerProperties::touch ()
{
if (++m_gen_id == 0) {
++m_gen_id;
}
}
void
LayerProperties::need_realize (unsigned int flags, bool /*force*/)
{
touch ();
if ((flags & nr_source) != 0) {
m_realize_needed_source = true;
}
@ -652,7 +678,7 @@ LayerPropertiesNode::LayerPropertiesNode (const LayerPropertiesNode &d)
m_children (d.m_children),
m_id (d.m_id)
{
for (iterator c = begin_children (); c != end_children (); ++c) {
for (iterator c = m_children.begin (); c != m_children.end (); ++c) {
c->set_parent (this);
}
}
@ -662,15 +688,15 @@ LayerPropertiesNode::operator= (const LayerPropertiesNode &d)
{
if (&d != this) {
LayerProperties::operator= (d);
m_children = d.m_children;
m_id = d.m_id;
for (iterator c = begin_children (); c != end_children (); ++c) {
for (iterator c = m_children.begin (); c != m_children.end (); ++c) {
c->set_parent (this);
}
LayerProperties::operator= (d);
need_realize (nr_hierarchy, true);
}
@ -722,12 +748,22 @@ LayerPropertiesNode::realize_source () const
void
LayerPropertiesNode::need_realize (unsigned int flags, bool force)
{
LayerProperties::need_realize (flags);
if ((flags & (nr_visual + nr_source)) != 0 && (force || ! realize_needed_visual () || ! realize_needed_source ())) {
LayerProperties::need_realize (flags);
for (iterator c = begin_children (); c != end_children (); ++c) {
for (iterator c = m_children.begin (); c != m_children.end (); ++c) {
c->need_realize (flags, force);
}
}
// Propagate the status change to the parents on hierarchy change.
// This is important to make any references to parent nodes aware of this
// change.
LayerPropertiesNode *p = const_cast<LayerPropertiesNode *> (parent ());
while (p) {
p->touch ();
p = const_cast<LayerPropertiesNode *> (p->parent ());
}
}
void
@ -773,7 +809,7 @@ LayerPropertiesNode::attach_view (LayoutView *view, unsigned int list_index)
mp_view.reset (view);
m_list_index = list_index;
for (iterator c = begin_children (); c != end_children (); ++c) {
for (iterator c = m_children.begin (); c != m_children.end (); ++c) {
c->attach_view (view, list_index);
}
// Attachment of a view is a strong indication that something significant changed -
@ -784,6 +820,7 @@ LayerPropertiesNode::attach_view (LayoutView *view, unsigned int list_index)
LayerPropertiesNode &
LayerPropertiesNode::insert_child (const iterator &iter, const LayerPropertiesNode &child)
{
refresh ();
iterator i = m_children.insert (iter, child);
i->set_parent (this);
need_realize (nr_hierarchy, true);
@ -793,6 +830,7 @@ LayerPropertiesNode::insert_child (const iterator &iter, const LayerPropertiesNo
void
LayerPropertiesNode::erase_child (const iterator &iter)
{
refresh ();
m_children.erase (iter);
need_realize (nr_hierarchy, true);
}
@ -800,6 +838,7 @@ LayerPropertiesNode::erase_child (const iterator &iter)
void
LayerPropertiesNode::add_child (const LayerPropertiesNode &child)
{
refresh ();
m_children.push_back (child);
m_children.back ().set_parent (this);
need_realize (nr_hierarchy, true);
@ -1049,7 +1088,6 @@ LayerPropertiesConstIterator::parent_obj () const
size_t uint = m_uint;
LayerPropertiesList::const_iterator iter = m_list->begin_const ();
size_t n = ((m_list->end_const () - m_list->begin_const ()) + 2);
size_t f = 1;
const LayerPropertiesNode *ret = 0;
while (uint > n) {
@ -1058,7 +1096,6 @@ LayerPropertiesConstIterator::parent_obj () const
tl_assert (rem < n - 1);
ret = &iter[rem - 1];
uint /= n;
f *= n;
n = ((ret->end_children () - ret->begin_children ()) + 2);
iter = ret->begin_children ();
}
@ -1095,14 +1132,12 @@ LayerPropertiesConstIterator::set_obj () const
size_t uint = m_uint;
LayerPropertiesList::const_iterator iter = m_list->begin_const ();
size_t n = ((m_list->end_const () - m_list->begin_const ()) + 2);
size_t f = 1;
while (uint > n) {
size_t rem = uint % n;
tl_assert (rem > 0);
tl_assert (rem < n - 1);
uint /= n;
f *= n;
n = ((iter[rem - 1].end_children () - iter[rem - 1].begin_children ()) + 2);
iter = iter[rem - 1].begin_children ();
}
@ -1911,7 +1946,7 @@ LayerPropertiesList::erase (const LayerPropertiesIterator &iter)
// LayerPropertiesNodeRef implementation
LayerPropertiesNodeRef::LayerPropertiesNodeRef (LayerPropertiesNode *node)
: m_iter (node)
: m_iter (node), m_synched_gen_id (0)
{
if (node) {
@ -1929,7 +1964,7 @@ LayerPropertiesNodeRef::LayerPropertiesNodeRef (LayerPropertiesNode *node)
}
LayerPropertiesNodeRef::LayerPropertiesNodeRef (const LayerPropertiesConstIterator &iter)
: m_iter (iter)
: m_iter (iter), m_synched_gen_id (0)
{
if (!iter.at_end () && !iter.is_null ()) {
@ -1949,12 +1984,13 @@ LayerPropertiesNodeRef::LayerPropertiesNodeRef (const LayerPropertiesConstIterat
}
LayerPropertiesNodeRef::LayerPropertiesNodeRef ()
: m_synched_gen_id (0)
{
// .. nothing yet ..
}
LayerPropertiesNodeRef::LayerPropertiesNodeRef (const LayerPropertiesNodeRef &other)
: LayerPropertiesNode (other), m_iter (other.m_iter), mp_node (other.mp_node)
: LayerPropertiesNode (other), m_iter (other.m_iter), mp_node (other.mp_node), m_synched_gen_id (0)
{
attach_view (other.view (), other.list_index ());
set_parent (other.parent ());
@ -1964,6 +2000,8 @@ LayerPropertiesNodeRef &LayerPropertiesNodeRef::operator= (const LayerProperties
{
if (this != &other) {
m_synched_gen_id = other.gen_id ();
mp_node = other.mp_node;
m_iter = other.m_iter;
attach_view (other.view (), other.list_index ());
@ -1981,6 +2019,8 @@ LayerPropertiesNodeRef::erase ()
{
if (is_valid ()) {
view ()->delete_layer ((unsigned int) list_index (), m_iter);
// detach from everthing
*this = LayerPropertiesNodeRef ();
}
}
@ -2009,13 +2049,30 @@ LayerPropertiesNodeRef::need_realize (unsigned int flags, bool force)
view ()->replace_layer_node ((unsigned int) list_index (), m_iter, *this);
}
// we're in sync now
m_synched_gen_id = mp_node->gen_id ();
} else if (mp_node) {
// fallback mode is to use the target node directly.
*mp_node = *this;
m_synched_gen_id = mp_node->gen_id ();
}
}
void
LayerPropertiesNodeRef::refresh () const
{
if (! mp_node.get () || m_synched_gen_id == mp_node->gen_id ()) {
return;
}
LayerPropertiesNodeRef *non_const_this = const_cast<LayerPropertiesNodeRef *> (this);
non_const_this->m_synched_gen_id = mp_node->gen_id ();
non_const_this->LayerPropertiesNode::operator= (*mp_node);
}
} // namespace lay

78
src/laybasic/laybasic/layLayerProperties.h
View File

@ -101,6 +101,18 @@ public:
*/
LayerProperties &operator= (const LayerProperties &d);
/**
* @brief Gets the generation number
*
* The generation number changes whenever something is changed
* with this layer properties object. "0" is reserved for the
* "uninitialized" state.
*/
size_t gen_id () const
{
return m_gen_id;
}
/**
* @brief Assignment alias for GSI binding
*/
@ -170,6 +182,7 @@ public:
ensure_visual_realized ();
return m_frame_color_real;
} else {
refresh ();
return m_frame_color;
}
}
@ -182,6 +195,7 @@ public:
*/
void set_frame_color_code (color_t c)
{
refresh ();
if (m_frame_color != c) {
m_frame_color = c;
need_realize (nr_visual);
@ -223,6 +237,7 @@ public:
ensure_visual_realized ();
return m_fill_color_real;
} else {
refresh ();
return m_fill_color;
}
}
@ -235,6 +250,7 @@ public:
*/
void set_fill_color_code (color_t c)
{
refresh ();
if (m_fill_color != c) {
m_fill_color = c;
need_realize (nr_visual);
@ -272,6 +288,7 @@ public:
*/
void set_frame_brightness (int b)
{
refresh ();
if (m_frame_brightness != b) {
m_frame_brightness = b;
need_realize (nr_visual);
@ -287,6 +304,7 @@ public:
ensure_visual_realized ();
return m_frame_brightness_real;
} else {
refresh ();
return m_frame_brightness;
}
}
@ -298,6 +316,7 @@ public:
*/
void set_fill_brightness (int b)
{
refresh ();
if (m_fill_brightness != b) {
m_fill_brightness = b;
need_realize (nr_visual);
@ -315,6 +334,7 @@ public:
ensure_visual_realized ();
return m_fill_brightness_real;
} else {
refresh ();
return m_fill_brightness;
}
}
@ -324,6 +344,7 @@ public:
*/
void set_dither_pattern (int index)
{
refresh ();
if (m_dither_pattern != index) {
m_dither_pattern = index;
need_realize (nr_visual);
@ -353,6 +374,7 @@ public:
ensure_visual_realized ();
return m_dither_pattern_real;
} else {
refresh ();
return m_dither_pattern;
}
}
@ -378,6 +400,7 @@ public:
*/
void set_line_style (int index)
{
refresh ();
if (m_line_style != index) {
m_line_style = index;
need_realize (nr_visual);
@ -407,6 +430,7 @@ public:
ensure_visual_realized ();
return m_line_style_real;
} else {
refresh ();
return m_line_style;
}
}
@ -432,6 +456,7 @@ public:
*/
void set_valid (bool v)
{
refresh ();
if (m_valid != v) {
m_valid = v;
need_realize (nr_visual);
@ -447,6 +472,7 @@ public:
ensure_visual_realized ();
return m_valid_real;
} else {
refresh ();
return m_valid;
}
}
@ -456,6 +482,7 @@ public:
*/
void set_visible (bool v)
{
refresh ();
if (m_visible != v) {
m_visible = v;
need_realize (nr_visual);
@ -471,12 +498,13 @@ public:
ensure_visual_realized ();
return m_visible_real;
} else {
refresh ();
return m_visible;
}
}
/**
* @brief Return true, if the layer is showing "something"
* @brief Returns true, if the layer is showing "something"
*
* A layer "shows something" if it is visible and it displays some information,
* either shapes or cell boxes. Invalid layers, i.e. such that have a layer selection
@ -523,6 +551,7 @@ public:
*/
void set_transparent (bool t)
{
refresh ();
if (m_transparent != t) {
m_transparent = t;
need_realize (nr_visual);
@ -538,6 +567,7 @@ public:
ensure_visual_realized ();
return m_transparent_real;
} else {
refresh ();
return m_transparent;
}
}
@ -547,6 +577,7 @@ public:
*/
void set_width (int w)
{
refresh ();
if (m_width != w) {
m_width = w;
need_realize (nr_visual);
@ -562,6 +593,7 @@ public:
ensure_visual_realized ();
return m_width_real;
} else {
refresh ();
return m_width;
}
}
@ -571,6 +603,7 @@ public:
*/
void set_marked (bool t)
{
refresh ();
if (m_marked != t) {
m_marked = t;
need_realize (nr_visual);
@ -586,6 +619,7 @@ public:
ensure_visual_realized ();
return m_marked_real;
} else {
refresh ();
return m_marked;
}
}
@ -595,6 +629,7 @@ public:
*/
void set_animation (int a)
{
refresh ();
if (m_animation != a) {
m_animation = a;
need_realize (nr_visual);
@ -610,6 +645,7 @@ public:
ensure_visual_realized ();
return m_animation_real;
} else {
refresh ();
return m_animation;
}
}
@ -623,6 +659,7 @@ public:
ensure_visual_realized ();
return m_xfill_real;
} else {
refresh ();
return m_xfill;
}
}
@ -637,6 +674,7 @@ public:
*/
void set_name (const std::string &n)
{
refresh ();
if (m_name != n) {
m_name = n;
need_realize (nr_meta);
@ -648,6 +686,7 @@ public:
*/
const std::string &name () const
{
refresh ();
return m_name;
}
@ -699,6 +738,7 @@ public:
ensure_source_realized ();
return m_source_real;
} else {
refresh ();
return m_source;
}
}
@ -849,11 +889,6 @@ protected:
*/
void do_realize (const LayoutView *view) const;
/**
* @brief Tell the children that a realize of the visual properties is needed
*/
virtual void need_realize (unsigned int flags, bool force = false);
/**
* @brief Tell, if a realize of the visual properties is needed
*/
@ -870,7 +905,28 @@ protected:
return m_realize_needed_source;
}
/**
* @brief Marks the properties object as modified
*
* This will basically increment the generation count.
* This method is implied when calling "needs_realize".
*/
void touch ();
/**
* @brief Tells the children that a realize of the visual properties is needed
* This is also the "forward sync" for the LayerPropertiesNodeRef.
*/
virtual void need_realize (unsigned int flags, bool force = false);
/**
* @brief Fetches the current status from the original properties for the LayerPropertiesNodeRef implementation
*/
virtual void refresh () const { }
private:
// the generation number
size_t m_gen_id;
// display styles
color_t m_frame_color;
mutable color_t m_frame_color_real;
@ -1018,6 +1074,7 @@ public:
*/
const_iterator begin_children () const
{
refresh ();
return m_children.begin ();
}
@ -1026,6 +1083,7 @@ public:
*/
const_iterator end_children () const
{
refresh ();
return m_children.end ();
}
@ -1034,6 +1092,7 @@ public:
*/
iterator begin_children ()
{
refresh ();
return m_children.begin ();
}
@ -1042,6 +1101,7 @@ public:
*/
iterator end_children ()
{
refresh ();
return m_children.end ();
}
@ -1882,6 +1942,10 @@ private:
* a node's property will update the properties in the view as well.
* Second, changes in the node's hierarchy will be reflected in the
* view's layer hierarchy too.
*
* The implementation is based on a synchronized mirror copy of the
* original node. This is not quite efficient and cumbersome. In the
* future, this may be replaced by a simple reference.
*/
class LAYBASIC_PUBLIC LayerPropertiesNodeRef
: public LayerPropertiesNode
@ -1944,8 +2008,10 @@ public:
private:
LayerPropertiesConstIterator m_iter;
tl::weak_ptr<LayerPropertiesNode> mp_node;
size_t m_synched_gen_id;
void need_realize (unsigned int flags, bool force);
void refresh () const;
};
}

102
testdata/ruby/layLayers.rb vendored
View File

@ -25,20 +25,20 @@ load("test_prologue.rb")
class LAYLayers_TestClass < TestBase
def lnode_str( space, l )
return space + l.current.source( true ) + "\n";
def lnode_str(space, l)
return space + l.current.source(true) + "\n";
end
def lnodes_str( space, l )
def lnodes_str(space, l)
res = ""
while ! l.at_end?
res += lnode_str( space, l )
res += lnode_str(space, l)
if (l.current.has_children?)
l.down_first_child
res += lnodes_str( " #{space}", l )
res += lnodes_str(" #{space}", l)
l.up
end
l.next_sibling( 1 )
l.next_sibling(1)
end
return res
end
@ -940,6 +940,96 @@ class LAYLayers_TestClass < TestBase
end
# dynamic update of tree with LayerPropertiesNodeRef
def test_7
if !RBA.constants.member?(:Application)
return
end
app = RBA::Application.instance
mw = app.main_window
mw.close_all
mw.load_layout( ENV["TESTSRC"] + "/testdata/gds/t11.gds", 1 )
lv = mw.current_view
lv.clear_layers
lp = RBA::LayerProperties::new
lp.source = "A@*"
lp = lv.insert_layer(lv.begin_layers, lp)
lpp = RBA::LayerProperties::new
lpp = lp.add_child(lpp)
lpp.source = "A.1"
lppp = RBA::LayerProperties::new
lppp = lpp.add_child(lppp)
lppp.source = "A.1.1@*"
lppp = RBA::LayerProperties::new
lppp = lpp.add_child(lppp)
li = lv.begin_layers # lp
li.down_first_child # lpp
li.down_first_child # before lppp
li.next # lppp
li.current.source = "X"
assert_equal(lppp.source, "X@1")
lppp.source = "A.1.2"
assert_equal(li.current.source, "A.1.2@1")
lpp = RBA::LayerProperties::new
lpp = lp.add_child(lpp)
lpp.source = "A.2@*"
lppp = RBA::LayerProperties::new
lppp = lpp.add_child(lppp)
lppp.source = "A.2.1"
lppp_saved = lppp
lppp = RBA::LayerProperties::new
lppp = lpp.add_child(lppp)
lppp.source = "A.2.2@*"
assert_equal(lnodes_str("", lv.begin_layers),
"A@*\n" +
" A.1@1\n" +
" A.1.1@1\n" +
" A.1.2@1\n" +
" A.2@*\n" +
" A.2.1@1\n" +
" A.2.2@*\n"
)
lp.source = "B@2"
assert_equal(lnodes_str("", lv.begin_layers),
"B@2\n" +
" A.1@1\n" +
" A.1.1@1\n" +
" A.1.2@1\n" +
" A.2@2\n" +
" A.2.1@1\n" +
" A.2.2@2\n"
)
lppp_saved.delete
assert_equal(lnodes_str("", lv.begin_layers),
"B@2\n" +
" A.1@1\n" +
" A.1.1@1\n" +
" A.1.2@1\n" +
" A.2@2\n" +
" A.2.2@2\n"
)
assert_equal(lppp.source, "A.2.2@*")
end
end
load("test_epilogue.rb")