Checks for grobs with circular parentage in the regtests. (issue4747045)

[mtsolo]

Reviewers: ,

Message:
This gets rid of all circular parentage in LilyPond (at least all that's
in the regtests...).

Description:
Checks for grobs with circular parentage in the regtests.

Eliminates all circular parenting cases from the code base, allowing
for null pointers to be parents but issuing a programming error (this
should only happen for staves that will never see the light of day due
to bad code).

Please review this at http://codereview.appspot.com/4747045/

Affected files:
  M lily/dynamic-align-engraver.cc
  M lily/grob.cc
  M lily/include/grob.hh
  M lily/include/main.hh
  M lily/pitched-trill-engraver.cc
  M lily/program-option-scheme.cc
  M make/lilypond-vars.make
  M make/lysdoc-targets.make
  M scm/lily.scm


Index: lily/dynamic-align-engraver.cc
diff --git a/lily/dynamic-align-engraver.cc b/lily/dynamic-align-engraver.cc
index  
f62c45f0dc6d3adec09cccf6e700fa1722806a24..cd57b480b8858834d755cd2ef99e64bff7a05337  
100644
--- a/lily/dynamic-align-engraver.cc
+++ b/lily/dynamic-align-engraver.cc
@@ -166,16 +166,26 @@ Dynamic_align_engraver::stop_translation_timestep ()

          Grob *bound = 0;
          if (scripts_.size ())
-           bound = scripts_[0];
-         else if (spanners.size ())
-           bound = spanners[0]->get_bound (d);
-         else
-           {
-             bound = unsmob_grob (get_property ("currentMusicalColumn"));
-             if (!early_end_)
-               programming_error ("started DynamicLineSpanner but have no left 
bound");
-           }
-
+            for (vsize i = 0; i < scripts_.size (); i++)
+              if (!Grob::in_own_family_tree (scripts_[i], line_))
+              {
+               bound = scripts_[i];
+               break;
+              }
+         if (!bound)
+            if (spanners.size ())
+              for (vsize i = 0; i < spanners.size (); i++)
+                if (!Grob::in_own_family_tree (spanners[i]->get_bound (d),  
line_))
+                {
+                  bound = spanners[i]->get_bound (d);
+                  break;
+                }
+          if (!bound)
+            {
+              bound = unsmob_grob (get_property ("currentMusicalColumn"));
+              if (!early_end_ && !spanners.size () && !scripts_.size ())
+                programming_error ("started DynamicLineSpanner but have no  
left bound");
+            }
          line_->set_bound (d, bound);
        }
     }
Index: lily/grob.cc
diff --git a/lily/grob.cc b/lily/grob.cc
index  
c613effccc65fd6be001aad742e41a1ed29b15a4..43c0cdbd4339307df676117d576c4346a39b23a6  
100644
--- a/lily/grob.cc
+++ b/lily/grob.cc
@@ -524,9 +524,31 @@ Grob::common_refpoint (Grob const *s, Axis a) const
   return 0;
 }

+bool
+Grob::in_own_family_tree (Grob *g, Grob *orig)
+{
+  if (!g)
+    {
+      orig->programming_error ("Setting grob parent to a null pointer.");
+      return false;
+    }
+  if (g == orig)
+    return true;
+  Grob *x_parent = g->get_parent (X_AXIS);
+  Grob *y_parent = g->get_parent (Y_AXIS);
+  bool in_tree = false;
+  if (x_parent)
+    in_tree = in_tree || in_own_family_tree (x_parent, orig);
+  if (y_parent)
+    in_tree = in_tree || in_own_family_tree (y_parent, orig);
+  return in_tree;
+}
+
 void
 Grob::set_parent (Grob *g, Axis a)
 {
+  if (do_internal_grob_family_tree_checking_global)
+    assert (!in_own_family_tree (g, this));
   dim_cache_[a].parent_ = g;
 }

