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

Issue 4860042: Does better polynomial calculations for avoid objects. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 10 months ago by MikeSol
Modified:
3 years, 3 months ago
Reviewers:
pkx166h, joeneeman, mikesol, hanwenn
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Does better polynomial calculations for avoid objects.

Patch Set 1 #

Total comments: 10

Patch Set 2 : Incorporates Joe's comments. #

Total comments: 3

Patch Set 3 : Incorporates comments from Han-Wen and Joe #

Patch Set 4 : Rebases against current master with changes from stem work. #

Patch Set 5 : Rebase against current master #

Unified diffs Side-by-side diffs Delta from patch set Stats (+60 lines, -11 lines) Patch
M lily/include/stem-tremolo.hh View 1 2 3 1 chunk +3 lines, -1 line 0 comments Download
M lily/stem-tremolo.cc View 1 2 3 5 chunks +55 lines, -10 lines 0 comments Download
M scm/define-grobs.scm View 1 2 3 4 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 11
MikeSol
Hey all, My summer of Lily plugs along with a fix for Issue 1328. It ...
7 years, 10 months ago (2011-08-10 12:58:10 UTC) #1
pkx166h
Passes Make and there are some reg test differences - see http://code.google.com/p/lilypond/issues/detail?id=1328#c2
7 years, 10 months ago (2011-08-10 21:01:20 UTC) #2
joeneeman
http://codereview.appspot.com/4860042/diff/1/flower/polynomial.cc File flower/polynomial.cc (right): http://codereview.appspot.com/4860042/diff/1/flower/polynomial.cc#newcode65 flower/polynomial.cc:65: Polynomial::minmax (Real l, Real r, bool dir) const Perhaps ...
7 years, 10 months ago (2011-08-18 21:36:45 UTC) #3
MikeSol
Thanks Joe! New patchset uploaded. Cheers, MS http://codereview.appspot.com/4860042/diff/1/flower/polynomial.cc File flower/polynomial.cc (right): http://codereview.appspot.com/4860042/diff/1/flower/polynomial.cc#newcode65 flower/polynomial.cc:65: Polynomial::minmax (Real ...
7 years, 10 months ago (2011-08-19 07:03:50 UTC) #4
hanwenn
http://codereview.appspot.com/4860042/diff/8001/flower/polynomial.cc File flower/polynomial.cc (right): http://codereview.appspot.com/4860042/diff/8001/flower/polynomial.cc#newcode80 flower/polynomial.cc:80: return ret_max ? sols.back () : sols[0]; you could ...
7 years, 10 months ago (2011-08-23 04:21:37 UTC) #5
joeneeman
http://codereview.appspot.com/4860042/diff/1/lily/bezier.cc File lily/bezier.cc (right): http://codereview.appspot.com/4860042/diff/1/lily/bezier.cc#newcode239 lily/bezier.cc:239: return p.minmax (sol[LEFT][0], sol[RIGHT][0], d != LEFT); On 2011/08/19 ...
7 years, 10 months ago (2011-08-23 04:55:57 UTC) #6
mikesol_ufl.edu
On Aug 23, 2011, at 6:21 AM, hanwenn@gmail.com wrote: > > http://codereview.appspot.com/4860042/diff/8001/flower/polynomial.cc > File flower/polynomial.cc ...
7 years, 10 months ago (2011-08-23 07:35:49 UTC) #7
mikesol_ufl.edu
On Aug 23, 2011, at 6:55 AM, joeneeman@gmail.com wrote: > > http://codereview.appspot.com/4860042/diff/1/lily/bezier.cc > File lily/bezier.cc ...
7 years, 10 months ago (2011-08-23 07:36:37 UTC) #8
MikeSol
Pushed as 29d1121d260318ed07f152c346a1a69e5dadb69d. Many thanks to Joe and Han-Wen for your comments. Han-Wen: I didn't ...
7 years, 10 months ago (2011-08-25 07:40:28 UTC) #9
hanwenn
On Thu, Aug 25, 2011 at 4:40 AM, <mtsolo@gmail.com> wrote: > Pushed as 29d1121d260318ed07f152c346a1a69e5dadb69d. > ...
7 years, 10 months ago (2011-08-25 13:18:39 UTC) #10
joeneeman
7 years, 10 months ago (2011-08-25 16:03:09 UTC) #11
On Tue, Aug 23, 2011 at 12:36 AM, Mike Solomon <mikesol@ufl.edu> wrote:
> On Aug 23, 2011, at 6:55 AM, joeneeman@gmail.com wrote:
>
>>
>> http://codereview.appspot.com/4860042/diff/1/lily/bezier.cc
>> File lily/bezier.cc (right):
>>
>> http://codereview.appspot.com/4860042/diff/1/lily/bezier.cc#newcode239
>> lily/bezier.cc:239: return p.minmax (sol[LEFT][0], sol[RIGHT][0], d !=
>> LEFT);
>> On 2011/08/19 07:03:50, MikeSol wrote:
>>> On 2011/08/18 21:36:45, joeneeman wrote:
>>> > If there are multiple intersections with (say) r, then
>> Polynomial::solve
>>> doesn't
>>> > seem to return them in any useful order. So sol[RIGHT][0] is really
>> just an
>>> > arbitrary solution, isn't it?
>>
>>> True, but this seems no worse than line 81 where ts[0] is returned.
>> Not that
>>> this is a good excuse...
>>
>> Right, but what really bothers me is that you're then using
>> sol[RIGHT][0] as though it means something. So what this function seems
>> to do (suppose ax=X_AXIS) is to take an arbitrary point where the curve
>> intersects x=r and an arbitrary point where the curve intersects x=l and
>> then finds the maximum y value of the curve between those two points.
>
> True.
> A well-formed slur should never retrograde along the X-axis, and thus, the
size of sol[LEFT] and sol[RIGHT] should be 1 after filtering out all values less
than 0 and greater than 1.
> I think that the programming errors in the current patch should do the trick.

Is it so difficult to just make the solution general? Assuming axis is
Y_AXIS and d is UP, the maximum must appear either at a critical point
of the y-axis polynomial or when the x-axis polynomial is equal to
either l or r. You can filter the critical points to make sure the x
coordinate is within the correct interval, and then filter everything
to make sure that the parameter is between 0 and 1. You'll have no
more than 8 points left and you take the one with the largest Y
coordinate.

I realize that it's unlikely to trigger, at least in the way that
Lilypond currently uses this code, but I do feel that code in a file
called bezier.cc should work for bezier curves in general.

Cheers,
Joe
Sign in to reply to this message.

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