Hi,
I think I found a proper way to calculate church rests.
I also updated so that it applies on the latest git HEAD.
Bertrand.
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailma
hi Bertrand,
(I'm not a grand-master, :( )
good that you caught the problem with hole in usable-duration-logs, but,
I think, that makes more comment necessary, see below.
all reviewers: is there a convention about using assertions in C++ code?
http://codereview.appspot.com/4536068/diff/55001/
On 2011/07/31 20:10:30, J_lowe wrote:
Passes Make and there is a reg test difference which looks ok. I
created a
tracker
http://code.google.com/p/lilypond/issues/detail?id=1794
so people can see this but also because this has been going on for a
while and
the tracker will at least keep
Passes Make and there is a reg test difference which looks ok. I created
a tracker
http://code.google.com/p/lilypond/issues/detail?id=1794
so people can see this but also because this has been going on for a
while and the tracker will at least keep this on people's radars.
http://codereview.app
Hi Pál (besides, are you Pál Benkő the chess master?)
Thanks for this nice review.
1. about the very existence of usable-duration-logs -
ok, it's generic, but who uses this genericity?
is it not always (0 -1 -2 -3)?
is it not always a range with lower end -3?
is it not always a
hi Bertrand,
the patch is correct, AFAICS; see some minor improvements below.
minor concerns (which shouldn't delay acception):
1. about the very existence of usable-duration-logs -
ok, it's generic, but who uses this genericity?
is it not always (0 -1 -2 -3)?
is it not always a range
It doesn't need to be ordered.
It can have holes, but there's a small issue with this for now:
Church rests are only looking for maximum and minimum values. You can
therefore find longas in a church rest if you set usable-duration-logs
to '(0 -1 -3).
I don't know whether it's better to keep this s
hi Bertrand,
I started at the patch but it's quite difficult for now,
I hope I'll have enough time in the evening. till then
could you tell me whether usable-duration-logs is
ordered? is it a range or can it have holes?
thanks,
p
___
lilypond-devel m
> I should have write "2 << (-i + 1)" instead of "/2"...
the +1 multiplies by 2, not divides.
Whoops, I meant -(i + 1)...
perhaps the best would be 1 << -i.
Thanks.
When I told you I never understood "<<", I wasn't kidding!
An update will follow this comment.
Bertrand
http://codereview.
> To be honest, I never understood well how this bitset operator works.
adds trailing zeros in binary.
> What I see is that "2 << -i" gives the same result than "2^(-i+1)".
> I should have write "2 << (-i + 1)" instead of "/2"...
the +1 multiplies by 2, not divides.
perhaps the best would be 1
http://codereview.appspot.com/4536068/diff/37002/lily/multi-measure-rest.cc#newcode241
lily/multi-measure-rest.cc:241: length = (2 << -i) / 2;
The division by 2 changes the result. Not that I understand too well
what it is
supposed to be doing.
To be honest, I never understood well how this
Works good for me.
On 2011/07/27 17:49:01, MikeSol wrote:
Why not just an array that acts as a log table?
int foo[10] = {1,2,4,8,16,32,64,128,256,512};
Bertrand already has something better, walking along the (very short)
list of usable-duration-logs.
http://codereview.appspot.com/453606
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (right):
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode131
lily/multi-measure-rest.cc:131: int measure_duration_log = int (ceil
(duration_log));
On 201
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (right):
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode131
lily/multi-measure-rest.cc:131: int measure_duration_log = int (ceil
(duration_log));
On 201
This is really dirty, but here's an update that does what you want (I
hope).
If you have ideas to clean a bit this mess...
Thanks,
Bertrand
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (right):
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode131
lily/multi-measure-rest.cc:131: int measure_duration_log = int (ceil
(duration_log));
To me,
On 2011/07/26 22:47:53, Bertrand Bordage wrote:
>
> For example, what to do about 3/2? We either get 2/1 rests, or 1/1
rests, and
> it is not clear to me that this rounding choice is necessarily the
same as
that
> for 3/4.
>
> I think it might be saner to [...]
This sounds complex...
Ho
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode130
lily/multi-measure-rest.cc:130: double duration_log = -log2
(ml.Rational::to_double ());
log2 is a floating point operation not guaranteed to be exact. One
usually uses
a loop for figuring out a proper inte
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (right):
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode254
lily/multi-measure-rest.cc:254: Stencil r (musfont->find_by_name
("rests." + to_string (k)))
Okay, I will fix these before the end of the day.
Thanks for taking time to do the review !
Bertrand
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
I would suggest reverting this patch as "needs work" for now.
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (right):
http://codereview.appspot.com/4536068/diff/30001/lily/multi-measure-rest.cc#newcode125
lily/multi-measure-rest.cc:12
thanks, pushed.
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Yes, thanks. 'Tis done.
Bertrand
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 6 June 2011 11:13, wrote:
> Thanks again, Neil !
> I applied these.
Thanks.
There's one more static_cast here:
+ length = static_cast (pow (2, -i));
Cheers,
Neil
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailm
Please send me the final version with
git format-patch origin
so that I can push it.
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
Thanks again, Neil !
I applied these.
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
LGTM.
http://codereview.appspot.com/4536068/diff/17008/input/regression/multi-measure-rest-standard.ly
File input/regression/multi-measure-rest-standard.ly (right):
http://codereview.appspot.com/4536068/diff/17008/input/regression/multi-measure-rest-standard.ly#newcode20
input/regression/multi-
Thanks, 'tis done.
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
http://codereview.appspot.com/4536068/diff/10008/lily/multi-measure-rest-engraver.cc
File lily/multi-measure-rest-engraver.cc (right):
http://codereview.appspot.com/4536068/diff/10008/lily/multi-measure-rest-engraver.cc#newcode232
lily/multi-measure-rest-engraver.cc:232: last_rest_->set_property
Carl's suggestions done !
Bertrand
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
longest-church-rest is the longest rest displayed in church rests.
One may want to only print a whole rest in a breve measure but print
breve and longa rests in a church rest.
But I agree with you, it can be changed for the min value of
usable-duration-logs.
You're right for measure-duration-log.
The code looks fine in general, but I question two of the properties
that have been added for MultiMeasureRest.
http://codereview.appspot.com/4536068/diff/19001/lily/multi-measure-rest.cc
File lily/multi-measure-rest.cc (right):
http://codereview.appspot.com/4536068/diff/19001/lily/multi-measur
Nitpicks done :
"duration-log-list" changed for "usable-duration-logs" (Thanks Carl !)
Whitespaces and tabs removed.
Bertrand
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/l
> By maxima, I mean the glyph "rests.M3".
> I chose to use maximas because they are easier to read than two longas
> with many space in between.
> In such cases, this is mandatory :
> { \compressFullBarRests \time 4/1 R\longa*10 }
>
> http://codereview.appspot.com/4536068/
>
ok, I fully agree. so
Sorry, I misread your first sentence. Forget the first line of my
previous post.
http://codereview.appspot.com/4536068/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
I applied it to the more recent git HEAD without any problem.
By maxima, I mean the glyph "rests.M3".
I chose to use maximas because they are easier to read than two longas
with many space in between.
In such cases, this is mandatory :
{ \compressFullBarRests \time 4/1 R\longa*10 }
http://coder
http://codereview.appspot.com/4536068/diff/1/scm/define-grobs.scm
File scm/define-grobs.scm (right):
http://codereview.appspot.com/4536068/diff/1/scm/define-grobs.scm#newcode1305
scm/define-grobs.scm:1305: (duration-log-list . (0 -1 -2 -3))
On 2011/05/18 13:54:43, Bertrand Bordage wrote:
Just o
This name is nice and generic, which is good. Bit it has no
information content
as far as I can see. Can we make it more explicit by changing either
the name
(to something like usable-duration-logs) or the description (to
something like
"List of duration-logs that can be used in typesettin
Reviewers: ,
Message:
Just one regtest change :
in multi-measure-rest.ly, the two longas of the third measure are
tranformed into a maxima (as expected).
Regards,
Bertrand
Description:
Adds longas, maximas and non-standard tweaks to MultiMeasureRest
Please review this at http://codereview.apps
This looks generally good to me.
I'm concerned about the name "duration-log-list". I've commented more
on it below.
Thanks,
Carl
http://codereview.appspot.com/4536068/diff/1/scm/define-grob-properties.scm
File scm/define-grob-properties.scm (right):
http://codereview.appspot.com/4536068/di
40 matches
Mail list logo
