Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
On 2012/12/31 00:07:12, thomasmorley65 wrote: I thought about extending \draw-line with a 'style-property. But in extreme cases you would have to write: \markup { \override #'(style . dashed) \override #'(on . 0.5) \override #'(off . 0.2) \override #'(thickness . 2) \draw-line #'(4 . 1) } This seems a bit overlaoded to me. I don't consider \markup { \override #'(on . 0.5) \override #'(off . 0.2) \override #'(thickness . 2) \draw-dashed-line #'(4 . 1) } considerably less overloaded. Ok, this will reduce the needed \overrides only by one, but I think any \override less would be nice. Hm. And I'm thinking about writing additional commands for zigzag- and trill-style lines... After all, lines are not the only things that might be desirable to do with a different line style. Do you have anything particular in mind? Boxes, circles, underlines, ... No, nothing particular, but it's not uncommon. https://codereview.appspot.com/7029045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: PO: remove duplicates entries for hh and cc from ALL_PO_SOURCES (issue 7029043)
LGTM https://codereview.appspot.com/7029043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Doc: NR 5.3 Add \single command (issue 6742057)
https://codereview.appspot.com/6742057/diff/7001/Documentation/notation/changing-defaults.itely File Documentation/notation/changing-defaults.itely (right): https://codereview.appspot.com/6742057/diff/7001/Documentation/notation/changing-defaults.itely#newcode1626 Documentation/notation/changing-defaults.itely:1626: * Set and unset:: On 2012/12/30 09:14:20, dak wrote: Not sure whether @code{...} can be used within section titles (possibly causing problems in PDF indices?). If it can, using @code{\set} etc would be appropriate. It can't be used for node names, but it _can_ be used in the section titles, whcih is the part that generates the visible output. We already use this trick, and James keeps on using it in the patch. https://codereview.appspot.com/6742057/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: IR: Improve performer documentation (issue 6868047)
David, On 2012/12/02 11:11:54, dak wrote: https://codereview.appspot.com/6868047/diff/1/scm/document-translation.scm File scm/document-translation.scm (right): Sorry for resurrecting this a month late, I think I only quite understood yesterday what you actually meant. It is somewhat obsolete for this patch, since I have gone on to work on a more complete one (see http://code.google.com/p/lilypond/issues/detail?id=3003#c3), but your point will be relevant for my new work again, so I guess I better start defending my case now. https://codereview.appspot.com/6868047/diff/1/scm/document-translation.scm#newcode21 scm/document-translation.scm:21: ;; type predicates ly:engraver? and ly:performer?. I think that this approach is not really helping as it is inflexible and it becomes harder to see crossovers. At one point of time, we will likely get translators that are used to output MusicXML, and several translators can be used as either translator or performer (and some, like the autobeamer, will also be used in MusicXML preparation). So I'd rather make separate chapters for translators used in $defaultlayout (default engravers) and those used in $defaultmidi (default performers), allow for duplicates but mention when the engraver is also being used as performer and vice versa, and make a separate list of translators not used by default (are there any?). Classifying translators by the output definitions in which they appear by default is not a good idea in my opinion. * Translators do not need to belong to any context by default, since they may implement optional functionality. Yes, there are already some, for example Ambitus_engraver, Completion_heads_engraver, Grid_line_span_engraver or Page_turn_engraver. These still clearly belong to one particular backend; all current examples to the graphical one, but I see no reason to assume there will never be optional performers (and, in the future, XML transcribers, however you will call them) too. I don't think lumping these all together in a separate category makes any sense. * Allowing for duplicate translator documentation nodes would not be logical. Currently this would affect only Timing_translator, but again I see no reason to assume this will always remain the case. Each translator is uniquely defined, has a single set of associated documentation data and, judging from Timing_translator, will work identically wherever it makes sense to \consist it. Specifically, if a translator can appear in more than one output definition, it will operate on a backend-independent level. Then why document it at backend-specific levels, with duplicated nodes that would have to have identical text? [The situation is different for contexts. Layout and MIDI contexts of the same name have separate definitions with separate docstrings, since each output definition comes with its own complete context hierarchy. They are mostly isomorphic, but conceptually this is just a manually enforced convention to allow you to use the same LilyPond score definitions for both graphical and MIDI output. Deviations already exist: GregorianTranscriptionStaff, GregorianTranscriptionVoice, NoteNames, PetrucciStaff and PetrucciVoice are defined as layout contexts, but not as MIDI contexts; ChordNameVoice exists as a MIDI context only. Some of these might actually be oversights (?), but probably not all. That it why, in my current personal development version of the Internals Reference, I have started to document the context hierarchies separately. You can have a look at the current state at http://src.johannesrohrer.de/lilypond/Documentation/internals/contexts.html.] My opinion is that you should classify translators based on what they do (if at all), and looking for where you \consist them by default is not a reliable way to find this out. The best ones I currently see are still to either look at a translator's base class (requires new specific type predicates, which I hesitate to add myself), or at its name. Hence I stand by my original code. Best regards, and happy new year to everyone, Johannes Rohrer https://codereview.appspot.com/6868047/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Issue 3073: Context definitions and context mods ignore \unset (issue 7038044)
LGTM https://codereview.appspot.com/7038044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
Harm-- This looks great! Thank you for the 'full-length option. I can't be alone in hating lines ending with incomplete dashes. All I have is a suggestion or two, and some quibbles. Besides what I've pointed out inline, I should mention that I got a number of whitespace errors when I applied your patch. There are a number of spots where you should trim the ends of lines (mostly in the .scm file). https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode155 scm/define-markup-commands.scm:155: If @code{full-length} is set @code{#t} (default) the dashed-line extends to the set to #t https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode156 scm/define-markup-commands.scm:156: whole length given by @var{dest}, without white space at begin/end. beginning or end. https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode169 scm/define-markup-commands.scm:169: (let* (;; Get the line-thickness. I think your variable name is sufficient--no comment needed. https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode181 scm/define-markup-commands.scm:181: (begin Branches of if expression should be aligned with test (here and elsewhere). https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode195 scm/define-markup-commands.scm:195: (new-off (/ (- line-length corr (* (+ 1 guess) on)) guess)) Why not use (1+ guess) https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode231 scm/define-markup-commands.scm:231: #f) Could omit the boolean--value will be unspecified. https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode233 scm/define-markup-commands.scm:233: ;; If `on´ or `off´ is negative, or both, `on´ and `off´ equals zero a Possibly ...or the sum of `on' and `off' equals [is] zero... https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode244 scm/define-markup-commands.scm:244: #f) Here again, you could omit the alternate. https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode248 scm/define-markup-commands.scm:248: (interval-widen (ordered-cons 0 x) half-thick) You do such a thorough job of commenting everything, but you don't explain why you need `half-thick' here. https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode264 scm/define-markup-commands.scm:264: Manual settings for @code{off} is possible to get larger or smaller space are possible https://codereview.appspot.com/7029045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Issue 3073: Context definitions and context mods ignore \unset (issue 7038044)
https://codereview.appspot.com/7038044/diff/1/input/regression/property-unset.ly File input/regression/property-unset.ly (right): https://codereview.appspot.com/7038044/diff/1/input/regression/property-unset.ly#newcode8 input/regression/property-unset.ly:8: systems here should revert to the @samp{Score}-level violin clef. This fails make check, I am wondering if there needs to be some 'escaping' of texinfo command chars? or is it the space between @code and {\\unset}? https://codereview.appspot.com/7038044/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Introducing two new markup-commands: draw-dashed-line and draw-dotted-line. (issue 7029045)
On 2012/12/31 14:59:45, david.nalesnik wrote: Harm-- This looks great! Thank you for the 'full-length option. I can't be alone in hating lines ending with incomplete dashes. Hi David, thanks for reviewing! All I have is a suggestion or two, and some quibbles. Besides what I've pointed out inline, I should mention that I got a number of whitespace errors when I applied your patch. There are a number of spots where you should trim the ends of lines (mostly in the .scm file). Done. (Hope so) https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode155 scm/define-markup-commands.scm:155: If @code{full-length} is set @code{#t} (default) the dashed-line extends to the set to #t Done https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode156 scm/define-markup-commands.scm:156: whole length given by @var{dest}, without white space at begin/end. beginning or end. Done https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode169 scm/define-markup-commands.scm:169: (let* (;; Get the line-thickness. I think your variable name is sufficient--no comment needed. Ok. Deleted. https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode181 scm/define-markup-commands.scm:181: (begin Branches of if expression should be aligned with test (here and elsewhere). Done https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode195 scm/define-markup-commands.scm:195: (new-off (/ (- line-length corr (* (+ 1 guess) on)) guess)) Why not use (1+ guess) Done https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode231 scm/define-markup-commands.scm:231: #f) Could omit the boolean--value will be unspecified. Done https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode233 scm/define-markup-commands.scm:233: ;; If `on´ or `off´ is negative, or both, `on´ and `off´ equals zero a Possibly ...or the sum of `on' and `off' equals [is] zero... Changed https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode244 scm/define-markup-commands.scm:244: #f) Here again, you could omit the alternate. Done https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode248 scm/define-markup-commands.scm:248: (interval-widen (ordered-cons 0 x) half-thick) You do such a thorough job of commenting everything, but you don't explain why you need `half-thick' here. Comment added. https://codereview.appspot.com/7029045/diff/1/scm/define-markup-commands.scm#newcode264 scm/define-markup-commands.scm:264: Manual settings for @code{off} is possible to get larger or smaller space are possible Done. https://codereview.appspot.com/7029045/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
invalid read in Context::properties_dict()
I have a reproducible segfault, but not a tiny example. (If anyone wants to reproduce this, ask me to prepare a tgz for you to download from my web site.) Is there an expert here who can tell me how likely it is that the Context properties actually referred to the object released in Skyline::Skyline, or if something more obscure happened? This is with the latest source built in LilyDev, but the same input causes a bus error with 2.16 on OS X. Valgrind and gdb output follow. Thanks, -- Dan VALGRIND OUTPUT (ABRIDGED) ==13752== Invalid read of size 4 ==13752==at 0x80CA0E9: Context::properties_dict() const (context.cc:68) ==13752==by 0x80CBC53: Context::internal_get_property(scm_unused_struct*) const (context.cc:445) ==13752==by 0x80CBC91: Context::internal_get_property(scm_unused_struct*) const (context.cc:449) ==13752==by 0x80CBC91: Context::internal_get_property(scm_unused_struct*) const (context.cc:449) ==13752==by 0x80CBC91: Context::internal_get_property(scm_unused_struct*) const (context.cc:449) ==13752==by 0x80C5DF3: ly_context_property(scm_unused_struct*, scm_unused_struct*, scm_unused_struct*) (context-scheme.cc:104) ==13752==by 0x4094759: scm_gsubr_apply (in /usr/lib/libguile.so.17.3.1) ==13752==by 0x407DB73: scm_dapply (in /usr/lib/libguile.so.17.3.1) ==13752==by 0x407E726: ??? (in /usr/lib/libguile.so.17.3.1) ==13752==by 0x407F02C: ??? (in /usr/lib/libguile.so.17.3.1) ==13752==by 0x407DBEA: scm_dapply (in /usr/lib/libguile.so.17.3.1) ==13752==by 0x407C507: scm_apply (in /usr/lib/libguile.so.17.3.1) ==13752== Address 0xcc33d90 is 32 bytes inside a block of size 40 free'd ==13752==at 0x4024851: operator delete(void*) (vg_replace_malloc.c:387) ==13752==by 0x824F921: Skyline::Skyline(std::vectorSkyline_pair, std::allocatorSkyline_pair const, Direction) (new_allocator.h:95) ==13752==by 0x824881C: Skyline_pair::Skyline_pair(std::vectorSkyline_pair, std::allocatorSkyline_pair const) (skyline-pair.cc:42) ==13752==by 0x807292E: Axis_group_interface::skyline_spacing(Grob*, std::vectorGrob*, std::allocatorGrob* ) (axis-group-interface.cc:884) ==13752==by 0x807351D: Axis_group_interface::calc_skylines(scm_unused_struct*) (axis-group-interface.cc:396) ==13752==by 0x407D63E: scm_dapply (in /usr/lib/libguile.so.17.3.1) ==13752==by 0x407C507: scm_apply (in /usr/lib/libguile.so.17.3.1) ==13752==by 0x4081A40: scm_call_1 (in /usr/lib/libguile.so.17.3.1) ==13752==by 0x811C1BB: Grob::try_callback_on_alist(scm_unused_struct**, scm_unused_struct*, scm_unused_struct*) (grob-property.cc:231) ==13752==by 0x811C392: Grob::internal_get_property(scm_unused_struct*) const (grob-property.cc:188) ==13752==by 0x806F544: Axis_group_interface::generic_group_extent(Grob*, Axis) (axis-group-interface.cc:440) ==13752==by 0x806F5B8: Axis_group_interface::height(scm_unused_struct*) (axis-group-interface.cc:365) GDB OUTPUT (ABRIDGED) Calculating page and line breaks (20 possible page breaks)...[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] Drawing systems... Program received signal SIGSEGV, Segmentation fault. 0x080cd5be in Scheme_hash_table::unsmob (s=0x30) at /home/dan/lilypond-git/lily/include/scm-hash.hh:62 62DECLARE_SMOBS (Scheme_hash_table); (gdb) print *this No symbol this in current context. (gdb) up #1 0x080ca0f4 in Context::properties_dict (this=0x8d4daa8) at /home/dan/lilypond-git/lily/context.cc:68 68return Scheme_hash_table::unsmob (properties_scm_); (gdb) print *this $1 = {_vptr.Context = 0x38254fb6, static smob_name_ = 0x8329c00 Context, static smob_tag_ = 10623, self_scm_ = 0x400dabb6, protection_cons_ = 0x689174e, client_count_ = 1074802671, infant_event_ = 0xdbbf9fb1, daddy_context_ = 0x40104870, definition_ = 0x0, definition_mods_ = 0x0, properties_scm_ = 0x30, context_list_ = 0x31, accepts_list_ = 0x8d4db00, aliases_ = 0x9239790, implementation_ = 0x0, id_string_ = { static npos = 4294967295, _M_dataplus = {std::allocatorchar = {__gnu_cxx::new_allocatorchar = {No data fields}, No data fields}, _M_p = 0x8000 Address 0x8000 out of bounds}}, event_source_ = 0xa99b6f5c, events_below_ = 0x3ff50f10, ancestor_lookup_ = 0x2ed35222} (gdb) up #2 0x080cbc54 in Context::internal_get_property (this=0x8d4daa8, sym=0xb7306af0) at /home/dan/lilypond-git/lily/context.cc:445 445 if (properties_dict ()-try_retrieve (sym, val)) (gdb) print *this $2 = {_vptr.Context = 0x38254fb6, static smob_name_ = 0x8329c00 Context, static smob_tag_ = 10623, self_scm_ = 0x400dabb6, protection_cons_ = 0x689174e, client_count_ = 1074802671, infant_event_ = 0xdbbf9fb1, daddy_context_ = 0x40104870, definition_ = 0x0, definition_mods_ = 0x0, properties_scm_ = 0x30, context_list_ = 0x31, accepts_list_ = 0x8d4db00, aliases_ = 0x9239790, implementation_ = 0x0, id_string_ = { static npos = 4294967295, _M_dataplus =
Re: invalid read in Context::properties_dict()
When the segfault occurs, multiple contexts in the call stack have already had their mark_smob() method called. I guess that means they're all invalid, and their memory might have been reallocated. Is that correct? For a while, I have been using the following scheme code to define instrument names for voices together with the notes for the voice, e.g. aNotes = \relative g { #(set-voice-name T1) c4 d e f . . . } Did something change in 2.16 that would make this start to segfault, or was it always invalid and just now started failing by chance? Thanks. #(define-markup-command (on-the-fly-context layout props procedure context arg) (symbol? markup?) Apply the @var{procedure} markup command to @var{arg}. @var{procedure} should take a single argument. (let* ((anonymous-with-signature (lambda (layout props context arg) (procedure layout props context arg (set-object-property! anonymous-with-signature 'markup-signature (list ly:context? markup?)) (interpret-markup layout props (list anonymous-with-signature context arg #(define (set-voice-name newName) (let ((m (make-music 'ApplyContext))) (define (do-it context) (let* ((voiceNames (ly:context-property context 'voiceNames))) (if (list? voiceNames) (set! voiceNames (append voiceNames (list newName))) (set! voiceNames (list newName))) (ly:context-set-property! context 'voiceNames voiceNames))) (set! (ly:music-property m 'procedure) do-it) (context-spec-music m 'Staff))) #(define-public (print-voice-names-or-default layout props context default) (let ((voiceNames (ly:context-property context 'voiceNames)) % segfault occurs in ly:context-property (props (cons (list '(baseline-skip . 2.4)) props))) (if (null? voiceNames) (interpret-markup layout props (make-column-markup default)) (interpret-markup layout props (make-column-markup voiceNames) #(define-public (set-auto-instrument-name defaultNames) (let ((m (make-music 'ApplyContext))) (define (do-it context) (ly:context-set-property! context 'instrumentName (cons on-the-fly-context-markup (list print-voice-names-or-default context defaultNames (set! (ly:music-property m 'procedure) do-it) (context-spec-music m 'Staff))) -- Dan ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel