2008/7/27 Han-Wen Nienhuys <[EMAIL PROTECTED]>:

> I don't understand the original code. 'me' should already be the
> staff-symbol

That's only true for Staff_symbol_referencer::on_line () before the patch.

The revised version, Staff_symbol::on_line (), deals with two separate
cases, i.e., standard staves with 'line-count and exotic staves with
'line-positions. It doesn't return the correct boolean value when
called for ties and dots, since in these cases 'me' isn't the
staff-symbol and line_count (me) will be zero here:

return ((abs (pos + line_count (me)) % 2) == 1);

You can see the result in the regtest example below (based on current
git); as it stands, on_line () only works properly when
'line-positions is set.

As a better solution than the quick fix I posted previously, I propose
moving on_line () back to staff-symbol-referencer.cc and creating a
new method get_line_positions () to retrieve 'line-positions safely.

Is this patch OK?

Regards,
Neil
From b2b04a389c16bb93e54028a6fd931eeb565a8090 Mon Sep 17 00:00:00 2001
From: Neil Puttock <[EMAIL PROTECTED]>
Date: Wed, 6 Aug 2008 22:09:48 +0100
Subject: [PATCH] Revised fix for Staff_symbol_referencer::on_line ()

---
 lily/include/staff-symbol-referencer.hh |    3 +-
 lily/include/staff-symbol.hh            |    1 -
 lily/staff-symbol-referencer.cc         |   35 ++++++++++++++++++++++++++++++-
 lily/staff-symbol.cc                    |   30 --------------------------
 4 files changed, 36 insertions(+), 33 deletions(-)

diff --git a/lily/include/staff-symbol-referencer.hh b/lily/include/staff-symbol-referencer.hh
index 4a43a94..8ae0ed8 100644
--- a/lily/include/staff-symbol-referencer.hh
+++ b/lily/include/staff-symbol-referencer.hh
@@ -39,7 +39,8 @@ public:
   static Real get_position (Grob *);
   static Real staff_radius (Grob *);
   static int get_rounded_position (Grob *);
-  static Interval extent_in_staff (Grob *); 
+  static Interval extent_in_staff (Grob *);
+  static SCM get_line_positions (Grob *); 
 };
 
 int compare_position (Grob *const &, Grob *const &);\
diff --git a/lily/include/staff-symbol.hh b/lily/include/staff-symbol.hh
index 6791e7b..4dacde8 100644
--- a/lily/include/staff-symbol.hh
+++ b/lily/include/staff-symbol.hh
@@ -24,7 +24,6 @@ public:
   
   static int get_steps (Grob *);
   static int line_count (Grob *);
-  static bool on_line (Grob *me, int pos);
   DECLARE_SCHEME_CALLBACK (print, (SCM));
   DECLARE_SCHEME_CALLBACK (height, (SCM));  
   DECLARE_GROB_INTERFACE();
diff --git a/lily/staff-symbol-referencer.cc b/lily/staff-symbol-referencer.cc
index dbff024..81ad054 100644
--- a/lily/staff-symbol-referencer.cc
+++ b/lily/staff-symbol-referencer.cc
@@ -20,6 +20,15 @@ Staff_symbol_referencer::line_count (Grob *me)
   return st ? Staff_symbol::line_count (st) : 0;
 }
 
+SCM
+Staff_symbol_referencer::get_line_positions (Grob *me)
+{
+  Grob *st = get_staff_symbol (me);
+  return st
+    ? st->get_property ("line-positions")
+    : SCM_EOL;
+}
+
 bool
 Staff_symbol_referencer::on_line (Grob *me)
 {
@@ -35,7 +44,31 @@ Staff_symbol_referencer::on_staff_line (Grob *me)
 bool
 Staff_symbol_referencer::on_line (Grob *me, int pos)
 {
-  return Staff_symbol::on_line (me, pos);
+  SCM line_positions = get_line_positions (me);
+  if (scm_is_pair (line_positions))
+    {
+      Real min_line = HUGE_VAL;
+      Real max_line = -HUGE_VAL;
+      for (SCM s = line_positions; scm_is_pair (s); s = scm_cdr (s))
+	{
+	  Real current_line = scm_to_double (scm_car (s));
+	  if (pos == current_line)
+	    return true;
+	  if (current_line > max_line)
+	    max_line = current_line;
+	  if (current_line < min_line)
+	    min_line = current_line;
+	
+	}
+      if (pos < min_line)
+	return (( (int) (rint (pos - min_line)) % 2) == 0);
+      if (pos > max_line)
+	return (( (int) (rint (pos - max_line)) % 2) == 0);
+
+      return false;
+    }
+  else
+    return ((abs (pos + Staff_symbol_referencer::line_count (me)) % 2) == 1);
 }
 
 bool
diff --git a/lily/staff-symbol.cc b/lily/staff-symbol.cc
index 6fefc48..a22c24b 100644
--- a/lily/staff-symbol.cc
+++ b/lily/staff-symbol.cc
@@ -167,36 +167,6 @@ Staff_symbol::height  (SCM smob)
   return ly_interval2scm (y_ext);
 }
 
-bool
-Staff_symbol::on_line (Grob *me, int pos)
-{
-  SCM line_positions = me->get_property ("line-positions");
-  if (scm_is_pair (line_positions))
-    {
-      Real min_line = HUGE_VAL;
-      Real max_line = -HUGE_VAL;
-      for (SCM s = line_positions; scm_is_pair (s); s = scm_cdr (s))
-	{
-	  Real current_line = scm_to_double (scm_car (s));
-	  if (pos == current_line)
-	    return true;
-	  if (current_line > max_line)
-	    max_line = current_line;
-	  if (current_line < min_line)
-	    min_line = current_line;
-	
-	}
-      if (pos < min_line)
-	return (( (int) (rint (pos - min_line)) % 2) == 0);
-      if (pos > max_line)
-	return (( (int) (rint (pos - max_line)) % 2) == 0);
-
-      return false;
-    }
-  else
-    return ((abs (pos + line_count (me)) % 2) == 1);
-}
-
 ADD_INTERFACE (Staff_symbol,
 	       "This spanner draws the lines of a staff.  A staff symbol"
 	       " defines a vertical unit, the @emph{staff space}.  Quantities"
-- 
1.5.4.3

<<attachment: lily-88ca2c79.compare.jpeg>>

_______________________________________________
lilypond-user mailing list
lilypond-user@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-user

Reply via email to