Re: [frogs] Re: tablature: ties and harmonics (issue1669041)
n.putt...@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 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [frogs] Re: tablature: ties and harmonics (issue1669041)
Carl Sorensen schrieb: On 6/30/10 1:48 AM, "Marc Hohl" 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 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [frogs] Re: tablature: ties and harmonics (issue1669041)
On 6/30/10 1:48 AM, "Marc Hohl" 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 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [frogs] Re: tablature: ties and harmonics (issue1669041)
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 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [frogs] Re: tablature: ties and harmonics (issue1669041)
On 6/27/10 3:19 AM, "Marc Hohl" wrote: > carl.d.soren...@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 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [frogs] Re: tablature: ties and harmonics (issue1669041)
carl.d.soren...@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 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: [frogs] Re: tablature: ties and harmonics (issue1669041)
n.putt...@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 ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