Index: lily/include/grob.hh
diff --git a/lily/include/grob.hh b/lily/include/grob.hh
index  
eda94b6d5888b1778897658c683c7c20c5c57944..36c0fda8cf0b29a3e03de3dd554915273945c5de  
100644
--- a/lily/include/grob.hh
+++ b/lily/include/grob.hh
@@ -137,6 +137,7 @@ public:

   /* refpoints */
   Grob *common_refpoint (Grob const *s, Axis a) const;
+  static bool in_own_family_tree(Grob *g, Grob *orig);
   void set_parent (Grob *e, Axis);
   Grob *get_parent (Axis a) const;
   void fixup_refpoint ();
Index: lily/include/main.hh
diff --git a/lily/include/main.hh b/lily/include/main.hh
index  
7fa0905c83dc559bf0dc5c49950b263d58617d99..c512cc97c3a306c22482e84bd94bfa9f6ef52034  
100644
--- a/lily/include/main.hh
+++ b/lily/include/main.hh
@@ -44,6 +44,7 @@ extern string output_name_global;
 extern bool be_safe_global;
 extern bool be_verbose_global;
 extern bool do_internal_type_checking_global;
+extern bool do_internal_grob_family_tree_checking_global;
 extern bool point_and_click_global;
 extern string lilypond_datadir;
 extern bool use_object_keys;
Index: lily/pitched-trill-engraver.cc
diff --git a/lily/pitched-trill-engraver.cc b/lily/pitched-trill-engraver.cc
index  
6960d3e2dd1883d390daff87f982aefa7483da2f..8d2d599c5953f20596a25c2b29c640215c5ad7d6  
100644
--- a/lily/pitched-trill-engraver.cc
+++ b/lily/pitched-trill-engraver.cc
@@ -114,17 +114,17 @@ Pitched_trill_engraver::make_trill (Stream_event *ev)
       trill_head_ = 0;
     }

+  trill_group_ = make_item ("TrillPitchGroup", ev->self_scm ());
   trill_head_ = make_item ("TrillPitchHead", ev->self_scm ());
   SCM c0scm = get_property ("middleCPosition");

   int c0 = scm_is_number (c0scm) ? scm_to_int (c0scm) : 0;

-  trill_head_->set_property ("staff-position",
-                            scm_from_int (unsmob_pitch (scm_pitch)->steps ()
-                                          + c0));
+  // Multiply by 0.5 for staff space
+  trill_group_->translate_axis ((unsmob_pitch (scm_pitch)->steps () + c0 )  
* 0.5,
+                                Y_AXIS);

-  trill_group_ = make_item ("TrillPitchGroup", ev->self_scm ());
-  trill_group_->set_parent (trill_head_, Y_AXIS);
+  trill_head_->set_parent (trill_group_, Y_AXIS);

   Axis_group_interface::add_element (trill_group_, trill_head_);

@@ -137,7 +137,7 @@ Pitched_trill_engraver::make_trill (Stream_event *ev)
       Side_position_interface::add_support (trill_accidental_,  
trill_head_);

       trill_head_->set_object ("accidental-grob",  
trill_accidental_->self_scm ());
-      trill_accidental_->set_parent (trill_head_, Y_AXIS);
+      trill_accidental_->set_parent (trill_group_, Y_AXIS);
       Axis_group_interface::add_element (trill_group_, trill_accidental_);
     }
 }
Index: lily/program-option-scheme.cc
diff --git a/lily/program-option-scheme.cc b/lily/program-option-scheme.cc
index  
63266566aca22d3ab9a0a221bcdba4e9405ff579..a6d8c90ee4b348b5ec3af6852c8657ab4e3ac1cf  
100644
--- a/lily/program-option-scheme.cc
+++ b/lily/program-option-scheme.cc
@@ -47,6 +47,11 @@ bool profile_property_accesses = false;
   crash if internally the wrong type is used for a grob property.
 */
 bool do_internal_type_checking_global;
+
+/*
+ crash internally if a grob is its own ancester.
+*/
+bool do_internal_grob_family_tree_checking_global;
 bool strict_infinity_checking = false;

 static SCM option_hash;
