Re: Axis group interface ignores column rank for pure-from-neighbor-interface (issue 5843063)

[mtsolo]

Reviewers: Keith, mike_apollinemike.com,

Message:
Running regtests.  Will post new patch once it gets a clean bill of
health.


http://codereview.appspot.com/5843063/diff/7/input/regression/pure-from-neighbor-interface-pure-height.ly
File input/regression/pure-from-neighbor-interface-pure-height.ly
(right):

http://codereview.appspot.com/5843063/diff/7/input/regression/pure-from-neighbor-interface-pure-height.ly#newcode1
input/regression/pure-from-neighbor-interface-pure-height.ly:1: \version
"2.15.34"
On 2012/03/21 05:58:42, Keith wrote:
> .35
> The title makes less sense now.  "lyrics-spanbarstub.ly" ?

Done.

http://codereview.appspot.com/5843063/diff/7/input/regression/pure-from-neighbor-interface-pure-height.ly#newcode4
input/regression/pure-from-neighbor-interface-pure-height.ly:4: texidoc
= "@code{axis-group-interface::pure-height} does not take spanned
On 2012/03/21 05:58:42, Keith wrote:
> The description no longer fits.  Maybe
> "empty measures do not confuse SpanBarStub.  These lyrics should
remain clear of
> the span bars."

Done.

http://codereview.appspot.com/5843063/diff/7/lily/item.cc
File lily/item.cc (right):

http://codereview.appspot.com/5843063/diff/7/lily/item.cc#newcode245
lily/item.cc:245: cache_pure_height (Grob::pure_height (this, 0,
INT_MAX));
On 2012/03/21 05:58:42, Keith wrote:
> This commit :

http://git.savannah.gnu.org/gitweb/?p=lilypond.git;a=commitdiff;h=9fe7859a8592a080413b744e3768db41059dbfe3
> depended on having 'start' and 'end' passed through, so we should put
this back.

> Caching in this way assumes that the pure_height of an Item is
independent of
> where the line-breaks are.

> Anything that gets pure_height from its neighbors would break that
assumption,
> but now it seems there are no such Items, as they all use
> pure-from-neighbors-interface to set their extra-spacing-height.

> Tied accidentals break that assumption as well, but that's not our
fault.  I'll
> write a cautionary comment in a separate commit.

Done.

http://codereview.appspot.com/5843063/diff/7/scm/define-grobs.scm
File scm/define-grobs.scm (right):

