Re: Implements multiple-line non-cross-staff glissandi (issue4527086)

[address@hidden]

On Jun 27, 2011, at 3:14 PM, Colin Campbell wrote:

> On 11-06-27 01:16 AM, address@hidden wrote:
>> 
>> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly
>>  
>> File input/regression/glissando-broken-multiple.ly (right):
>> 
>> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly#newcode7
>>  
>> input/regression/glissando-broken-multiple.ly:7: ragged-right = ##T }
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> ##t
>>> }
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly#newcode10
>>  
>> input/regression/glissando-broken-multiple.ly:10: \override Glissando
>> #'breakable = ##t
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> you probably also want after-line-breaking = ##t to show the last
>> glissando
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly#newcode11
>>  
>> input/regression/glissando-broken-multiple.ly:11: d1 \glissando |
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> d1\glissando
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/input/regression/glissando-broken-multiple.ly#newcode15
>>  
>> input/regression/glissando-broken-multiple.ly:15: c,1^\ff\trill
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> is the dynamic relevant?
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm
>> File scm/output-lib.scm (right):
>> 
>> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode798
>>  
>> scm/output-lib.scm:798: (define-public (glissando::before-line-breaking
>> grob)
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> use assoc-set! to simplify this (i.e., just mutate bound-details
>> instead of
>>> rebuilding it)
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode809
>>  
>> scm/output-lib.scm:809: (ly:grob-set-property! grob 'bound-details bd)))
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> (set! (ly:grob-property grob 'bound-details)
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode811
>>  
>> scm/output-lib.scm:811: (define-public
>> (glissando::internal-before-line-breaking grob dir)
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> define inside callback as helper function
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode823
>>  
>> scm/output-lib.scm:823: (extra-dy (assoc-get 'extra-dy details 0.0))
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> extra-dy isn't a nested property
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode826
>>  
>> scm/output-lib.scm:826: (y (+ (interval-center (ly:grob-extent bound
>> common-y Y))
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> this overwrites user override for 'Y
>> 
>> Fixed to allow for user Y offset.
>> 
>> http://codereview.appspot.com/4527086/diff/17001/scm/output-lib.scm#newcode829
>>  
>> scm/output-lib.scm:829: (cons (cons 'Y y) details)))
>> On 2011/06/26 17:01:36, Neil Puttock wrote:
>>> (acons 'Y y details)
>> 
>> Done.
>> 
>> http://codereview.appspot.com/4527086/
> 
> 
> 
> Would you (Mike and Neil) like me to re-start the countdown? Otherwise, Neil, 
> if you're happy with Mike's changes, this can be pushed, marked closed on 
> Reitveld and fixed on the tracker.
> 
> Thanks, Mike and Neil


Hey Colin,

If you have time to run the regtests on the new patch, I'd appreciate it (I'm 
slammed for time today & tomorrow).

Cheers,
MS