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

Issue 4639065: Improves some parmesan noteheads. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years ago by Bertrand Bordage
Modified:
3 years, 3 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Improves some parmesan noteheads.

Patch Set 1 #

Patch Set 2 : Petrucci noteheads shorter than brevis are finished. #

Patch Set 3 : New directed longas/maximas; design enhanced. #

Patch Set 4 : Code cleanup and some minor fixes. #

Patch Set 5 : Separate parmesan-noteheads, fixes horizontal stem attachment, etc. #

Patch Set 6 : Typo. #

Total comments: 6

Patch Set 7 : Fix mensural ligatures. #

Patch Set 8 : Fix mensural ligature's thickness; decrease whole hole, increase longa stem thickness... #

Total comments: 2

Patch Set 9 : Totally remove the left�stemmed longa. #

Total comments: 14
Unified diffs Side-by-side diffs Delta from patch set Stats (+628 lines, -281 lines) Patch
M input/regression/mensural-ligatures.ly View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M input/regression/note-head-style.ly View 1 2 3 4 2 chunks +5 lines, -5 lines 0 comments Download
M lily/mensural-ligature.cc View 1 2 3 4 5 6 7 8 4 chunks +41 lines, -23 lines 0 comments Download
M lily/mensural-ligature-engraver.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M lily/note-head.cc View 1 2 3 4 1 chunk +13 lines, -8 lines 7 comments Download
M lily/stem.cc View 1 2 3 4 1 chunk +5 lines, -1 line 7 comments Download
M ly/engraver-init.ly View 1 2 3 4 5 6 7 3 chunks +57 lines, -0 lines 0 comments Download
M mf/GNUmakefile View 1 2 3 4 4 chunks +13 lines, -2 lines 0 comments Download
M mf/parmesan-generic.mf View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download
M mf/parmesan-noteheads.mf View 1 2 3 4 5 6 7 8 9 chunks +345 lines, -238 lines 0 comments Download
A mf/parmesan-noteheads-generic.mf View 1 2 3 4 1 chunk +39 lines, -0 lines 0 comments Download
A mf/parmesan-noteheads11.mf View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
A mf/parmesan-noteheads13.mf View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
A mf/parmesan-noteheads14.mf View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
A mf/parmesan-noteheads16.mf View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
A mf/parmesan-noteheads18.mf View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
A mf/parmesan-noteheads20.mf View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
A mf/parmesan-noteheads23.mf View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
A mf/parmesan-noteheads26.mf View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M scripts/build/gen-emmentaler-scripts.py View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M scripts/build/mf-to-table.py View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 28
Bertrand Bordage
Hi everyone, This patch is finally ready for review! What has been done for Petrucci/mensural/neomensural: ...
7 years, 9 months ago (2011-09-12 01:44:33 UTC) #1
lemzwerg
LGTM Werner
7 years, 9 months ago (2011-09-12 13:32:17 UTC) #2
benko.pal
there are problems: the patch as is fails the mensural-ligatures regtest. see below. p http://codereview.appspot.com/4639065/diff/13002/lily/mensural-ligature.cc ...
7 years, 9 months ago (2011-09-12 19:16:26 UTC) #3
Bertrand Bordage
Thanks for your reviews! On 2011/09/12 19:16:26, benko.pal wrote: > http://codereview.appspot.com/4639065/diff/13002/lily/mensural-ligature.cc#newcode147 > lily/mensural-ligature.cc:147: Direction stem_dir ...
7 years, 9 months ago (2011-09-12 22:24:24 UTC) #4
janek
Hi, i've looked at latest screenshot attached to tracker issue and... wow! It looks really ...
7 years, 9 months ago (2011-09-12 22:37:05 UTC) #5
benko.pal
hi Bertrand, > I am a total ligature newbie. But I see some stemmed notes ...
7 years, 9 months ago (2011-09-13 07:38:46 UTC) #6
Bertrand Bordage
On 2011/09/12 22:37:05, janek wrote: > i've looked at latest screenshot attached to tracker issue ...
7 years, 9 months ago (2011-09-13 13:50:04 UTC) #7
pkx166h
Thought I'd commented on this one but see http://code.google.com/p/lilypond/issues/detail?id=1839#c13 There are the reg test differences.
7 years, 9 months ago (2011-09-13 14:56:14 UTC) #8
janek
2011/9/13 <bordage.bertrand@gmail.com>: > http://codereview.appspot.com/4639065/diff/13002/ly/engraver-init.ly#newcode1063 >> >> ly/engraver-init.ly:1063: \override Stem #'thickness = #2 >> I'd make ...
7 years, 9 months ago (2011-09-13 16:25:07 UTC) #9
benko.pal
hi all, the patch is ok from my point of view. a minor question: the ...
7 years, 9 months ago (2011-09-14 05:56:42 UTC) #10
pacovila
2011/9/14 Benkő Pál <benko.pal@gmail.com>: > hi all, > > the patch is ok from my ...
7 years, 9 months ago (2011-09-14 17:28:10 UTC) #11
benko.pal
2011/9/14 Francisco Vila <paconet.org@gmail.com>: >> have you seen the facsimiles in the message >> http://lists.gnu.org/archive/html/lilypond-devel/2010-12/msg00398.html ...
7 years, 9 months ago (2011-09-14 19:02:48 UTC) #12
benko.pal
hi Bertrand, > What has been done for Petrucci/mensural/neomensural: > * Stems centered around the ...
7 years, 9 months ago (2011-09-14 21:05:28 UTC) #13
Bertrand Bordage
New patch set (inspired by Janek's ideas). On 2011/09/14 21:05:28, benko.pal wrote: > > * ...
7 years, 9 months ago (2011-09-15 17:08:13 UTC) #14
benko.pal
>> > * Use the left-stemmed longa in ligatures. > >> exactly what is this ...
7 years, 9 months ago (2011-09-15 18:46:42 UTC) #15
benko.pal
hi Bertrand, sorry, I messed up my build last time, and the mensural-ligatures.ly regtest is ...
7 years, 9 months ago (2011-09-15 19:30:48 UTC) #16
Bertrand Bordage
Ok Pal, I removed the left-stemmed longa. I don't know where you could put these ...
7 years, 9 months ago (2011-09-15 19:55:09 UTC) #17
benko.pal
hi Bertrand, > Ok Pal, I removed the left-stemmed longa. thanks, mensural-ligatures.ly is now ok. ...
7 years, 9 months ago (2011-09-15 21:09:08 UTC) #18
pkx166h
Passes make and new reg test differences (look ok) attached here http://code.google.com/p/lilypond/issues/detail?id=1839#c17
7 years, 9 months ago (2011-09-15 21:12:29 UTC) #19
benko.pal
> Passes make and new reg test differences (look ok) attached here in all four ...
7 years, 9 months ago (2011-09-16 08:20:02 UTC) #20
Bertrand Bordage
Pushed as: 0dcc93c0a5a97d048db2f7631966f41ae0059ab5 and 0e31cd90e44673eca7ac59705ce4bed33dd8e80e Thank you all for this review! Bertrand
7 years, 9 months ago (2011-09-17 16:40:11 UTC) #21
Keith
Bertrand, It looks like the additional font lookups are significantly slowing down the Windows build, ...
7 years, 8 months ago (2011-10-02 23:00:49 UTC) #22
Keith
http://codereview.appspot.com/4639065/diff/42001/lily/note-head.cc File lily/note-head.cc (right): http://codereview.appspot.com/4639065/diff/42001/lily/note-head.cc#newcode42 lily/note-head.cc:42: /* Cache the previously-used stencils */ static Stencil recent_symmetric_heads[4/*2 ...
7 years, 8 months ago (2011-10-03 01:15:48 UTC) #23
Keith
http://codereview.appspot.com/4639065/diff/42001/lily/note-head.cc File lily/note-head.cc (right): http://codereview.appspot.com/4639065/diff/42001/lily/note-head.cc#newcode49 lily/note-head.cc:49: } Yuck. The decision tree is complex enough that ...
7 years, 8 months ago (2011-10-03 04:57:59 UTC) #24
Bertrand Bordage
Thanks Keith, I'll quickly fix that in a new issue. http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc#newcode853 ...
7 years, 8 months ago (2011-10-03 08:52:27 UTC) #25
Keith
http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc#newcode855 lily/stem.cc:855: if (attach && !scm_is_eq (style, ly_symbol2scm ("mensural")) On 2011/10/03 ...
7 years, 8 months ago (2011-10-03 09:30:31 UTC) #26
Bertrand Bordage
http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc File lily/stem.cc (right): http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc#newcode855 lily/stem.cc:855: if (attach && !scm_is_eq (style, ly_symbol2scm ("mensural")) I meant: ...
7 years, 8 months ago (2011-10-07 09:47:23 UTC) #27
Keith
7 years, 8 months ago (2011-10-07 17:56:08 UTC) #28
http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc
File lily/stem.cc (right):

http://codereview.appspot.com/4639065/diff/42001/lily/stem.cc#newcode855
lily/stem.cc:855: if (attach && !scm_is_eq (style, ly_symbol2scm ("mensural"))
On 2011/10/07 09:47:24, Bertrand Bordage wrote:
> stems have to be centered around the attachment point, which is positioned at
> the center of the notehead.

Okay; I thought the centered case was excluded by if(attach)
Sign in to reply to this message.

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