Obfuscated, but so was the old code. I'll leave here the notes I used to decipher it, and maybe come back later to clean up the code.
https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc File lily/accidental-placement.cc (right): https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode123 lily/accidental-placement.cc:123: Real ape_priority (Accidental_placement_entry const *a) Evil name. The return values are not used as a priority. Formerly, "ape_highest_extent()" would have been appropriate, because it was not even then a priority. Rather, the highest ape was placed first, then the lowest, then the second highest... https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode129 lily/accidental-placement.cc:129: bool ape_less (Accidental_placement_entry *const &a, Bad name. The apes were formerly sorted by "priority" (really height) so this should have been ape_lower(). Now, they are sorted by number of accidentals in the ape, then by "priority" (really height) to break ties. https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode186 lily/accidental-placement.cc:186: vector<Accidental_placement_entry *> asc = *apes; accidental sorted-columns (asc) https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode189 lily/accidental-placement.cc:189: // we do the staggering below based on size 'size' must mean number of accidentals in the ape, but you don't stagger based on 'size', you prioritize based on 'size', then within each groups of apes with the same 'size', you stagger the based on their height. https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode190 lily/accidental-placement.cc:190: // this ensures that if a placement has 4 entries, it will An accidental-placement-entry (ape) contains the accidentals that get placed together (same pitch names in different octaves) so it would rarely have 4 entries -- more likely just 2 accidentals. https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode191 lily/accidental-placement.cc:191: // always be closer to the NoteColumn than a placement with 1 Not always closer, merely placed before. In one of the regression tests, a single accidental had room to slide closer to a couple previously-placed octave-pairs of accidentals. https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode193 lily/accidental-placement.cc:193: // while still preserving octave alignment Nothing in here deals with octaves. The grouping seems to be done in scheme, so here we can only promise to keep the apes together. https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode194 lily/accidental-placement.cc:194: vector<vector<Accidental_placement_entry *> > ascs; accidental sorted-column sets (ascs) https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode196 lily/accidental-placement.cc:196: vsize sz = INT_MAX; Number of accidentals in a group to be placed as a column (sz) https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode197 lily/accidental-placement.cc:197: for (vsize i = 0; i < asc.size (); i++) The "accidental sorted-columns" (asc) is sorted in an order not directly related to the order of placement. We need to break it up in to subsets (ascs) that contain columns with similar numbers of accidentals in them. Each sub-set is 'staggered' but the subset containing groups-of-2 accidentals placed before the subset with single accidentals. https://codereview.appspot.com/7101045/diff/1/lily/accidental-placement.cc#newcode210 lily/accidental-placement.cc:210: int parity = 1; Not really "parity", but which end, highest-or-lowest, of the sub-lists of accidentals we pick from. Apparently, when we finish with the accidentals in octaves, we want to pick the highest of the remaining accidentals to place next. https://codereview.appspot.com/7101045/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
