tablature: ties and harmonics

tablature: ties and harmonics

Harmonic parentheses should mimic the visibility according to the
corresponding TabNoteHead in tie situations.

* the hard coded values for drawing the angled brackets were
  changed to properties (but they still cannot be reached within
  parenthesize-tab-note-head - is there a possibility to overcome this limitation?)

* the parenthesize-tab-note-head draws harmonic parentheses
  if necessary and surronds fret number AND harmonic parentheses by
  normal parentheses (at the moment, the fret number seems to disapper at random,
  see the regression test file)

[Neil Puttock, 2010-06-13 19:51:18]

http://codereview.appspot.com/1669041/diff/1/2
File input/regression/tablature-tie-harmonic.ly (right):

http://codereview.appspot.com/1669041/diff/1/2#newcode3
input/regression/tablature-tie-harmonic.ly:3: \header{ texidoc = "Harmonics in
tablature follow the rules for
\header {
  texidoc = "Harmonics in tablature follow the rules for
fret numbers in tie situations."
}

http://codereview.appspot.com/1669041/diff/1/3
File ly/engraver-init.ly (right):

http://codereview.appspot.com/1669041/diff/1/3#newcode748
ly/engraver-init.ly:748: %% the HarmonicParenthesesItem engraver has to listen
to the
HarmonicParenthesesItem grob

http://codereview.appspot.com/1669041/diff/1/3#newcode750
ly/engraver-init.ly:750: \override HarmonicParenthesesItem #'after-line-breaking
= #harmonic-parentheses-item::apply-tab-note-head-visibility
It'd be simpler to use 'transparent (see my comments concerning 'whiteout vs
'transparent below)

http://codereview.appspot.com/1669041/diff/1/4
File scm/define-grobs.scm (right):

http://codereview.appspot.com/1669041/diff/1/4#newcode871
scm/define-grobs.scm:871: (angularity . 1.5)
These (angularity, white-padding & width) need documenting in
define-grob-properties.scm and adding to an interface in
define-grob-interfaces.scm (either a new interface or parentheses-interface.)

http://codereview.appspot.com/1669041/diff/1/4#newcode872
scm/define-grobs.scm:872: (half-thickness . 0.05)
This should come from line-thickness.

http://codereview.appspot.com/1669041/diff/1/4#newcode878
scm/define-grobs.scm:878: (width . 0.5)
could be more specific

http://codereview.appspot.com/1669041/diff/1/5
File scm/output-lib.scm (right):

http://codereview.appspot.com/1669041/diff/1/5#newcode505
scm/output-lib.scm:505: (let* ((lp (draw-angled-bracket-stencil LEFT grob))
indent

http://codereview.appspot.com/1669041/diff/1/6
File scm/tablature.scm (right):

http://codereview.appspot.com/1669041/diff/1/6#newcode146
scm/tablature.scm:146: (let* ((articulations (ly:event-property (event-cause
grob) 'articulations))
let

http://codereview.appspot.com/1669041/diff/1/6#newcode150
scm/tablature.scm:150: (if (eq? 'harmonic-event (ly:event-property art 'class))
(ly:in-event-class? art 'harmonic-event)

http://codereview.appspot.com/1669041/diff/1/6#newcode157
scm/tablature.scm:157: (half-thickness 0.05) ;; urg, hard-coded values
Hmm, tricky.  It'll be difficult (rather convoluted) to get from TabNoteHead ->
HarmonicParenthesesItem.

(not half-thickness though, since this should come from line-thickness and
ly:output-def-lookup)

http://codereview.appspot.com/1669041/diff/1/6#newcode161
scm/tablature.scm:161: (dirp (ly:stencil-aligned-to
could do with a more descriptive name

http://codereview.appspot.com/1669041/diff/1/6#newcode291
scm/tablature.scm:291: (display "\ntransparent: ")(display
tab-note-head-transparent)
remove debug code

http://codereview.appspot.com/1669041/diff/1/6#newcode293
scm/tablature.scm:293: ;; we simply remove the stencil, so we don't have to care
about 'whiteout
I've thought about this for a while, and it's silly that 'whiteout and
'transparent aren't mutually exclusive: if something's transparent, its whiteout
shouldn't be visible.

(see grob.cc)

[marc, 2010-06-14 09:34:13]

n.puttock@gmail.com schrieb:
>
>
>
>
>
> http://codereview.appspot.com/1669041/diff/1/2
> File input/regression/tablature-tie-harmonic.ly (right):
>
> http://codereview.appspot.com/1669041/diff/1/2#newcode3
> input/regression/tablature-tie-harmonic.ly:3: \header{ texidoc =
> "Harmonics in tablature follow the rules for
> \header {
>   texidoc = "Harmonics in tablature follow the rules for
> fret numbers in tie situations."
> }
Done.
>
> http://codereview.appspot.com/1669041/diff/1/3
> File ly/engraver-init.ly (right):
>
> http://codereview.appspot.com/1669041/diff/1/3#newcode748
> ly/engraver-init.ly:748: %% the HarmonicParenthesesItem engraver has to
> listen to the
> HarmonicParenthesesItem grob
>
> http://codereview.appspot.com/1669041/diff/1/3#newcode750
> ly/engraver-init.ly:750: \override HarmonicParenthesesItem
> #'after-line-breaking =
> #harmonic-parentheses-item::apply-tab-note-head-visibility
> It'd be simpler to use 'transparent (see my comments concerning
> 'whiteout vs 'transparent below)
Ok, done. But at the moment, 'whiteout and 'transparent can be
used independently - if you compile the regression file, you see
white spots. What is the right way to solve this?
>
> http://codereview.appspot.com/1669041/diff/1/4
> File scm/define-grobs.scm (right):
>
> http://codereview.appspot.com/1669041/diff/1/4#newcode871
> scm/define-grobs.scm:871: (angularity . 1.5)
> These (angularity, white-padding & width) need documenting in
> define-grob-properties.scm and adding to an interface in
> define-grob-interfaces.scm (either a new interface or
> parentheses-interface.)
Ok, done.
>
> http://codereview.appspot.com/1669041/diff/1/4#newcode872
> scm/define-grobs.scm:872: (half-thickness . 0.05)
> This should come from line-thickness.
Done.
>
> http://codereview.appspot.com/1669041/diff/1/4#newcode878
> scm/define-grobs.scm:878: (width . 0.5)
> could be more specific
I replaced it by "bracket-width" globally.
>
> http://codereview.appspot.com/1669041/diff/1/5
> File scm/output-lib.scm (right):
>
> http://codereview.appspot.com/1669041/diff/1/5#newcode505
> scm/output-lib.scm:505: (let* ((lp (draw-angled-bracket-stencil LEFT
> grob))
> indent
Sorry, done. (I moved the code into the helper function and didn't check 
the indentation)
>
> http://codereview.appspot.com/1669041/diff/1/6
> File scm/tablature.scm (right):
>
> http://codereview.appspot.com/1669041/diff/1/6#newcode146
> scm/tablature.scm:146: (let* ((articulations (ly:event-property
> (event-cause grob) 'articulations))
> let
Yes, of course.
>
> http://codereview.appspot.com/1669041/diff/1/6#newcode150
> scm/tablature.scm:150: (if (eq? 'harmonic-event (ly:event-property art
> 'class))
> (ly:in-event-class? art 'harmonic-event)
Done. Is this function documented somewhere? I didn't find it.
>
> http://codereview.appspot.com/1669041/diff/1/6#newcode157
> scm/tablature.scm:157: (half-thickness 0.05) ;; urg, hard-coded values
> Hmm, tricky.  It'll be difficult (rather convoluted) to get from
> TabNoteHead -> HarmonicParenthesesItem.
I don't know whether it is worth the effort (probably no-one wil
touch the harmonic angle brackets), but in general it is not
very elegant to store the values in one place while not being able
to use them globally.
>
> (not half-thickness though, since this should come from line-thickness
> and ly:output-def-lookup)
Done.
>
> http://codereview.appspot.com/1669041/diff/1/6#newcode161
> scm/tablature.scm:161: (dirp (ly:stencil-aligned-to
> could do with a more descriptive name
Done.
>
> http://codereview.appspot.com/1669041/diff/1/6#newcode291
> scm/tablature.scm:291: (display "\ntransparent: ")(display
> tab-note-head-transparent)
> remove debug code
Done.
>
> http://codereview.appspot.com/1669041/diff/1/6#newcode293
> scm/tablature.scm:293: ;; we simply remove the stencil, so we don't have
> to care about 'whiteout
> I've thought about this for a while, and it's silly that 'whiteout and
> 'transparent aren't mutually exclusive: if something's transparent, its
> whiteout shouldn't be visible.
>
> (see grob.cc)
You are right - can this behavior be changed in grob.cc to be valid
for all grobs in lilypond? If yes, how?

Thank you

Marc

http://codereview.appspot.com/1669041/show

[marc, 2010-06-19 09:13:58]

Marc Hohl schrieb:
> n.puttock@gmail.com schrieb:
> [...]
>>
>> http://codereview.appspot.com/1669041/diff/1/6#newcode293
>> scm/tablature.scm:293: ;; we simply remove the stencil, so we don't have
>> to care about 'whiteout
>> I've thought about this for a while, and it's silly that 'whiteout and
>> 'transparent aren't mutually exclusive: if something's transparent, its
>> whiteout shouldn't be visible.
>>
>> (see grob.cc)
> You are right - can this behavior be changed in grob.cc to be valid
> for all grobs in lilypond? If yes, how?
Sorry, silly question. I have uploaded a separate patch
(http://codereview.appspot.com/1681043); if it is accepted, I will continue
on this issue and simplify parts of scm/tablature.scm.

Marc

>
> http://codereview.appspot.com/1669041/show
>

[marc, 2010-06-21 19:06:52]

Marc Hohl schrieb:
> Marc Hohl schrieb:
> [...]
> if it is accepted, I will continue
> on this issue and simplify parts of scm/tablature.scm.
>
Ok, done.

Now I *must* overlook the obvious, because the fret numbers still disappear,
even if they are not tied together.

Please have a look at this simple input file:

---

\version "2.13.25"

test = \relative c {
  < c\harmonic >2  < c\harmonic >
}

\context StaffGroup <<
  \context Staff {
     \clef "G_8"
     \test
  }
  \context TabStaff {
  \test
  }
 >>

---

The fret number will be invisible, and since no tie function in 
scm/tablature.scm
is called during this compilation, I think the error must be buried 
somewhere in
scm/output-lib.scm, but during the tests for the changes in this file, 
everything
worked correctly ...

Marc

http://codereview.appspot.com/1669041/show

[Neil Puttock, 2010-06-22 21:41:30]

On 2010/06/21 19:06:52, marc wrote:

> Now I *must* overlook the obvious, because the fret numbers still disappear,
> even if they are not tied together.

Remember what I said about 'layer?

Both TabNoteHead and HarmonicParenthesesItem have the same default layer (1),
but the latter is likely to be printed over the notehead since its engraver
depends on acknowledging a notehead first.

I think you'll have to set TabNoteHead's layer to 2 to ensure it's always
printed over the harmonic brackets.

Cheers,
Neil

[marc, 2010-06-23 06:33:34]

n.puttock@gmail.com schrieb:
> On 2010/06/21 19:06:52, marc wrote:
>
>> Now I *must* overlook the obvious, because the fret numbers still
> disappear,
>> even if they are not tied together.
>
> Remember what I said about 'layer?
Yes, of course, but since the HarmonicParenthesesItem *did* work before
without coping with layers and I only restructured the stencil building
process, I don't understand why this behavior has changed.

I just created a separate branch, loaded the untouched scm/output-lib.scm
in my editor, replaced the old code with the new (added the property values
by hand, because they are not reachable in this branch), and the tab number
appears. So these changes are obviously not the reason for the tab number
to dissappear.
>
> Both TabNoteHead and HarmonicParenthesesItem have the same default layer
> (1), but the latter is likely to be printed over the notehead since its
> engraver depends on acknowledging a notehead first.
I understand this, but why did it work before then?
>
> I think you'll have to set TabNoteHead's layer to 2 to ensure it's
> always printed over the harmonic brackets.
Ok, done.

Thanks,

Marc

http://codereview.appspot.com/1669041/show

[Carl, 2010-06-23 12:47:24]

Looks pretty good.  I have a few comments about things that seem to be
unnecessarily specific to harmonics.

Thanks,

Carl

http://codereview.appspot.com/1669041/diff/26001/27003
File scm/define-grob-interfaces.scm (right):

http://codereview.appspot.com/1669041/diff/26001/27003#newcode91
scm/define-grob-interfaces.scm:91: 'harmonic-parentheses-interface
Calling this "parenthesis-interface" would allow its use for other applications
of parentheses and would be a good idea, in my opinion.

http://codereview.appspot.com/1669041/diff/26001/27004
File scm/define-grob-properties.scm (right):

http://codereview.appspot.com/1669041/diff/26001/27004#newcode56
scm/define-grob-properties.scm:56: (angularity ,number? "Angularity of a
bracket.")
"angle bracket" instead of "bracket"?

http://codereview.appspot.com/1669041/diff/26001/27004#newcode130
scm/define-grob-properties.scm:130: (bracket-width ,number? "Width of the
harmonic angle bracket.")
Why do we need bracket-width?  Why can't we just use the pre-existing width
property?

http://codereview.appspot.com/1669041/diff/26001/27004#newcode850
scm/define-grob-properties.scm:850: (white-padding ,number? "Amount of padding
surrounding a harmonic
Why make this specific for harmonics?  Why not just "Amount of white space
boundary in a whiteout" or something similar?

http://codereview.appspot.com/1669041/diff/26001/27007
File scm/tablature.scm (right):

http://codereview.appspot.com/1669041/diff/26001/27007#newcode155
scm/tablature.scm:155: (define (draw-harmonic-stencil dir grob)
Why not use parenthesize-stencil (see scm/stencil.scm)?  This seems like
duplicated code, and we should probably avoid that if possible.

[marc, 2010-06-27 09:19:42]

Carl.D.Sorensen@gmail.com schrieb:
> Looks pretty good.  I have a few comments about things that seem to be
> unnecessarily specific to harmonics.
>
> Thanks,
>
> Carl
>
>
>
> http://codereview.appspot.com/1669041/diff/26001/27003
> File scm/define-grob-interfaces.scm (right):
>
> http://codereview.appspot.com/1669041/diff/26001/27003#newcode91
> scm/define-grob-interfaces.scm:91: 'harmonic-parentheses-interface
> Calling this "parenthesis-interface" would allow its use for other
> applications of parentheses and would be a good idea, in my opinion.
Ok, done.
>
> http://codereview.appspot.com/1669041/diff/26001/27004
> File scm/define-grob-properties.scm (right):
>
> http://codereview.appspot.com/1669041/diff/26001/27004#newcode56
> scm/define-grob-properties.scm:56: (angularity ,number? "Angularity of a
> bracket.")
> "angle bracket" instead of "bracket"?
Done.
>
> http://codereview.appspot.com/1669041/diff/26001/27004#newcode130
> scm/define-grob-properties.scm:130: (bracket-width ,number? "Width of
> the harmonic angle bracket.")
> Why do we need bracket-width?  Why can't we just use the pre-existing
> width property?
Hm, Neil proposed to use a more descriptive name for this property, IIUC.
Done.
>
> http://codereview.appspot.com/1669041/diff/26001/27004#newcode850
> scm/define-grob-properties.scm:850: (white-padding ,number? "Amount of
> padding surrounding a harmonic
> Why make this specific for harmonics?  Why not just "Amount of white
> space boundary in a whiteout" or something similar?
Done.
>
> http://codereview.appspot.com/1669041/diff/26001/27007
> File scm/tablature.scm (right):
>
> http://codereview.appspot.com/1669041/diff/26001/27007#newcode155
> scm/tablature.scm:155: (define (draw-harmonic-stencil dir grob)
> Why not use parenthesize-stencil (see scm/stencil.scm)?  This seems like
> duplicated code, and we should probably avoid that if possible.
>
I just copied the code from scm/output-lib.scm similar to the parenthesizing
routine within scm/tablature.scm. I thought I could call the newly 
introduced
helper routine itself, but this is not possible due to the limitation 
that I can't
(at least easily?) get the current property values of the 
HarmonicParenthesesItem
grob within the tie callback.
parenthesize-stencil (in scm,/stencil.scm) does not take the whiteout 
into account,
and at the moment, the whiteout handling is not done properly throughout
scm/tablature.scm. Sometimes it is hardcoded, sometimes it takes the
'whiteout property into account. I think the brackets and parentheses
should follow the TabNoteHead #'whiteout more consequently in this respect.


I'll shut down my computer for now - today he seems to hate me ;-)
I had to redo my patch sets and did a partial revert of a former patch,
no idea why this happened ...

http://codereview.appspot.com/1669041/show

[c_sorensen, 2010-06-27 13:42:05]

On 6/27/10 3:19 AM, "Marc Hohl" <marc@hohlart.de> wrote:

> Carl.D.Sorensen@gmail.com schrieb:
>> http://codereview.appspot.com/1669041/diff/26001/27003
>> File scm/define-grob-interfaces.scm (right):
>> 
>> http://codereview.appspot.com/1669041/diff/26001/27003#newcode91
>> scm/define-grob-interfaces.scm:91: 'harmonic-parentheses-interface
>> Calling this "parenthesis-interface" would allow its use for other
>> applications of parentheses and would be a good idea, in my opinion.
> Ok, done.
>> 
>> http://codereview.appspot.com/1669041/diff/26001/27004
>> File scm/define-grob-properties.scm (right):
>> 
>> http://codereview.appspot.com/1669041/diff/26001/27004#newcode56
>> scm/define-grob-properties.scm:56: (angularity ,number? "Angularity of a
>> bracket.")
>> "angle bracket" instead of "bracket"?
> Done.
>> 
>> http://codereview.appspot.com/1669041/diff/26001/27004#newcode130
>> scm/define-grob-properties.scm:130: (bracket-width ,number? "Width of
>> the harmonic angle bracket.")
>> Why do we need bracket-width?  Why can't we just use the pre-existing
>> width property?
> Hm, Neil proposed to use a more descriptive name for this property, IIUC.
> Done.

Well, if we want to have this be part of the parenthesis-interface, then we
may want to go ahead with bracket-width.

>> 
>> http://codereview.appspot.com/1669041/diff/26001/27007#newcode155
>> scm/tablature.scm:155: (define (draw-harmonic-stencil dir grob)
>> Why not use parenthesize-stencil (see scm/stencil.scm)?  This seems like
>> duplicated code, and we should probably avoid that if possible.
>> 
> I just copied the code from scm/output-lib.scm similar to the parenthesizing
> routine within scm/tablature.scm. I thought I could call the newly
> introduced
> helper routine itself, but this is not possible due to the limitation
> that I can't
> (at least easily?) get the current property values of the
> HarmonicParenthesesItem
> grob within the tie callback.
> parenthesize-stencil (in scm,/stencil.scm) does not take the whiteout
> into account,
> and at the moment, the whiteout handling is not done properly throughout
> scm/tablature.scm. Sometimes it is hardcoded, sometimes it takes the
> 'whiteout property into account. I think the brackets and parentheses
> should follow the TabNoteHead #'whiteout more consequently in this respect.

If instead of making the whiteout and the stencil different stencils, we
combined them, then parenthesize-stencil would work great.  But this might
not be possible if we need to have the stencil and the whiteout on separate
layers.  As an alternative, parenthesize-stencil could be modified to take
white-padding as an argument.  There's nothing that says we can't modify
scm/stencil.scm so that it works better for tablature.  It's much better,
IMO, to have one general-purpose piece of code than to have two separate
special-purpose code chunks.

Thanks,

Carl

[marc, 2010-06-30 07:48:12]

Carl Sorensen schrieb:
> [...]
>>> http://codereview.appspot.com/1669041/diff/26001/27004#newcode130
>>> scm/define-grob-properties.scm:130: (bracket-width ,number? "Width of
>>> the harmonic angle bracket.")
>>> Why do we need bracket-width?  Why can't we just use the pre-existing
>>> width property?
>>>       
>> Hm, Neil proposed to use a more descriptive name for this property, IIUC.
>> Done.
>>     
>
> Well, if we want to have this be part of the parenthesis-interface, then we
> may want to go ahead with bracket-width.
>   
Ok, done.
>   
> [...]
> If instead of making the whiteout and the stencil different stencils, we
> combined them, then parenthesize-stencil would work great.  But this might
> not be possible if we need to have the stencil and the whiteout on separate
> layers.  As an alternative, parenthesize-stencil could be modified to take
> white-padding as an argument.  There's nothing that says we can't modify
> scm/stencil.scm so that it works better for tablature.  It's much better,
> IMO, to have one general-purpose piece of code than to have two separate
> special-purpose code chunks.
>   
So I tried to extend the parenthesize-stencil function in scm/stencil.scm.
It compiles without error, but the regression file is cluttered up.

I feel that this is not the right approach, either. I still don't understand
why I have to shift the TabNoteHead #'layer at all. As far as I see it,
HarmonicParenthesesItem #'stencils is a list of stencils, containing the
left and the right angle bracket, which are placed either side of the
TabNoteHead #'stencil. So the order in which the stancils are placed
should not change anything at all, but if the TabNoteHead #'stencil is set
*before* the HarmonicParenthesesItem #'stencils, it seems that some
whitespace occurs between the angle brackets, overriding the
already placed TabNoteHead.
If on the other hand the whiteout would not appear, we could just use the
parenthesize-tab-note-head function defined in scm/tablature.scm.
This function could detect whether we have a harmonic and in this case
would place the parentheses just further apart.

Furthermore, I have a bad feeling to misuse your time reviewing my patch
for error seeking issues, but I am stuck here. I like the general idea of
not defining the same functionality over and over again, but on the 
other hand,
the parenthesize-stencil function in scm/stencil.scm does not seem to
be used anywhere else ...

Thanks in advance,

Marc

[c_sorensen, 2010-06-30 15:52:30]

On 6/30/10 1:48 AM, "Marc Hohl" <marc@hohlart.de> wrote:

> Carl Sorensen schrieb:
>> [...]
>>>> http://codereview.appspot.com/1669041/diff/26001/27004#newcode130
>>>> scm/define-grob-properties.scm:130: (bracket-width ,number? "Width of
>>>> the harmonic angle bracket.")
>>>> Why do we need bracket-width?  Why can't we just use the pre-existing
>>>> width property?
>>>>      
>>> Hm, Neil proposed to use a more descriptive name for this property, IIUC.
>>> Done.
>>>    
>> 
>> Well, if we want to have this be part of the parenthesis-interface, then we
>> may want to go ahead with bracket-width.
>>  
> Ok, done.
>>  
>> [...]
>> If instead of making the whiteout and the stencil different stencils, we
>> combined them, then parenthesize-stencil would work great.  But this might
>> not be possible if we need to have the stencil and the whiteout on separate
>> layers.  As an alternative, parenthesize-stencil could be modified to take
>> white-padding as an argument.  There's nothing that says we can't modify
>> scm/stencil.scm so that it works better for tablature.  It's much better,
>> IMO, to have one general-purpose piece of code than to have two separate
>> special-purpose code chunks.
>>  
> So I tried to extend the parenthesize-stencil function in scm/stencil.scm.
> It compiles without error, but the regression file is cluttered up.

Can you tell me what you mean by "the regression file is cluttered up"?  Can
you tell me which examples fail the regression test for graphical output?
(The changes that indicate different amounts of memory don't bother me a
bit).

> 
> I feel that this is not the right approach, either. I still don't understand
> why I have to shift the TabNoteHead #'layer at all. As far as I see it,
> HarmonicParenthesesItem #'stencils is a list of stencils, containing the
> left and the right angle bracket, which are placed either side of the
> TabNoteHead #'stencil. So the order in which the stancils are placed
> should not change anything at all, but if the TabNoteHead #'stencil is set
> *before* the HarmonicParenthesesItem #'stencils, it seems that some
> whitespace occurs between the angle brackets, overriding the
> already placed TabNoteHead.

Have you looked at the list of stencils created by
parenthesis-item::calc-parentehesis-stencils grob

and 

map stencil-whiteout (parenthesis-item::calc-parentehesis-stencils grob) to
see how they differ?

Sometimes lists get reversed during LilyPond processing.  Perhaps this is
happening to this list.  You might see what happens if you reverse the list
of stencils....



> If on the other hand the whiteout would not appear, we could just use the
> parenthesize-tab-note-head function defined in scm/tablature.scm.
> This function could detect whether we have a harmonic and in this case
> would place the parentheses just further apart.
> 
> Furthermore, I have a bad feeling to misuse your time reviewing my patch
> for error seeking issues, but I am stuck here.

I don't view it as a misuse of time.  But right now I'm very busy, so I may
not get to it very quickly....

> I like the general idea of
> not defining the same functionality over and over again, but on the
> other hand,
> the parenthesize-stencil function in scm/stencil.scm does not seem to
> be used anywhere else ...

Parenthesize-stencil will be used for the new chord namer, so we need to
keep it in stencil.scm and keep the current functionality.


Thanks,

Carl

[Neil Puttock, 2010-06-30 22:26:01]

On 2010/06/30 07:48:12, marc wrote:
 
> So I tried to extend the parenthesize-stencil function in scm/stencil.scm.
> It compiles without error, but the regression file is cluttered up.

I can't test this.  Would you mind uploading another version here which includes
all the changes since the first patch set?

> I feel that this is not the right approach, either. I still don't understand
> why I have to shift the TabNoteHead #'layer at all. As far as I see it,
> HarmonicParenthesesItem #'stencils is a list of stencils, containing the
> left and the right angle bracket, which are placed either side of the
> TabNoteHead #'stencil. So the order in which the stancils are placed
> should not change anything at all, but if the TabNoteHead #'stencil is set
> *before* the HarmonicParenthesesItem #'stencils, it seems that some
> whitespace occurs between the angle brackets, overriding the
> already placed TabNoteHead.

You added the default for 'whiteout to HarmonicParenthesesItem in your first
patch.  It applies the whiteout to the total extent: the two brackets plus the
space between.

I don't know why grobs with the same layer sometimes swap places, but there's
nothing we can do about it apart from change 'layer to ensure visibility, unless
you remove 'whiteout for the angle brackets.

Cheers,
Neil

[marc, 2010-07-01 06:07:44]

Carl Sorensen schrieb:
>
> On 6/30/10 1:48 AM, "Marc Hohl" <marc@hohlart.de> wrote:
>
>   
>> Carl Sorensen schrieb:
>>     
>>> [...]
>>>       
>>>>> http://codereview.appspot.com/1669041/diff/26001/27004#newcode130
>>>>> scm/define-grob-properties.scm:130: (bracket-width ,number? "Width of
>>>>> the harmonic angle bracket.")
>>>>> Why do we need bracket-width?  Why can't we just use the pre-existing
>>>>> width property?
>>>>>      
>>>>>           
>>>> Hm, Neil proposed to use a more descriptive name for this property, IIUC.
>>>> Done.
>>>>    
>>>>         
>>> Well, if we want to have this be part of the parenthesis-interface, then we
>>> may want to go ahead with bracket-width.
>>>  
>>>       
>> Ok, done.
>>     
>>>  
>>> [...]
>>> If instead of making the whiteout and the stencil different stencils, we
>>> combined them, then parenthesize-stencil would work great.  But this might
>>> not be possible if we need to have the stencil and the whiteout on separate
>>> layers.  As an alternative, parenthesize-stencil could be modified to take
>>> white-padding as an argument.  There's nothing that says we can't modify
>>> scm/stencil.scm so that it works better for tablature.  It's much better,
>>> IMO, to have one general-purpose piece of code than to have two separate
>>> special-purpose code chunks.
>>>  
>>>       
>> So I tried to extend the parenthesize-stencil function in scm/stencil.scm.
>> It compiles without error, but the regression file is cluttered up.
>>     
>
> Can you tell me what you mean by "the regression file is cluttered up"?  Can
> you tell me which examples fail the regression test for graphical output?
> (The changes that indicate different amounts of memory don't bother me a
> bit).
>   
Oh, sorry, I meant the newly introduced 
input/regression/tablature-tie-harmonic.ly.
With the patch set before, it worked as expected, now the angle brackets 
surround
invisible fret numbers again, and after line breaks, the harmonic 
brackets overlap
the parentheses construct (while erasing the fret number :-( )

>> I feel that this is not the right approach, either. I still don't understand
>> why I have to shift the TabNoteHead #'layer at all. As far as I see it,
>> HarmonicParenthesesItem #'stencils is a list of stencils, containing the
>> left and the right angle bracket, which are placed either side of the
>> TabNoteHead #'stencil. So the order in which the stancils are placed
>> should not change anything at all, but if the TabNoteHead #'stencil is set
>> *before* the HarmonicParenthesesItem #'stencils, it seems that some
>> whitespace occurs between the angle brackets, overriding the
>> already placed TabNoteHead.
>>     
>
> Have you looked at the list of stencils created by
> parenthesis-item::calc-parentehesis-stencils grob
>
> and 
>
> map stencil-whiteout (parenthesis-item::calc-parentehesis-stencils grob) to
> see how they differ?
>
> Sometimes lists get reversed during LilyPond processing.  Perhaps this is
> happening to this list.  You might see what happens if you reverse the list
> of stencils....
>   
I think I understand now - Neil has given me the clue to this.
> [...]
>> Furthermore, I have a bad feeling to misuse your time reviewing my patch
>> for error seeking issues, but I am stuck here.
>>     
>
> I don't view it as a misuse of time.  But right now I'm very busy, so I may
> not get to it very quickly....
>   
Thanks for your kind words - you do a great job in motivating frustrated
people like me ;-)
>   
>> I like the general idea of
>> not defining the same functionality over and over again, but on the
>> other hand,
>> the parenthesize-stencil function in scm/stencil.scm does not seem to
>> be used anywhere else ...
>>     
>
> Parenthesize-stencil will be used for the new chord namer, so we need to
> keep it in stencil.scm and keep the current functionality.
>   
Ok.

Thank you

Marc

[marc, 2010-07-01 06:13:05]

n.puttock@gmail.com schrieb:
> On 2010/06/30 07:48:12, marc wrote:
>
>> So I tried to extend the parenthesize-stencil function in
> scm/stencil.scm.
>> It compiles without error, but the regression file is cluttered up.
>
> I can't test this.  Would you mind uploading another version here which
> includes all the changes since the first patch set?
Of course! I hope I did it right - at least it looks like I have catched 
everything.
>
>> I feel that this is not the right approach, either. I still don't
> understand
>> why I have to shift the TabNoteHead #'layer at all. As far as I see
> it,
>> HarmonicParenthesesItem #'stencils is a list of stencils, containing
> the
>> left and the right angle bracket, which are placed either side of the
>> TabNoteHead #'stencil. So the order in which the stancils are placed
>> should not change anything at all, but if the TabNoteHead #'stencil is
> set
>> *before* the HarmonicParenthesesItem #'stencils, it seems that some
>> whitespace occurs between the angle brackets, overriding the
>> already placed TabNoteHead.
>
> You added the default for 'whiteout to HarmonicParenthesesItem in your
> first patch.  It applies the whiteout to the total extent: the two
> brackets plus the space between.
Ah, that makes everything clearer, thanks for this explanation!
>
> I don't know why grobs with the same layer sometimes swap places, but
> there's nothing we can do about it apart from change 'layer to ensure
> visibility, unless you remove 'whiteout for the angle brackets.
Now the whole #'layer stuff makes a lot more sense to me ...

Thank you

Marc