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

Issue 4636081: modifying default behaviour of tremolo slashes

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 11 months ago by Janek Warchol
Modified:
4 years, 2 months ago
Reviewers:
mike, janek, carl.d.sorensen, Neil Puttock, reinhold, MikeSol, xratamacue, joeneeman, Valentin Villenave, pkx166h, Graham Percival (old account)
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

modifying default behaviour of tremolo slashes It turned out that tremolo slashes should have quite constant slope (definately not depending on beam slope), so i change that. I also changed slash style so that it isn't rectangle in any cases by dafault.

Patch Set 1 #

Total comments: 1

Patch Set 2 : introduce shape property and use style to choose between behaviours #

Patch Set 3 : document properties #

Total comments: 6

Patch Set 4 : current Lily behavoiur is default again #

Patch Set 5 : removing redundant lines #

Patch Set 6 : renaming default to parallelogram #

Patch Set 7 : replacing parallelogram with beam-like #

Patch Set 8 : adjusting to fixcc #

Patch Set 9 : small fix #

Total comments: 2

Patch Set 10 : add regtest #

Total comments: 6

Patch Set 11 : style fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+78 lines, -12 lines) Patch
A input/regression/stem-tremolo-style.ly View 1 2 3 4 5 6 7 8 9 1 chunk +55 lines, -0 lines 0 comments Download
M lily/include/stem-tremolo.hh View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M lily/stem-tremolo.cc View 1 2 3 4 5 6 7 8 9 10 6 chunks +18 lines, -10 lines 0 comments Download
M scm/define-grob-properties.scm View 1 2 3 4 5 6 7 8 9 1 chunk +3 lines, -0 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 32
Janek Warchol
Passes regtests.
7 years, 11 months ago (2011-07-04 19:26:04 UTC) #1
Carl
Looks mostly good to me. Thanks, Carl http://codereview.appspot.com/4636081/diff/1/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/1/lily/stem-tremolo.cc#newcode42 lily/stem-tremolo.cc:42: that it's ...
7 years, 11 months ago (2011-07-04 19:50:09 UTC) #2
Neil Puttock
Hi Janek, I'm afraid I don't like this at all. While the authorities may be ...
7 years, 11 months ago (2011-07-04 20:11:48 UTC) #3
joeneeman
Actually, non-rectangular, constant-sloped beams used to be the default, but I changed them (some years ...
7 years, 11 months ago (2011-07-11 04:09:45 UTC) #4
Janek Warchol
Finally - new patch set uploaded. Now it's possible to easily switch between different tremolo ...
7 years, 11 months ago (2011-07-17 07:25:43 UTC) #5
pkx166h
Hello, can someone else verify? I get a make fail: --snip-- [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-markup.scm] Writing "internals.texi"...ERROR: In ...
7 years, 11 months ago (2011-07-18 20:13:19 UTC) #6
Janek Warchol
2011/7/18 <pkx166h@gmail.com>: > Hello, can someone else verify? > > I get a make fail: ...
7 years, 11 months ago (2011-07-18 21:43:09 UTC) #7
Neil Puttock
2011/7/18 Janek Warchoł <lemniskata.bernoullego@gmail.com>: > Btw, > >> [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-markup.scm] >> Writing "internals.texi"...ERROR: In procedure procedure-name: ...
7 years, 11 months ago (2011-07-18 21:51:43 UTC) #8
Janek Warchol
2011/7/18 Neil Puttock <n.puttock@gmail.com>: > 2011/7/18 Janek Warchoł <lemniskata.bernoullego@gmail.com>: > >> Btw, >> >>> [/home/jlowe/lilypond-git/build/out/share/lilypond/current/scm/document-markup.scm] ...
7 years, 11 months ago (2011-07-18 22:16:15 UTC) #9
pkx166h
Make works now and I get some reg test output differences See http://code.google.com/p/lilypond/issues/detail?id=1735#c4 James
7 years, 11 months ago (2011-07-19 21:39:28 UTC) #10
Janek Warchol
2011/7/19 <pkx166h@gmail.com>: > Make works now and I get some reg test output differences > ...
7 years, 11 months ago (2011-07-19 21:44:00 UTC) #11
joeneeman
http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc#newcode42 lily/stem-tremolo.cc:42: style = ly_symbol2scm ("constant"); You can remove these two ...
7 years, 11 months ago (2011-07-21 08:39:53 UTC) #12
Janek Warchol
New patch set uploaded. Current Lily default behaviour is kept as default now. http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc File ...
7 years, 11 months ago (2011-07-23 19:58:19 UTC) #13
Janek Warchol
http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc#newcode93 lily/stem-tremolo.cc:93: style = ly_symbol2scm ("constant"); On 2011/07/23 19:58:19, Janek Warchol ...
7 years, 11 months ago (2011-07-23 21:15:12 UTC) #14
joeneeman
http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/10001/lily/stem-tremolo.cc#newcode93 lily/stem-tremolo.cc:93: style = ly_symbol2scm ("constant"); On 2011/07/23 19:58:19, Janek Warchol ...
7 years, 11 months ago (2011-07-24 07:11:41 UTC) #15
pkx166h
makes fine. No reg test differences now.
7 years, 11 months ago (2011-07-24 10:49:34 UTC) #16
Janek Warchol
Joe: redundant lines removed. Interesting thing happens when i compare regtests: i see a difference ...
7 years, 11 months ago (2011-07-25 00:08:17 UTC) #17
Janek Warchol
New patch set uploaded. Minor style change: slash shape that was named "default" (which was ...
7 years, 11 months ago (2011-07-29 05:38:26 UTC) #18
reinhold_kainhofer.com
Am Freitag 29 Juli 2011, 07:38:26 schrieben Sie: > New patch set uploaded. Minor style ...
7 years, 11 months ago (2011-07-29 09:11:28 UTC) #19
Janek Warchol
Reinhold: ok, replaced parallelogram with beam-like
7 years, 11 months ago (2011-07-29 09:24:19 UTC) #20
pkx166h
make passes and no reg tests differences
7 years, 11 months ago (2011-07-30 22:43:01 UTC) #21
Graham Percival (old account)
cannot apply to master due to fixcc.
7 years, 10 months ago (2011-08-03 19:46:10 UTC) #22
Janek Warchol
Oh no! I hoped to get it on this countdown. A version compatible with fixcc ...
7 years, 10 months ago (2011-08-03 22:08:24 UTC) #23
Neil Puttock
Needs a regression test exercising the different settings. http://codereview.appspot.com/4636081/diff/38001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/38001/lily/stem-tremolo.cc#newcode118 lily/stem-tremolo.cc:118: shape ...
7 years, 10 months ago (2011-08-11 20:05:10 UTC) #24
Janek Warchol
Regtest added. http://codereview.appspot.com/4636081/diff/38001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/38001/lily/stem-tremolo.cc#newcode118 lily/stem-tremolo.cc:118: shape = ly_symbol2scm ("beam-like"); On 2011/08/11 20:05:10, ...
7 years, 10 months ago (2011-08-22 10:11:55 UTC) #25
MikeSol
LGTM Cheers, MS http://codereview.appspot.com/4636081/diff/42001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/42001/lily/stem-tremolo.cc#newcode61 lily/stem-tremolo.cc:61: return scm_from_double ((Stem::duration_log (stem) >= 3 ...
7 years, 10 months ago (2011-08-23 07:48:39 UTC) #26
janek
New patch set uploaded. http://codereview.appspot.com/4636081/diff/42001/lily/stem-tremolo.cc File lily/stem-tremolo.cc (right): http://codereview.appspot.com/4636081/diff/42001/lily/stem-tremolo.cc#newcode61 lily/stem-tremolo.cc:61: return scm_from_double ((Stem::duration_log (stem) >= ...
7 years, 10 months ago (2011-08-27 09:37:05 UTC) #27
pkx166h
Still passes make and reg tests
7 years, 10 months ago (2011-08-28 11:00:39 UTC) #28
janek
Hey Mike, 2011/8/27 <janek.lilypond@gmail.com>: > http://codereview.appspot.com/4636081/diff/42001/scm/define-grobs.scm#newcode1944 > scm/define-grobs.scm:1944: (X-extent . ,ly:stem-tremolo::width) > On 2011/08/23 07:48:39, ...
7 years, 9 months ago (2011-09-05 20:31:48 UTC) #29
mike_apollinemike.com
On Sep 5, 2011, at 10:31 PM, Janek Warchoł wrote: > Hey Mike, > > ...
7 years, 9 months ago (2011-09-16 16:03:40 UTC) #30
janek
2011/9/16 mike@apollinemike.com <mike@apollinemike.com>: > On Sep 5, 2011, at 10:31 PM, Janek Warchoł wrote: > ...
7 years, 9 months ago (2011-09-17 19:29:40 UTC) #31
Valentin Villenave
4 years, 2 months ago (2015-04-12 20:25:32 UTC) #32
On 2011/09/17 19:29:40, janek wrote:
> It is ready to go except for the above.

Greetings,
I realize that this patch has been sort of forgotten, so I thought I’d update it
and re-upload it:
https://codereview.appspot.com/227070043/
Sign in to reply to this message.

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