Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Jan 27, 2012, at 8:10 PM, Keith OHara wrote: > On 2011/11/28 09:11:47, mike_apollinemike.com wrote: >> If the hairpins stop before span bars but >> extend all the way when span bar's don't exist >> (including when they are not >> present because of the RemoveEmptyStaffContext), >> then I'd much

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On 2011/11/28 09:11:47, mike_apollinemike.com wrote: If the hairpins stop before span bars but extend all the way when span bar's don't exist (including when they are not present because of the RemoveEmptyStaffContext), then I'd much rather go with your patch, as it is much less invasive than mi

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Sun, 18 Dec 2011 23:31:23 -0800, m...@apollinemike.com wrote: And while you're at it, in the same patch, could you try removing line 170 from hairpin.cc (the if condition for direction equaling left) That would pull the hairpin back from the entire non-musical column at the end of the

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Dec 19, 2011, at 7:37 AM, Keith OHara wrote: > Le 8 Dec 2011 à 00:17:56 -0800, m...@apollinemike.com a écrit : >> Le Dec 8, 2011 à 1:30 AM, k-ohara5...@oco.net a écrit : >> >>> The old code was making a distinction between hairpins that end at the >>> end of a line and those that continue on

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Le 8 Dec 2011 à 00:17:56 -0800, m...@apollinemike.com a écrit : Le Dec 8, 2011 à 1:30 AM, k-ohara5...@oco.net a écrit : The old code was making a distinction between hairpins that end at the end of a line and those that continue on the next line. Does your code preserve this distinction? Y

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Le Dec 9, 2011 à 4:37 AM, Keith OHara a écrit : > On Thu, 08 Dec 2011 15:51:22 -0800, Thomas Morley > wrote: > >> Will this patch fix the problem drawing the hairpin under a new >> KeySignature at the line-end as shown with this example? >> >> { c'1\< \key bes\major \break >> d'2 c'\! } > >

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Thu, 08 Dec 2011 15:51:22 -0800, Thomas Morley wrote: Will this patch fix the problem drawing the hairpin under a new KeySignature at the line-end as shown with this example? { c'1\< \key bes\major \break d'2 c'\! } I would have thought we wanted to have the hairpin continue under the

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Hi Mike, please excuse me disturbing the discussion. I do not understand it, so I simply ask: Will these patch fix the problem drawing the hairpin under a new KeySignature at the line-end as shown with this example? { c'1\< \key bes\major \break d'2 c'\! } (Perhaps you remember helping me t

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Le Dec 8, 2011 à 9:37 AM, Keith OHara a écrit : > On Thu, 08 Dec 2011 00:17:56 -0800, m...@apollinemike.com > wrote: > >>> http://codereview.appspot.com/5438060/diff/2027/scm/define-grob-properties.scm#newcode1049 >>> scm/define-grob-properties.scm:1049: be drawn above and below the staff. >>>

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Thu, 08 Dec 2011 00:17:56 -0800, m...@apollinemike.com wrote: http://codereview.appspot.com/5438060/diff/2027/scm/define-grob-properties.scm#newcode1049 scm/define-grob-properties.scm:1049: be drawn above and below the staff. If no span bar is in a position, be drawn below and above the sta

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Le Dec 8, 2011 à 1:30 AM, k-ohara5...@oco.net a écrit : > Mike, > The old code was making a distinction between hairpins that end at the > end of a line and those that continue on the next line. > Sadly, the meaning of the boolean 'broken[]' stores at first whether > the bound on either end of a h

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Mike, The old code was making a distinction between hairpins that end at the end of a line and those that continue on the next line. Sadly, the meaning of the boolean 'broken[]' stores at first whether the bound on either end of a hair pin is at a line break, and then broken[RIGHT] is changed so

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On 2011/12/05 06:53:18, MikeSol wrote: The issue is that if I did this, an arriving hairpin would be in its own "concurrent-hairpins" grob array. I thought that was the idea, because that is the array of all hairpins that are checked for span-bar closeness. Somehow the current hairpin has to

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

http://codereview.appspot.com/5438060/diff/14024/lily/concurrent-hairpin-engraver.cc File lily/concurrent-hairpin-engraver.cc (right): http://codereview.appspot.com/5438060/diff/14024/lily/concurrent-hairpin-engraver.cc#newcode79 lily/concurrent-hairpin-engraver.cc:79: for (vsize j = i + 1; j <

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

http://codereview.appspot.com/5438060/diff/14024/lily/concurrent-hairpin-engraver.cc File lily/concurrent-hairpin-engraver.cc (right): http://codereview.appspot.com/5438060/diff/14024/lily/concurrent-hairpin-engraver.cc#newcode78 lily/concurrent-hairpin-engraver.cc:78: for (vsize i = 0; i < arri

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Passes make and make check James http://codereview.appspot.com/5438060/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Fails make --snip-- [/home/jlowe/lilypond-git/ly/context-mods-init.ly] [/home/jlowe/lilypond-git/ly/engraver-init.ly]] [/home/jlowe/lilypond-git/ly/generate-documentation.ly [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/documentation-lib.scm] [/home/jlowe/lilypond-git/build/

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Wed, 30 Nov 2011 13:30:16 -0800, wrote: Looks good to me, but I want to be sure that both Keith and Mike agree that this is what ought to be applied, so we don't have two non-compatible solutions fighting with each other. No problem there; Mike incorporated my 2-line patch into this. I wan

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Looks good to me, but I want to be sure that both Keith and Mike agree that this is what ought to be applied, so we don't have two non-compatible solutions fighting with each other. http://codereview.appspot.com/5438060/diff/4006/lily/include/system.hh File lily/include/system.hh (right): http:

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Reviewers: J_lowe, carl.d.sorensen_gmail.com, mike_apollinemike.com, Keith, Message: This most recent patchset has keith's patch applied on top of it in order to work. Cheers, MS Description: Adds padding between Hairpins and SpanBars. Please review this at http://codereview.appspot.com/543806

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

passes make - reg tests attached http://code.google.com/p/lilypond/issues/detail?id=2057#c18 James http://codereview.appspot.com/5438060/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Nov 29, 2011, at 10:22 AM, Keith OHara wrote: > On Mon, 28 Nov 2011 23:38:28 -0800, m...@apollinemike.com > wrote: > >> In the original report, my eyes zoomed directly to the second system, last >> measure (which my patch fixes). > > Just to be clear, the one-line fix linked to issue 2060

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Mon, 28 Nov 2011 23:38:28 -0800, m...@apollinemike.com wrote: In the original report, my eyes zoomed directly to the second system, last measure (which my patch fixes). Just to be clear, the one-line fix linked to issue 2060 fixes that original report as well. I see what you mean tha

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

passes make and make check james http://codereview.appspot.com/5438060/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Mon, 28 Nov 2011 23:38:28 -0800, m...@apollinemike.com wrote: Could you post some PDFs of the patch in action? There's an example at Just reading the code, with respect to: if (bound->is_non_musical (bound) || bound->break_statu

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Nov 29, 2011, at 8:25 AM, Keith OHara wrote: > On Mon, 28 Nov 2011 22:58:07 -0800, m...@apollinemike.com > wrote: > >> I've attached the regtest I added compiled w/ Keith's patch & with my patch. > > Your test case has one very long hairpin in each staff, spanning three > systems. > Do we

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Mon, 28 Nov 2011 22:58:07 -0800, m...@apollinemike.com wrote: I've attached the regtest I added compiled w/ Keith's patch & with my patch. Your test case has one very long hairpin in each staff, spanning three systems. Do we want to insert a gap in the broken hairpin to make room for the

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On 2011/11/28 09:11:47, mike_apollinemike.com wrote: If the hairpins stop before span bars but extend all the way when span bar's don't exist (including when they are not present because of the RemoveEmptyStaffContext), then I'd much rather go with your patch, as it is much less invasive than min

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Passes Make and make check james http://codereview.appspot.com/5438060/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Nov 28, 2011, at 10:05 AM, k-ohara5...@oco.net wrote: > Hairpins already get padding at the span bars, so rather than add a > second implementation can't we just repair the existing implementation ? > > Maybe will do. (I'm just > starting a make check.

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Hairpins already get padding at the span bars, so rather than add a second implementation can't we just repair the existing implementation ? Maybe will do. (I'm just starting a make check.) http://codereview.appspot.com/5438060/diff/7010/lily/system.cc

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

On Nov 28, 2011, at 2:24 AM, carl.d.soren...@gmail.com wrote: > Looks pretty good. Thanks for hitting this so quickly. Just a couple > of stylistic comments. > > Thanks, > > Carl > > > > http://codereview.appspot.com/5438060/diff/1/lily/include/system.hh > File lily/include/system.hh (righ

Re: Adds padding between Hairpins and SpanBars. (issue 5438060)

Looks pretty good. Thanks for hitting this so quickly. Just a couple of stylistic comments. Thanks, Carl http://codereview.appspot.com/5438060/diff/1/lily/include/system.hh File lily/include/system.hh (right): http://codereview.appspot.com/5438060/diff/1/lily/include/system.hh#newcode45 li

Adds padding between Hairpins and SpanBars. (issue 5438060)

Passes make and make check James http://codereview.appspot.com/5438060/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel