Better width functions for the current arpeggio print functions. (issue4517051)

2011-05-10 Thread mtsolo

Reviewers: ,

Message:
I had added a Rietveld issue about this, but the example on my Rietveld
issue was bunk so I pulled it.  The current width function in
arpeggio.cc does not give the correct width for chord brackets or chord
slurs, nor does it give the correct width for arpeggios with arrows (I
think).  This should fix it w/o causing problems w/ the print function
triggering a vertical alignment.

It is kinda code-dupy, so if anyone has suggestions for how to roll this
into one unified function, I'll take it!

Description:
Better width functions for the current arpeggio print functions.

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

Affected files:
  M lily/arpeggio.cc
  M lily/include/arpeggio.hh


Index: lily/arpeggio.cc
diff --git a/lily/arpeggio.cc b/lily/arpeggio.cc
index  
b609f846d7a350fd6a541dec4bb2540285d4bcea..70c8c82a89438428cb6021504f227591bf804d71  
100644

--- a/lily/arpeggio.cc
+++ b/lily/arpeggio.cc
@@ -92,6 +92,13 @@ MAKE_SCHEME_CALLBACK (Arpeggio, print, 1);
 SCM
 Arpeggio::print (SCM smob)
 {
+  return internal_print (smob);
+}
+
+MAKE_SCHEME_CALLBACK (Arpeggio, internal_print, 1);
+SCM
+Arpeggio::internal_print (SCM smob)
+{
   Grob *me = unsmob_grob (smob);
   Interval heads = robust_scm2interval (me->get_property ("positions"),
Interval ())
@@ -156,10 +163,32 @@ Arpeggio::print (SCM smob)
 /* Draws a vertical bracket to the left of a chord
Chris Jackson  */

+/*
+  We have to do a callback, because print () triggers a
+  vertical alignment if it is cross-staff.
+*/
+MAKE_SCHEME_CALLBACK (Arpeggio, chord_bracket_width, 1);
+SCM
+Arpeggio::chord_bracket_width (SCM smob)
+{
+  Stencil *s = unsmob_stencil (internal_brew_chord_bracket (smob));
+  if (!s)
+return ly_interval2scm (Interval (0,0));
+
+  return ly_interval2scm (s->extent (X_AXIS));
+}
+
 MAKE_SCHEME_CALLBACK (Arpeggio, brew_chord_bracket, 1);
 SCM
 Arpeggio::brew_chord_bracket (SCM smob)
 {
+  return internal_brew_chord_bracket (smob);
+}
+
+MAKE_SCHEME_CALLBACK (Arpeggio, internal_brew_chord_bracket, 1);
+SCM
+Arpeggio::internal_brew_chord_bracket (SCM smob)
+{
   Grob *me = unsmob_grob (smob);
   Interval heads = robust_scm2interval (me->get_property ("positions"),
Interval ())
@@ -175,10 +204,32 @@ Arpeggio::brew_chord_bracket (SCM smob)
   return mol.smobbed_copy ();
 }

+/*
+  We have to do a callback, because print () triggers a
+  vertical alignment if it is cross-staff.
+*/
+MAKE_SCHEME_CALLBACK (Arpeggio, chord_slur_width, 1);
+SCM
+Arpeggio::chord_slur_width (SCM smob)
+{
+  Stencil *s = unsmob_stencil (internal_brew_chord_slur (smob));
+  if (!s)
+return ly_interval2scm (Interval (0,0));
+
+  return ly_interval2scm (s->extent (X_AXIS));
+}
+
 MAKE_SCHEME_CALLBACK (Arpeggio, brew_chord_slur, 1);
 SCM
 Arpeggio::brew_chord_slur (SCM smob)
 {
+  return internal_brew_chord_slur (smob);
+}
+
+MAKE_SCHEME_CALLBACK (Arpeggio, internal_brew_chord_slur, 1);
+SCM
+Arpeggio::internal_brew_chord_slur (SCM smob)
+{
   Grob *me = unsmob_grob (smob);
   SCM dash_definition = me->get_property ("dash-definition");
   Interval heads = robust_scm2interval (me->get_property ("positions"),
@@ -206,8 +257,11 @@ MAKE_SCHEME_CALLBACK (Arpeggio, width, 1);
 SCM
 Arpeggio::width (SCM smob)
 {
-  Grob *me = unsmob_grob (smob);
-  return ly_interval2scm (get_squiggle (me).extent (X_AXIS));
+  Stencil *s = unsmob_stencil (internal_print (smob));
+  if (!s)
+return ly_interval2scm (Interval (0,0));
+
+  return ly_interval2scm (s->extent (X_AXIS));
 }

 MAKE_SCHEME_CALLBACK (Arpeggio, pure_height, 3);
Index: lily/include/arpeggio.hh
diff --git a/lily/include/arpeggio.hh b/lily/include/arpeggio.hh
index  
094afe687b329c765ab2288a2e010b09be33cefc..7ca7a54b41e97eef964b9395dd97d284a0361417  
100644

--- a/lily/include/arpeggio.hh
+++ b/lily/include/arpeggio.hh
@@ -29,10 +29,15 @@ class Arpeggio
 public:
   static Grob *get_common_y (Grob *);
   DECLARE_SCHEME_CALLBACK (print, (SCM));
+  DECLARE_SCHEME_CALLBACK (internal_print, (SCM));
   DECLARE_SCHEME_CALLBACK (calc_positions, (SCM));
   DECLARE_SCHEME_CALLBACK (brew_chord_bracket, (SCM));
   DECLARE_SCHEME_CALLBACK (brew_chord_slur, (SCM));
+  DECLARE_SCHEME_CALLBACK (internal_brew_chord_bracket, (SCM));
+  DECLARE_SCHEME_CALLBACK (internal_brew_chord_slur, (SCM));
   DECLARE_SCHEME_CALLBACK (width, (SCM));
+  DECLARE_SCHEME_CALLBACK (chord_bracket_width, (SCM));
+  DECLARE_SCHEME_CALLBACK (chord_slur_width, (SCM));
   DECLARE_SCHEME_CALLBACK (pure_height, (SCM, SCM, SCM));
   DECLARE_GROB_INTERFACE();
 };



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Better width functions for the current arpeggio print functions. (issue4517051)

2011-05-11 Thread Carl . D . Sorensen

Seems reasonable to me.

I couldn't think of any way to generalize the internal call.

Carl


http://codereview.appspot.com/4517051/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Better width functions for the current arpeggio print functions. (issue4517051)

2011-05-11 Thread m...@apollinemike.com
On May 11, 2011, at 9:51 AM, carl.d.soren...@gmail.com wrote:

> Seems reasonable to me.
> 
> I couldn't think of any way to generalize the internal call.
> 
> Carl
> 
> 
> http://codereview.appspot.com/4517051/


Thanks!

There is a note in arpeggio.cc saying that width cannot be gleaned from the 
print function because it triggers a vertical alignment when arpeggios are 
cross-staff.  By turning the function into an internal function and calling it 
form outside functions, I'm assuming that this avoids triggering the vertical 
alignment, but I'm not sure this is the case.  Does anyone know 
how/where/when/why the vertical alignment is triggered?

Cheers,
MS
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Better width functions for the current arpeggio print functions. (issue4517051)

2011-05-11 Thread Neil Puttock
On 11 May 2011 15:18, m...@apollinemike.com  wrote:

> There is a note in arpeggio.cc saying that width cannot be gleaned from the 
> print function because it triggers a vertical alignment when arpeggios are 
> cross-staff.  By turning the function into an internal function and calling 
> it form outside functions, I'm assuming that this avoids triggering the 
> vertical alignment, but I'm not sure this is the case.  Does anyone know 
> how/where/when/why the vertical alignment is triggered?

Probably when the print function reads 'positions:
ly:arpeggio::calc-positions requests the common refpoint in the
Y-axis, which will be a VerticalAlignment for cross-staff arpeggios.

Cheers,
Neil

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Better width functions for the current arpeggio print functions. (issue4517051)

2011-05-11 Thread n . puttock


http://codereview.appspot.com/4517051/diff/1/lily/arpeggio.cc
File lily/arpeggio.cc (right):

http://codereview.appspot.com/4517051/diff/1/lily/arpeggio.cc#newcode98
lily/arpeggio.cc:98: MAKE_SCHEME_CALLBACK (Arpeggio, internal_print, 1);
Why are you exporting these internal functions?

http://codereview.appspot.com/4517051/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Better width functions for the current arpeggio print functions. (issue4517051)

2011-05-11 Thread mike
On May 11, 2011, at 12:02 PM, n.putt...@gmail.com wrote:

> 
> http://codereview.appspot.com/4517051/diff/1/lily/arpeggio.cc
> File lily/arpeggio.cc (right):
> 
> http://codereview.appspot.com/4517051/diff/1/lily/arpeggio.cc#newcode98
> lily/arpeggio.cc:98: MAKE_SCHEME_CALLBACK (Arpeggio, internal_print, 1);
> Why are you exporting these internal functions?
> 
> http://codereview.appspot.com/4517051/

The internal function idea is now bunk - you're right about the positions thing.

The issue is that, for the chord bracket and chord slur (and Bertrand's 
eventual chord brace, which hypothetically varies significantly in its X 
dimension as it gets larger), the width of the grob is dependent on knowing the 
extremal note head positions, which triggers a vertical alignment.  Any 
suggestions on how to get this information without triggering the vertical 
alignment?

Cheers,
MS


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Better width functions for the current arpeggio print functions. (issue4517051)

2011-05-11 Thread Neil Puttock
On 11 May 2011 19:11,   wrote:

> The issue is that, for the chord bracket and chord slur (and Bertrand's 
> eventual chord brace, which hypothetically varies significantly in its X 
> dimension as it gets larger), the width of the grob is dependent on knowing 
> the extremal note head positions, which triggers a vertical alignment.

Surely not the chord bracket?  It has a fixed width (line 171 of
arpeggio.cc), though perhaps it should be tweakable.

> Any suggestions on how to get this information without triggering the 
> vertical alignment?

Sorry, no.

Cheers,
Neil

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Better width functions for the current arpeggio print functions. (issue4517051)

2011-05-11 Thread m...@apollinemike.com

On May 11, 2011, at 2:19 PM, Neil Puttock wrote:

> On 11 May 2011 19:11,   wrote:
> 
>> The issue is that, for the chord bracket and chord slur (and Bertrand's 
>> eventual chord brace, which hypothetically varies significantly in its X 
>> dimension as it gets larger), the width of the grob is dependent on knowing 
>> the extremal note head positions, which triggers a vertical alignment.
> 
> Surely not the chord bracket?  It has a fixed width (line 171 of
> arpeggio.cc), though perhaps it should be tweakable.
> 

Sorry - yes, you're right.  I amend my comment to chord slurs and braces.

>> Any suggestions on how to get this information without triggering the 
>> vertical alignment?
> 
> Sorry, no.
> 

I'll try to think of something...

~Mike
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel