I guess I should have subscribed to the list before publishing/mailing
my patch. ;)
Well, 2nd attempt...
All the best
Torsten
https://codereview.appspot.com/6854106/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
Hi James,
I don't have push access, so it'd be nice if you pushed it for me.
Torsten
https://codereview.appspot.com/6854106/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
On 2018/06/12 10:05:17, dak wrote:
However, cough cough, all this is an unnecessary complication. Sorry
for
overlooking this. Just write
Stencil q = *s;
[…]
Just to mention it:
How about the definition of ly:stencil-rotate in lily/stencil-scheme.cc?
There, we should have the exact same
First of all: thanks for taking care of this issue!
But the resulting skylines don't quite work as expected yet (see
attached image in the issue tracker). Seems to be a problem with the
centre of rotation co-ordinates.
Greetings,
Torsten
https://codereview.appspot.com/344960043/
Yup, that's it!
Now it behaves exactly identical to Stencil::rotate_degrees and the PNG
regtests are successfull, too.
Thanks!
Torsten
https://codereview.appspot.com/341380043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://codereview.appspot.com/344970043/diff/1/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):
https://codereview.appspot.com/344970043/diff/1/lily/stencil-integral.cc#newcode1099
lily/stencil-integral.cc:1099: Offset center (robust_scm2double
(scm_cadr (rot), 0.0),
Reviewers: ,
Message:
Please review (ideally DAK)
Thanks,
Torsten
Description:
issue #5341: Urgent corrections to stencil-integral.cc (skylines)
modified: ../lily/stencil-integral.cc
Main correction:
Stencil::skylines_from_stencil
Avoid garbage collection by introducing an SCM variable
On 2018/05/03 17:42:22, lemzwerg wrote:
LGTM
https://codereview.appspot.com/342060043/diff/1/lily/key-signature-interface.cc
File lily/key-signature-interface.cc (right):
https://codereview.appspot.com/342060043/diff/1/lily/key-signature-interface.cc#newcode114
Hi Carl,
Concise, comprehensible, - LGTM!
Perhaps it should be explicitly pointed out that the duration shorthand
does not work for rests.
There have been some misconceptions lately on the user list, and so I
think this detail probably deserves special attendance.
Thanks,
Torsten
Reviewers: ,
Message:
Please review...
Inadvertently, file
../lily/key-signature-interface.cc
slipped into the patch (from issue 5312 currently in staging).
So ignore is (or shall I upload a new, cleansed patch for reviewing?)
Thanks,
Torsten
Description:
issue 5319: Make skylines reflect
Reviewers: ,
Message:
Please have a look at this.
I'll update issue tracker manually since I haven't got project
authorization (yet).
Thanks a lot
Torsten
Description:
issue #3653: Beam ends not matched to StaffSymbol thickness
file: lily/beam.cc
function: Beam::calc_beam_segments
Tiny flaw
Reviewers: lemzwerg, Dan Eble,
https://codereview.appspot.com/337560043/diff/1/lily/beam-quanting.cc
File lily/beam-quanting.cc (right):
https://codereview.appspot.com/337560043/diff/1/lily/beam-quanting.cc#newcode717
lily/beam-quanting.cc:717: if ((concaveness >= 1) || (damping >=
1))
On 2018/02/23 11:41:13, Malte Meyn wrote:
Probably that won’t be enough, it has to be 7,8 → 4 and 9,10 → 5; but
the idea
is correct. Maybe I’ll replace the cond by an if and case ;)
Ah, yes, it's true that the rest height is identical for 128th/256th and
512th/1024th, therefore my
On 2018/02/23 07:30:00, lemzwerg wrote:
LGTM, thanks!
In my opinion, there's a misplacement of rest dots. Therefore,
file scm/output-lib.scm (dots::calc-staff-position)
will have to be adapted, too (see related image and comment in issue
#5277)
There's no appropriate code for log > 7 (the
On 2018/08/13 16:56:51, dak wrote:
[…]
Hi David,
Thanks for the hints, I'll look into issues 4065 and 677.
As far as layout staff sizes, \magnifyStaff, fontSize/staff-space and
all the others are concerned (I've tested many variations back and
forth) the results were (surprisingly) stable.
Reviewers: dak,
Message:
On 2018/08/13 12:17:29, dak wrote:
Does not help with the other problems we had with fontsize mismatches
(out of
multiple calls of set-global-staff-size), right?
What would a correct solution look like?
Hmmm, what were the other fontsize mismatch problems?
The
On 2018/08/13 16:21:10, lemzwerg wrote:
LGTM, thanks!
https://codereview.appspot.com/341450043/diff/1/input/regression/book-change-global-staffsize-abs-fonts.ly
File input/regression/book-change-global-staffsize-abs-fonts.ly
(right):
Reviewers: ,
Message:
Please review.
Thanks,
Torsten
Description:
issue 5393: Another fingering vs. accidental problem
modified: ../input/regression/finger-chords-accidental.ly
modified: ../input/regression/finger-chords-dot.ly
Existing regression tests supplemented by
Reviewers: ,
Message:
Please review.
Thanks,
Torsten
Description:
issue 5413: X-aligning problem with chords containing unisons
Changes to be committed:
modified: ../lily/stem.cc Stem::extremal_heads
- To avoid confusion, process noteheads in <> from left to right
- lowest note: return
Reviewers: ,
Message:
Please review.
Thanks,
Torsten
Description:
issue 3692: Fingering collision with accidentals
Changes to be committed:
modified: ../Documentation/changes.tely
- Announcing the new X-align-on-main-noteheads feature
for the New_fingering_engraver
new file:
Reviewers: ,
Message:
Please review...
Thanks
Torsten
Description:
issue 5307: Skyline Refinements (Rounded Boxes and Rotated Ellipses)
lily/stencil-integral.cc
function make_partial_ellipse_boxes:
Use correct scaling factors for x_rad and y_rad for an arbitrary
transformation matrix
This
lily/stencil-integral.cc:410: int quantization = max (0, (int)
(rounded *
(x_scale + y_scale) * radius * M_PI / QUANTIZATION_UNIT / 4));
[Please use a line length of at most 78 characters if possible.]
In the original coding, many lines exceed 78 characters (including the
one you mention
On 2018/04/24 17:59:31, Carl wrote:
LGTM. I am just a *little* bit concerned about having the dimensions
of the
Emmentaler natural glyph hardcoded in the source, but we already have
magic
numbers reflecting the characteristics of the Emmentaler glyphs.
Maybe it would be good to put a
On 2018/04/24 21:49:37, Carl wrote:
On 2018/04/24 18:43:45, Be-3 wrote:
> The intervals are just *approximating* the outlines of a run-of-the
mill
> natural glyph. I even played around with the concept using squared
paper.
> This approach more or less relies on the fact that the
Reviewers: ,
Message:
Please review...
Thanks,
Torsten
PS: no new regression test needed, there are loads of key cancellations
in the existing regression tests.
Description:
issue 5312: Key cancellation glyph position inconsistent
file lily/key-signature-interface.cc
Using two intervals
Reviewers: lemzwerg,
Message:
On 2020/02/19 08:05:57, lemzwerg wrote:
> LGTM, thanks!
>
>
https://codereview.appspot.com/577520043/diff/555310043/python/musicexp.py
> File python/musicexp.py (left):
>
>
https://codereview.appspot.com/577520043/diff/555310043/python/musicexp.py#oldcode365
>
> I'll really make myself unpopular here, […]
No, that's what discussions are for.
I probably shouldn't have uploaded a second patch set before the
property
name had finally been decided upon, but I was kind of urged by the
countdown
starting and didn't have the time for working on it earlier.
On 2020/02/26 21:04:59, Be-3 wrote:
> Changes applied following the reviewers' comments
All of the recommendations applied:
Rename stru Beam_stem_length -> Beam_stem_end (Han-Wen)
Rename property french-correction -> stem-end-shorten (Han-Wen)
Make property stem-end-shorten
Reviewers: ,
Message:
Please review.
Description:
Issue 5806: Tweak mf files to avoid FontForge internal overlap error
When compiling LilyPond, there were 352 error messages caused by
FontForge trying to resolve overlapping path problems:
>>> Internal Error (overlap) in [...]
Fortunately, the
On 2020/02/29 10:44:40, antlists_youngman.org.uk wrote:
> Smelling pistake, sorry ... - just spotted it.
Hi Wol,
Too late - this has already been pushed.
But as it's only the commit message description, it won't have
any effect anyway.
But taking a closer look, I've found "propoer" with an extra
Re-formatted: spaces inserted and strictly 8 char indentations.
https://codereview.appspot.com/571780043/diff/549620043/mf/feta-arrowheads.mf
File mf/feta-arrowheads.mf (right):
https://codereview.appspot.com/571780043/diff/549620043/mf/feta-arrowheads.mf#newcode58
mf/feta-arrowheads.mf:58: y5
@multitable column entries are for spacing purposes only. In this case,
they contain the exceptionally long English names (independent of the
document language).
https://codereview.appspot.com/577520043/diff/555310043/Documentation/es/notation/pitches.itely
File
OK, now I got it.
Suppose I've erroneously treated these cases as broken line + 8-char
indent.
Now it should be the way you want it to be.
Cheers,
Torsten
https://codereview.appspot.com/571780043/
On 2020/02/24 06:44:39, hanwenn wrote:
> One thing to consider: since the mechanics are now very predictable,
maybe we
> can name the property in after its mechanics? ie. french-correction ->
> stem-end-shorten or something?
After having thought about it for quite a while I'm not too happy with
On 2020/02/25 13:33:13, lemzwerg wrote:
> If you really wanted 'i.e.' without a trailing comma, it would be
necessary to
> write 'i.e.@:'. Otherwise makeinfo inserts two spaces in the '.info'
file
> because we don't use '@frenchspacing' in the English docs.
Thanks for the valuable explanation.
Proposed changes applied to my local branch (most of them), see
comments.
Ta,
Torsten
https://codereview.appspot.com/557500043/diff/551490044/Documentation/changes.tely
File Documentation/changes.tely (right):
I think I forgot go mention that I now added
Patch set 2: Indentation and separating spaces
Formatting changes as proposed by Werner.
https://codereview.appspot.com/571780043/
Reviewers: ,
Message:
Please review.
Ta,
Torsten
Description:
Issue 5815: Mention Windows 10 in Windows package download
Windows 10 added in all occurrences.
Czech text changed from English to Czech.
Duplicate Czech file removed (wrong directory)..
modified:
Reviewers: ,
Message:
Please review.
Ta,
Torsten
Description:
Issue 5808: Web: Productions - typo fix
modified: Documentation/web/introduction.itexi
Please review this at https://codereview.appspot.com/557570043/
Affected files (+1, -1 lines):
M
We still have issues with English THREE-Q-* pitches (never worked,
though).
Please see my comment in musicexp.py
Torsten
https://codereview.appspot.com/547610043/diff/557380043/python/musicexp.py
File python/musicexp.py (right):
Proposed solution for English
(I know that this issue is about German only, but as English has been
touched, too, by removing a pointless replace statement).
https://codereview.appspot.com/547610043/diff/557380043/python/musicexp.py
File python/musicexp.py (right):
Reviewers: ,
Message:
Deep breath---please review.
Ta, Torsten
Description:
Issue 5788: New French Beamimg Approach
Completely new approach to French beaming.
This will automatically tackle all kinds of not-yet resolved positioning
problems caused by the current French beaming implementation.
On 2020/02/15 16:58:45, hanwenn wrote:
>
https://codereview.appspot.com/571630043/diff/561450045/python/musicexp.py
> File python/musicexp.py (right):
>
>
https://codereview.appspot.com/571630043/diff/561450045/python/musicexp.py#newcode364
> python/musicexp.py:364: function_dict = {
> I meant
Pls see my comment about a patch introducing full quarter tone support
for all the languages - including a complete and consistent MusicXML
import language support.
I've asked on dev list about introducing català and português as
"official" consistent proper language names.
It's already included
On 2020/02/11 20:54:19, dak wrote:
>
https://codereview.appspot.com/547610043/diff/557380043/python/musicexp.py
> File python/musicexp.py (right):
>
>
https://codereview.appspot.com/547610043/diff/557380043/python/musicexp.py#newcode324
> python/musicexp.py:324: return str
> On 2020/02/11
German and English successfully tested.
proposal for espanol (missing quarter notes) and portuques (still
completely missing) added (as it is about 2.20)
https://codereview.appspot.com/547610043/
Reviewers: ,
Message:
Please review.
Ta,
Torsten
Description:
Issue 5829: Re-indent all mf files
As a second step after having replaced all tabs by 8 character
indentations,
these indentations will now be reduced to 2 characters.
"Tabs are used..." passage removed from
Contributor's Guide
LGTM (in combination with issue 5833). Many users of Windows will be
quite happy about this (myself included).
Wouldn't this have merited an entry in changes.tely? After all, UTF-8
filenames had never worked in Windows from the beginning, so it'd
probably be a good idea to let the users know.
LGTM. Arigato!
https://codereview.appspot.com/551580045/
On 2020/03/08 11:38:29, dak wrote:
> We don't really document bug fixes as a rule in changes.tely. It's
pretty long
> already...
Yes, certainly.
But I think this is a special case. Speaking for myself, I'm taking it
for granted that UTF-8 filenames just can't be handled by the Windows
version
Proposed corrections/changes applied to
input/regression/beaming-more-than-4-beams-normal-size.ly
in my local branch.
https://codereview.appspot.com/559700043/diff/545760043/input/regression/beaming-more-than-4-beams-normal-size.ly
File input/regression/beaming-more-than-4-beams-normal-size.ly
Reviewers: ,
Message:
Please review.
TIA,
Torsten
Description:
Issue 5036: 128 beaming output not producing output as expected (?)
In case of more than 4 normal-sized beams, a grid shift correction
will be applied to the outside-staff quants so that the inner beams
within the staff can be
52 matches
Mail list logo
