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

Issue 4867043: Uses Y-offset for stem tremolos instead of translated stencil. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 8 months ago by MikeSol
Modified:
8 years, 1 month ago
Reviewers:
pkx166h, joeneeman, mikesol
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Uses Y-offset for stem tremolos instead of translated stencil.

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+117 lines, -15 lines) Patch
M lily/beam.cc View 1 chunk +22 lines, -0 lines 1 comment Download
M lily/include/beam.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/stem.hh View 1 chunk +1 line, -0 lines 0 comments Download
M lily/include/stem-tremolo.hh View 1 chunk +4 lines, -1 line 0 comments Download
M lily/stem.cc View 2 chunks +7 lines, -1 line 0 comments Download
M lily/stem-tremolo.cc View 6 chunks +80 lines, -12 lines 0 comments Download
M scm/define-grobs.scm View 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 4
MikeSol
My summer of lily continues with this patch. Currently, the pure height function in stem ...
12 years, 8 months ago (2011-08-11 11:16:07 UTC) #1
joeneeman
lgtm http://codereview.appspot.com/4867043/diff/1/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/4867043/diff/1/lily/beam.cc#newcode1797 lily/beam.cc:1797: Beam::middle_stem_estimation (Grob *me, Direction dir) As far as ...
12 years, 8 months ago (2011-08-27 19:28:51 UTC) #2
pkx166h
Mike I get a failed patch apply here on the current tre (28 Aug) --snip-- ...
12 years, 8 months ago (2011-08-28 10:51:15 UTC) #3
mikesol_ufl.edu
12 years, 8 months ago (2011-08-29 14:40:15 UTC) #4
On Aug 28, 2011, at 12:51 PM, pkx166h@gmail.com wrote:

> Mike I get a failed patch apply here on the current tre (28 Aug)
> 
> --snip--
> 

You were applying an old patch that conflicted with current master.  I rebased
and pushed as 7623fef74bf21fc726a8c60b535e7794f9776700.

> Also we have a number of tracker issues for temolos and I am wondering
> if this covers one or more of these:
> 
> http://code.google.com/p/lilypond/issues/detail?id=1444
> 

These are chord tremolos (not stem tremolos), so it is a different issue.

> http://code.google.com/p/lilypond/issues/detail?id=376

I don't think this is a bug...it seems like a statement of personal preference,
and should be corroborated by the literature.

> 
> http://code.google.com/p/lilypond/issues/detail?id=704
> 

idem - chord tremolos.

> http://code.google.com/p/lilypond/issues/detail?id=1087

idem - chord tremolos.

> 
> http://code.google.com/p/lilypond/issues/detail?id=734
> 

My patch doesn't fix this.  However, it facilitates the fixing of this.  If
someone wants to take a crack at it, go to Stem_tremolo::y_offset and use:

final_y_offset = minmax (direction_of_stem, final_y_offset - direction_of_stem *
me->extent(me, Y_AXIS)[direction_of_stem],
height_of_staff_symbol[direction_of_stem]);

A similar thing would need to be done for the pure heights.

The above is pseudo-code, but hopefully you get the idea.

Cheers,
MS

Sign in to reply to this message.

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