[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: First attempt at new accidentals in git
From: |
Han-Wen Nienhuys |
Subject: |
Re: First attempt at new accidentals in git |
Date: |
Mon, 12 Nov 2007 00:13:42 -0200 |
2007/11/11, Rune Zedeler <address@hidden>:
> I have put the first attempt at moving accidentals to scheme in dev/rune
> branch.
> (Please check that VERSION is 2.11.32.rz3 - I have had some git-problems)
>
> * Split pitch into pitchclass and pitch - pitchclass not containing
> octave information.
> Han-Wen please comment on code. I had some problems with getting guile
> to accept hierarchical smobs.
>
what's
midt i at ændre pitch og tilføje pitchclass
first round of review; focus on pitch representation:
> --- a/lily/context-def.cc
> +++ b/lily/context-def.cc
> @@ -223,13 +223,16 @@ Context_def::path_to_acceptable_context (SCM type_sym,
> Output_def *odef) const
> {
> Context_def *g = accepteds[i];
>
> - vector<Context_def*> result
> - = g->path_to_acceptable_context (type_sym, odef);
> - if (result.size () && result.size () < best_depth)
> + if (g!=this)
style: spaces around !=
I think this was discussed on the list before that this only handles
simple minded cycles? If yes, please add a comment.
> --- a/lily/include/key-entry.hh
> +++ b/lily/include/key-entry.hh
> @@ -13,31 +13,34 @@
> #include "smobs.hh"
> #include "moment.hh"
> #include "rational.hh"
> +#include "pitch.hh"
> +
> +/*
> +FIXME!!!
> +Does it work with simple smobs when it contains a reference to a pitchclass_?
> +*/
Yes, however, you must make sure that the Pitchclass* does not come
from a SCM, ie.
pitchclass_ = unsmob_pitchclass (scm_obj) // wrong, pitchclass_
// may be deleted if scm_obj becomes unreachable
pitchclass_ = new Pitchclass (*unsmob_pitchclass (scm_obj)) // wrong
at the end in ~Key_entry() you need to delete pitchclass_. Also, don't
forget copy ctor / = operator(), if applicable. If not, please make
copy ctor private.
> /** A "tonal" pitch. This is a pitch used in diatonal western music
> @@ -22,71 +23,53 @@
> alteration).
>
> */
> -class Pitch
> +class Pitch : public Pitchclass
> {
> private:
> int octave_;
> Pitch negated () const;
> string to_string () const;
> + SCM virtual_smobbed_copy () { // AAARGH!
> + return smobbed_copy ();
> + }
> DECLARE_SIMPLE_SMOBS (Pitch);
This looks bad. You have to remove Pitch as a smobtype, and then
create an unsmob_pitch, which does
dynamic_cast<Pitch*> (unsmob_pitchclass (obj))
Then, double check that all relevant functions distinguish between
unsmob_pitch() and unsmob_pitchclass() correctly.
You should also modify the print routine for the Pitch(class) smob so that it
will print
#<Pitch c''>
vs
#<Pitchclass c>
depending on the type.
> @@ -0,0 +1,79 @@
> +#ifndef MUSICAL_PITCHCLASS_HH
PITCHCLASS_HH
> +
> +INSTANTIATE_COMPARE (Pitchclass, Pitchclass::compare);
that puzzles me: how can you compare pitchclasses, which are
essentially cyclic?
> Key_entry::Key_entry (int n, Rational a)
> Key_entry::Key_entry (int n, Rational a, int o, int b, Moment mp)
> Key_entry::Key_entry ()
I dislike this. I prefer a default ctor, and then use set_XX methods
to set appropriate members. This is a lot of repetitive code.
> +Key_entry::~Key_entry ()
> +{
> + // TODO: what here?
> + //cout << "Killing entry" << endl;
> +
> +}
2 choices:
1. use an SCM for keeping the Pitchclass reference; mark it in
Key_entry::mark(). Deletion happens automatically. You can't modify it
(the data may be shared.)
2. use a C++ pointer for keeping the Pitchclass. It has to be copied
and deleted in the C++ way.
> {
> - return notename_ + octave_ * scale_->step_tones_.size ();
> + return notename_ + octave_ * (int)scale_->step_tones_.size ();
prefer int(xxx)
> +Pitchclass *
> +unsmob_pitch_or_pitchclass (SCM s, int number)
> +{
> + Pitchclass *pc = unsmob_pitch (s);
> + if (pc==NULL)
if (!pc)
> * Moved accidental rules to scheme
> Now the user can write his own accidental rules in the .ly if he is not
> satisfied with the existing ones.
>
> If one for instance should wish to add a cautionary accidental at the
> start of each measure (hmm), then one may do like this:
>
> %%% BEGIN
> \version "2.11.32"
>
> #(define (start-of-measure-acc-rule context pitch barnum measurepos)
> (cons #f (equal? measurepos (ly:make-moment 0 1))))
>
> mus = {
> \key des \major
> \transpose c c' {
> g16 g g g' g' d' d' d' g' g' f f gis gis g g' ges1
> }
> }
> <<
> \new Staff {
> \set Staff.autoCautionaries = #(list 'Staff start-of-measure-acc-rule)
> \relative c' { c4 d e f g gis a ais b2 c }
> }
> >>
> %%% END
>
> * Added accidental-rules 'neo-modern and 'neo-modern-cautionary - adding
> even more accidentals to the modern accidentals. Notes different from
> the key signature now always gets an accidental if they do not directly
> follow a note of the same pitch.
>
> * Added accidental-rule 'dodecaphonic - thereby obsoleting
> http://lsr.dsi.unimi.it/LSR/Item?id=314 :-)
>
> TODO:
>
> * convert-ly? I don't think this is nessesary. The change is
> downwards-compatible as long as the .ly do not mess around with
> 'autoAccidentals and 'autoCautionaries directly. Perhaps we could add a
> simple warning if these properties are accessed.
>
> * Make better use of pitchclass. Currently it is only used in the
> key-signature.
>
> * Some fixme's still in the code. Primarily regarding garbagecollection.
> Currently it probably leaks like mad.
>
> * Bugtesting, bugtesting, bugtesting.
>
> -Rune
>
>
> _______________________________________________
> lilypond-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/lilypond-devel
>
--
Han-Wen Nienhuys - address@hidden - http://www.xs4all.nl/~hanwen