Re: accidental changes: review
On Tue, 25 Sep 2007, Han-Wen Nienhuys wrote: 2007/9/25, Rune Zedeler [EMAIL PROTECTED]: + int octave_; + int notename_; + Rational alteration_; Why don't you use a Pitch object for this combination? You would get Scale* for free. Primarily because the two values octave_ and has_octave_ are very closely related. They really /should/ be joined to an int option - if such a thing did exist in c++. you might want to look into making octave optional for pitch; I'm not sure if it is worth the trouble, but it would make key signatures more logical to specify. Otherwise, you could use an int* I am just curious: Why not introducing a new class Octaveless_pitch or Octave_Pitch (i.e. pitch within an octave) that contains all of Pitch except for the octaves, and then making Pitch a subclass of Octaveless_pitch that just adds the octaves to its superclass? Greetings, Juergen ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: accidental changes: review
On 9/26/07, Juergen Reuter [EMAIL PROTECTED] wrote: On Tue, 25 Sep 2007, Han-Wen Nienhuys wrote: 2007/9/25, Rune Zedeler [EMAIL PROTECTED]: + int octave_; + int notename_; + Rational alteration_; Why don't you use a Pitch object for this combination? You would get Scale* for free. Primarily because the two values octave_ and has_octave_ are very closely related. They really /should/ be joined to an int option - if such a thing did exist in c++. you might want to look into making octave optional for pitch; I'm not sure if it is worth the trouble, but it would make key signatures more logical to specify. Otherwise, you could use an int* I am just curious: Why not introducing a new class Octaveless_pitch or Octave_Pitch (i.e. pitch within an octave) that contains all of Pitch except for the octaves, and then making Pitch a subclass of Octaveless_pitch that just adds the octaves to its superclass? [The (American) academic terms for these are pitch-class for the octaveless notenames and pitch for actual pitches specified in a certain octave. Robert Morris's Composition with Pitch-classes [1987; Yale] is a nice example. So here, Pitch_class would be the superclass from which Pitch would inherit.] -- Trevor Bača [EMAIL PROTECTED] ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: accidental changes: review
2007/9/26, Juergen Reuter [EMAIL PROTECTED]: I am just curious: Why not introducing a new class Octaveless_pitch or Octave_Pitch (i.e. pitch within an octave) that contains all of Pitch except for the octaves, and then making Pitch a subclass of Octaveless_pitch that just adds the octaves to its superclass? There would only be one scheme type, the one for the base class, so this will require some extra typechecking for the Scheme interfaces -- Han-Wen Nienhuys - [EMAIL PROTECTED] - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
accidental changes: review
- SCM key = scm_cons (scm_from_int (o), scm_from_int (n));
+ Duration *dur = unsmob_duration (note-get_property (duration));
+
+ SCM smp = get_property (measurePosition);
+ Moment mp = robust_scm2moment (smp, Moment (0));
+ /*
+ TODO: Check this. Is this correct? -rz :
+ */
+ Moment end_mp = mp.grace_part_ Rational(0)
+ ? Moment(mp.main_part_, mp.grace_part_+dur-get_length())
+ : Moment(mp.main_part_+dur-get_length(), mp.grace_part_);
grace_part_ can never be larger than zero, so you could simplify to
: Moment(mp.main_part_+dur-get_length(), 0);
other than that, it looks ok.
--- /dev/null
+ int octave_;
+ int notename_;
+ Rational alteration_;
Why don't you use a Pitch object for this combination? You would get
Scale* for free.
+ bool has_position_;
Can you document what this does?
--- a/lily/key-engraver.cc
+++ b/lily/key-engraver.cc
@@ -15,6 +15,7 @@
#include protected-scm.hh
#include staff-symbol-referencer.hh
#include stream-event.hh
+#include key-entry.hh
#include translator.icc
@@ -72,36 +73,56 @@ Key_engraver::create_key (bool is_default)
|| key == SCM_EOL)
could you add a comment on what is happening here?
{
-SCM new_alter_pair = scm_assoc (scm_caar (s), key);
-Rational old_alter = robust_scm2rational (scm_cdar (s), 0);
-if (new_alter_pair == SCM_BOOL_F
+Key_entry *old_entry = Key_entry::unsmob (scm_car (s));
+Key_entry *new_entry = NULL;
+for (SCM t = key; scm_is_pair (t); t = scm_cdr (t))
+ {
+Key_entry *entry = Key_entry::unsmob (scm_car (t));
+if ( old_entry-get_notename () == entry -
get_notename ())
no space after (
+ {
+new_entry = entry;
+break;
+ }
+ }
+Rational old_alter = old_entry-get_alteration ();
+Rational new_alter = new_entry-get_alteration ();
+SCM old_alterpair = old_entry - to_name_alter_pair ();
no space around -
- item_-set_property (alteration-alist, key);
+ SCM key_scm = scm_list_copy (key);
+ for (SCM s = key_scm; scm_is_pair (s); s = scm_cdr (s))
+ {
+Key_entry *entry = Key_entry::unsmob (scm_car (s));
+*SCM_CARLOC (s) = entry - to_name_alter_pair ();
del space around -
+
+
+IMPLEMENT_TYPE_P (Key_entry, ly:key-signature-entry?);
+
+SCM
+Key_entry::mark_smob (SCM x)
+{
+ return SCM_EOL; // FIXME: I don't understand what this function is
supposed to do. This is just blindly coppied from moment.cc
call scm_gc_mark() on all SCM values that are inside the smob. You can
return one of them iso. scm_gc_mark()ing them.
+LY_DEFINE (ly_make_keysig_entry, ly:make-keysig-entry,
+ 2, 3, 0, (SCM note, SCM alter, SCM octave, SCM barnumber, SCM
measurepos),
+ An entry for key signature. Key signatures consists of lists of
these. The three last arguments are used for local key changes.)
+{
please break string and wrap lines
+ LY_ASSERT_TYPE (scm_is_integer, note, 1);
+ LY_ASSERT_TYPE (scm_is_integer, alter, 2);
+
+ if (octave == SCM_UNDEFINED)
+{
+ Key_entry e (scm_to_int (note), scm_to_int (alter));
+
+ return e.smobbed_copy ();
+}
+ else
+{
+ LY_ASSERT_TYPE (scm_is_integer, octave, 3);
+ LY_ASSERT_TYPE (scm_is_integer, barnumber, 4);
+ LY_ASSERT_SMOB (Moment, measurepos, 5);
can you check for unsmob_moment() here?
/* FIXME: why is octave == 0 and default not middleC ? */
+/* Because otherwise Pitch () would not be a zero element -
+ e.g. implementation of negated () would not work. -rz ! */
Huh? I recall that octave == 0 is actually middle C. The strange thing
is that c' (with one quote) is is encoded as octave 0.
--
Han-Wen Nienhuys - [EMAIL PROTECTED] - http://www.xs4all.nl/~hanwen
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: accidental changes: review
Thanks for comments. Han-Wen Nienhuys skrev: + int octave_; + int notename_; + Rational alteration_; Why don't you use a Pitch object for this combination? You would get Scale* for free. Primarily because the two values octave_ and has_octave_ are very closely related. They really /should/ be joined to an int option - if such a thing did exist in c++. Therefore I find it very errorprone to move the octave_ into another object without has_octave_. The pitch object will not make sense if has_octave_==false. We have LY_DEFINE (ly_make_keysig_entry, ly:make-keysig-entry, 2, 3, 0, (SCM note, SCM alter, SCM octave, SCM barnumber, SCM measurepos), An entry for key signature. Key signatures consists of lists of these. The three last arguments are used for local key changes.) It will not make sense to join the three first together, because the two first are related, and the three last are related. /* FIXME: why is octave == 0 and default not middleC ? */ +/* Because otherwise Pitch () would not be a zero element - + e.g. implementation of negated () would not work. -rz ! */ Huh? I recall that octave == 0 is actually middle C. The strange thing is that c' (with one quote) is is encoded as octave 0. Just to ensure I understand you correctly: The FIXME is wrong? It /should/ say FIXME: Why does octave == 0 refer to middleC-octave and not the octave below? -Rune ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: accidental changes: review
2007/9/25, Rune Zedeler [EMAIL PROTECTED]: + int octave_; + int notename_; + Rational alteration_; Why don't you use a Pitch object for this combination? You would get Scale* for free. Primarily because the two values octave_ and has_octave_ are very closely related. They really /should/ be joined to an int option - if such a thing did exist in c++. you might want to look into making octave optional for pitch; I'm not sure if it is worth the trouble, but it would make key signatures more logical to specify. Otherwise, you could use an int* /* FIXME: why is octave == 0 and default not middleC ? */ +/* Because otherwise Pitch () would not be a zero element - + e.g. implementation of negated () would not work. -rz ! */ Huh? I recall that octave == 0 is actually middle C. The strange thing is that c' (with one quote) is is encoded as octave 0. Just to ensure I understand you correctly: The FIXME is wrong? I think the fixme should just go away. -- Han-Wen Nienhuys - [EMAIL PROTECTED] - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
