Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(473)

Issue 5438060: Adds padding between Hairpins and SpanBars. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 3 months ago by MikeSol
Modified:
12 years, 1 month ago
Reviewers:
pkx166h, mike, thomasmorley65, carl.d.sorensen, Keith
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Adds padding between Hairpins and SpanBars.

Patch Set 1 #

Total comments: 3

Patch Set 2 : Fixes unnecessary addition of padding. #

Patch Set 3 : Incorporates Carl's suggestions. #

Total comments: 1

Patch Set 4 : Stops all end-of-line hairpins at the same point. #

Patch Set 5 : Gets rid of additional bound-padding add ons. #

Patch Set 6 : Better control at all span bars. #

Total comments: 7

Patch Set 7 : Keeps bound-padding at all but LEFT broken bar lines without span bars #

Patch Set 8 : Cleaner implementation of handling concurrent hairpins. #

Total comments: 4

Patch Set 9 : Incorporate's Keith's suggestions. #

Patch Set 10 : Likely didn't change from last time - to test patchy. #

Total comments: 2

Patch Set 11 : Changes definition of has-span-bar. #

Patch Set 12 : Rebase against current master. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+275 lines, -10 lines) Patch
A input/regression/hairpin-span-bar.ly View 1 chunk +30 lines, -0 lines 0 comments Download
A lily/concurrent-hairpin-engraver.cc View 1 2 3 4 5 6 7 8 1 chunk +118 lines, -0 lines 0 comments Download
M lily/hairpin.cc View 1 2 3 4 5 6 7 5 chunks +81 lines, -4 lines 0 comments Download
M lily/include/hairpin.hh View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M lily/include/system.hh View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M lily/span-bar.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M lily/system.cc View 1 2 3 4 5 1 chunk +30 lines, -0 lines 0 comments Download
M ly/engraver-init.ly View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 6 7 8 9 10 4 chunks +7 lines, -2 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M scm/output-lib.scm View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 26
pkx166h
Passes make and make check James
12 years, 3 months ago (2011-11-27 20:51:09 UTC) #1
Carl
Looks pretty good. Thanks for hitting this so quickly. Just a couple of stylistic comments. ...
12 years, 3 months ago (2011-11-28 01:24:34 UTC) #2
mike_apollinemike.com
On Nov 28, 2011, at 2:24 AM, Carl.D.Sorensen@gmail.com wrote: > Looks pretty good. Thanks for ...
12 years, 3 months ago (2011-11-28 07:29:19 UTC) #3
Keith
Hairpins already get padding at the span bars, so rather than add a second implementation ...
12 years, 3 months ago (2011-11-28 09:05:07 UTC) #4
mike_apollinemike.com
On Nov 28, 2011, at 10:05 AM, k-ohara5a5a@oco.net wrote: > Hairpins already get padding at ...
12 years, 3 months ago (2011-11-28 09:11:47 UTC) #5
pkx166h
Passes Make and make check james
12 years, 3 months ago (2011-11-28 16:29:21 UTC) #6
Keith
On 2011/11/28 09:11:47, mike_apollinemike.com wrote: > If the hairpins stop before span bars but > ...
12 years, 3 months ago (2011-11-29 02:13:25 UTC) #7
mike_apollinemike.com
On Nov 29, 2011, at 3:13 AM, k-ohara5a5a@oco.net wrote: > On 2011/11/28 09:11:47, mike_apollinemike.com wrote: ...
12 years, 3 months ago (2011-11-29 06:58:20 UTC) #8
Keith
On Mon, 28 Nov 2011 22:58:07 -0800, mike@apollinemike.com <mike@apollinemike.com> wrote: > I've attached the regtest ...
12 years, 3 months ago (2011-11-29 07:25:10 UTC) #9
mike_apollinemike.com
On Nov 29, 2011, at 8:25 AM, Keith OHara wrote: > On Mon, 28 Nov ...
12 years, 3 months ago (2011-11-29 07:38:35 UTC) #10
Keith
On Mon, 28 Nov 2011 23:38:28 -0800, mike@apollinemike.com <mike@apollinemike.com> wrote: > Could you post some ...
12 years, 3 months ago (2011-11-29 08:03:23 UTC) #11
pkx166h
passes make and make check james
12 years, 3 months ago (2011-11-29 08:03:44 UTC) #12
pkx166h
passes make - reg tests attached http://code.google.com/p/lilypond/issues/detail?id=2057#c18 James
12 years, 3 months ago (2011-11-29 21:25:14 UTC) #13
MikeSol
This most recent patchset has keith's patch applied on top of it in order to ...
12 years, 3 months ago (2011-11-30 13:00:43 UTC) #14
Carl
Looks good to me, but I want to be sure that both Keith and Mike ...
12 years, 3 months ago (2011-11-30 21:30:16 UTC) #15
Keith
On Wed, 30 Nov 2011 13:30:16 -0800, <Carl.D.Sorensen@gmail.com> wrote: > Looks good to me, but ...
12 years, 3 months ago (2011-12-01 00:32:44 UTC) #16
pkx166h
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/out/share/lilypond/current/scm/lily-sort.scm] [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-functions.scm] [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-translation.scm] [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-music.scm] [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-type-predicates.scm] [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-identifiers.scm] [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-context-mods.scm] [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-backend.scm ...
12 years, 3 months ago (2011-12-01 17:53:44 UTC) #17
pkx166h
Passes make and make check James
12 years, 3 months ago (2011-12-02 09:07:02 UTC) #18
Keith
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 < arriving_hairpins_.size () ...
12 years, 3 months ago (2011-12-03 03:56:56 UTC) #19
MikeSol
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 < ...
12 years, 3 months ago (2011-12-05 06:53:18 UTC) #20
Keith
On 2011/12/05 06:53:18, MikeSol wrote: > The issue is that if I did this, an ...
12 years, 3 months ago (2011-12-05 07:58:08 UTC) #21
Keith
Mike, The old code was making a distinction between hairpins that end at the end ...
12 years, 3 months ago (2011-12-08 00:30:20 UTC) #22
mike_apollinemike.com
Le Dec 8, 2011 à 1:30 AM, k-ohara5a5a@oco.net a écrit : > Mike, > The ...
12 years, 3 months ago (2011-12-08 08:18:05 UTC) #23
Keith
On Thu, 08 Dec 2011 00:17:56 -0800, mike@apollinemike.com <mike@apollinemike.com> wrote: >> http://codereview.appspot.com/5438060/diff/2027/scm/define-grob-properties.scm#newcode1049 >> scm/define-grob-properties.scm:1049: be ...
12 years, 3 months ago (2011-12-08 08:38:13 UTC) #24
mike_apollinemike.com
Le Dec 8, 2011 à 9:37 AM, Keith OHara a écrit : > On Thu, ...
12 years, 3 months ago (2011-12-08 08:47:55 UTC) #25
thomasmorley65
12 years, 3 months ago (2011-12-08 23:51:22 UTC) #26
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 to create a work-around for this:
http://lists.gnu.org/archive/html/lilypond-user/2011-08/msg00583.html
)

If not, would it be hard to do so?

Cheers,
  Harm
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b