@@ -73,6 +78,11 @@ internal_set_option (SCM var,
       parse_protect_global = to_boolean (val);
       val = scm_from_bool (to_boolean (val));
     }
+  else if (var == ly_symbol2scm ("check-grob-parents"))
+    {
+      do_internal_grob_family_tree_checking_global = to_boolean (val);
+      val = scm_from_bool (to_boolean (val));
+    }
   else if (var == ly_symbol2scm ("check-internal-types"))
     {
       do_internal_type_checking_global = to_boolean (val);
Index: make/lilypond-vars.make
diff --git a/make/lilypond-vars.make b/make/lilypond-vars.make
index  
cf1bc11203487102b4c3bb39d569aae1445a81fc..be2ea9194b3dcef74b91887854a25d763c8b542f  
100644
--- a/make/lilypond-vars.make
+++ b/make/lilypond-vars.make
@@ -51,6 +51,7 @@ $(LANG_DOCTITLE_FLAGS) \
 --header=texidoc \
 $(LANG_TEXIDOC_FLAGS) \
 -dcheck-internal-types \
+-dcheck-grob-parents \
 -ddump-signatures \
 -danti-alias-factor=$(ANTI_ALIAS_FACTOR)

Index: make/lysdoc-targets.make
diff --git a/make/lysdoc-targets.make b/make/lysdoc-targets.make
index  
e39fa1c58bc047a626e8ef32abe649a40e95550c..d62a9ad36c0c6b6a49b88eff3a4e9b672d49a162  
100644
--- a/make/lysdoc-targets.make
+++ b/make/lysdoc-targets.make
@@ -16,6 +16,6 @@ local-test:
                echo -e '\n\n\n' ; \
                (cd $(top-src-dir) && git diff ) ; \
        fi > $(outdir)/tree.gittxt
-	$(MAKE) LILYPOND_BOOK_LILYPOND_FLAGS="-dbackend=eps --formats=ps  
$(LILYPOND_JOBS) -dseparate-log-files -dinclude-eps-fonts  
-dgs-load-lily-fonts --header=texidoc -I  
$(top-src-dir)/Documentation/included/ -ddump-profile  
-dcheck-internal-types -ddump-signatures -danti-alias-factor=1"  
LILYPOND_BOOK_VERBOSE= $(outdir)/collated-files.html  
LYS_OUTPUT_DIR=$(top-build-dir)/out/lybook-testdb
+	$(MAKE) LILYPOND_BOOK_LILYPOND_FLAGS="-dbackend=eps --formats=ps  
$(LILYPOND_JOBS) -dseparate-log-files -dinclude-eps-fonts  
-dgs-load-lily-fonts --header=texidoc -I  
$(top-src-dir)/Documentation/included/ -ddump-profile  
-dcheck-internal-types -dcheck-grob-parents -ddump-signatures  
-danti-alias-factor=1" LILYPOND_BOOK_VERBOSE= $(outdir)/collated-files.html  
LYS_OUTPUT_DIR=$(top-build-dir)/out/lybook-testdb
 	rsync -L -a --exclude 'out-*' --exclude 'out' --exclude mf --exclude  
source --exclude mf $(top-build-dir)/out/share $(outdir)

Index: scm/lily.scm
diff --git a/scm/lily.scm b/scm/lily.scm
index  
203a449c8f00f1e9d9c7108cb7a8cd045077777f..6d87c697435cfb5d720e1a03f7a0e4d4bee7dedc  
100644
--- a/scm/lily.scm
+++ b/scm/lily.scm
@@ -54,6 +54,8 @@ EPS backend.")
     (backend ps
 "Select backend.  Possible values: 'eps, 'null,
 'ps, 'scm, 'socket, 'svg.")
+    (check-grob-parents #f
+"Check to see that grob family trees are not recursive.")
     (check-internal-types #f
 "Check every property assignment for types.")
     (clip-systems #f