Re: Add independent control of thickness and offset for underline markup (issue1347041)

[Carl Sorensen]

On 5/29/10 10:10 AM, "Kieren MacMillan" <address@hidden>
wrote:

> Hi Neil,
> 
> Thanks for the comments!
> [Should I be somehow commenting on codereview instead of here?]

Either way works.

> 
>> I don't think we can remove the link between 'line-thickness and
>> underline offset, since it should scale based on staff-size.  At small
>> staff-sizes, 'line-thickness gets progressively larger, which matches
>> the thicker underline with a slightly bigger gap.
> 
> OK. But I want two things out of this patch:
>     1. Control of offset independent of thickness, even if offset is defined
> as a "multiple of 'line-thickness" (that I can adjust).
>     2. An easy way to globally set the property (i.e., not just a direct
> \override of the \underline command).
> 
>> What you could do instead is leave out the default value for offset and
>> keep the existing behaviour unless the user sets offset (if it's unset,
>> it will return #f).
> 
> I tried
>   #:properties ((thickness 1))
> and it failed.
> 
>> http://codereview.appspot.com/1347041/diff/1/2#newcode280
>> scm/define-markup-commands.scm:280: #:properties ((thickness 1) (offset
>> 0.5))
>> I think this default for offset is too large.
> 
> This is precisely why we need the offset property...  ;)
> 
>> http://codereview.appspot.com/1347041/diff/1/2#newcode301
>> scm/define-markup-commands.scm:301: (y (* -1 offset))
>> Simpler:
>> 
>> (- offset)

>> Couldn't we define the offset as a multiple of 'line-thickness, instead of
>> an absolute value?  That's how fret diagrams are scaled.
> 
> Sounds good to me!
> To be clear, 'line-thickness and the thickness property of \underline are
> [potentially] two different independent values, right?

Yes.  I would suggest that both thickness and offset should be multipliers
of 'line-thickness (which shows up in your scheme code as thick.

> Once these points are addressed, how do I submit a revision?
> 
Attach (or inline) a diff patch on an email, and I'll apply and repost to
Rietveld.

Thanks,

Carl