http://codereview.appspot.com/5843063/diff/7/scm/define-grobs.scm#newcode1883
scm/define-grobs.scm:1883: (Y-extent . (1 . -1))
On 2012/03/21 05:58:42, Keith wrote:
> I suggest (Y-extent . #f)
> because (1 . -1) looks so much like (-1 . 1), and it also makes
suspicious
> people like me think it is an uncommented magic number.

Done.

http://codereview.appspot.com/5843063/diff/7/scm/output-lib.scm
File scm/output-lib.scm (right):

http://codereview.appspot.com/5843063/diff/7/scm/output-lib.scm#newcode437
scm/output-lib.scm:437: (let* ((height (ly:grob-pure-height grob grob 0
10000000))
On 2012/03/21 05:58:42, Keith wrote:
> The C code uses INT_MAX to represent the end of the whole score, so it
would be
> nice to use the same here, if possible.

There'd have to be a constant defined for INT_MAX in guile.  I believe
that INT_MAX can change between computers (not sure if it's the compiler
or system that does this).  Probably worth doing in a separate commit
where INT-MAX is defined in the same place as all of the PI constants
and then plugged anywhere where there's a large integer.

Description:
Axis group interface ignores column rank for
pure-from-neighbor-interface

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

Affected files:
  A input/regression/pure-from-neighbor-interface-pure-height.ly
  M lily/item.cc
  M lily/pure-from-neighbor-engraver.cc
  M scm/define-grobs.scm
  M scm/output-lib.scm


Index: input/regression/pure-from-neighbor-interface-pure-height.ly
diff --git a/input/regression/pure-from-neighbor-interface-pure-height.ly  
b/input/regression/pure-from-neighbor-interface-pure-height.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..3e7515dfea1cc991fef736e76c9566ffca728c64
--- /dev/null
+++ b/input/regression/pure-from-neighbor-interface-pure-height.ly
@@ -0,0 +1,18 @@
+\version "2.15.34"
+
+\header {
+  texidoc = "@code{axis-group-interface::pure-height} does not take spanned
+rank interval into account for @code{pure-from-neighbor-interface} grobs.
+"
+}
+
+\new StaffGroup <<
+  \new Staff { \repeat unfold 8 { R1 e'1 } }
+  \addlyrics {
+    Worked twice...
+    and then
+    I continued...
+    working... correctly.
+  }
+  \new Staff { R1*16 }
+>>
Index: lily/item.cc
diff --git a/lily/item.cc b/lily/item.cc
index  
6ea5643a5445e7d791a119479e73d54a5fe7aead..19c9926c3b755ae660e3a698da441498d755d5e1  
100644
--- a/lily/item.cc
+++ b/lily/item.cc
@@ -242,7 +242,7 @@ Item::pure_height (Grob *g, int start, int end)
   if (cached_pure_height_valid_)
     return cached_pure_height_ + pure_relative_y_coordinate (g, start,  
end);

-  cache_pure_height (Grob::pure_height (this, start, end));
+  cache_pure_height (Grob::pure_height (this, 0, INT_MAX));
   return cached_pure_height_ + pure_relative_y_coordinate (g, start, end);
 }

Index: lily/pure-from-neighbor-engraver.cc
diff --git a/lily/pure-from-neighbor-engraver.cc  
b/lily/pure-from-neighbor-engraver.cc
index  
3dc87664f83ecffe7341263ae97302a761be768f..a6e7b5f32fe4539867f3ddbabe136357b8ba9335  
100644
--- a/lily/pure-from-neighbor-engraver.cc
+++ b/lily/pure-from-neighbor-engraver.cc
@@ -97,9 +97,9 @@ Pure_from_neighbor_engraver::finalize ()
   int pos[2] = { -1, 0};
   for (vsize i = 0; i < pure_relevants_.size (); i++)
     {
-      if (pos[1] < (int) need_pure_heights_from_neighbors.size ()
-          && (pure_relevants_[i]->spanned_rank_interval ()[LEFT]
-              >  
need_pure_heights_from_neighbors[pos[1]][0]->spanned_rank_interval  
()[LEFT]))
+      while (pos[1] < (int) need_pure_heights_from_neighbors.size ()
+             && (pure_relevants_[i]->spanned_rank_interval ()[LEFT]
+                 >  
need_pure_heights_from_neighbors[pos[1]][0]->spanned_rank_interval  
()[LEFT]))
         {
           pos[0] = pos[1];
           pos[1]++;
Index: scm/define-grobs.scm
diff --git a/scm/define-grobs.scm b/scm/define-grobs.scm
index  
44130334346af5416004a06ac39d5a5d7afce624..cf6ed9270771f6ed219b153472527a794b08da7c  
100644
--- a/scm/define-grobs.scm
+++ b/scm/define-grobs.scm
@@ -1879,7 +1879,8 @@
     (SpanBarStub
      . (
         (X-extent . ,grob::x-parent-width)
-	(Y-extent . ,(ly:make-unpure-pure-container #f  
ly:axis-group-interface::pure-height))
+	 
(extra-spacing-height . ,pure-from-neighbor-interface::extra-spacing-height)
+       (Y-extent . (1 . -1))
        (meta . ((class . Item)
 		 (object-callbacks .  
((pure-Y-common . ,ly:axis-group-interface::calc-pure-y-common)
 				       
(pure-relevant-grobs . ,ly:pure-from-neighbor-interface::calc-pure-relevant-grobs)))
Index: scm/output-lib.scm
diff --git a/scm/output-lib.scm b/scm/output-lib.scm
index  
17bbea995b1f7c7bf03005b4ffffea2d6c9fd221..bb8f4ddf50d96c80bdece61ebb473d1d36604396  
100644
--- a/scm/output-lib.scm
+++ b/scm/output-lib.scm
@@ -434,7 +434,7 @@ and duration-log @var{log}."
       (cons -0.1 0.1)))

 (define-public (pure-from-neighbor-interface::extra-spacing-height grob)
-  (let* ((height (ly:grob::stencil-height grob))
+  (let* ((height (ly:grob-pure-height grob grob 0 10000000))
          (from-neighbors (interval-union
                             height
                             (ly:axis-group-interface::pure-height