Re: Issue 5036: 128 beaming output not producing output as expected (?) (issue 559700043 by torsten.haemme...@web.de)

2020-03-24 Thread torsten . haemmerle
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 (right):

https://codereview.appspot.com/559700043/diff/545760043/input/regression/beaming-more-than-4-beams-normal-size.ly#newcode7
input/regression/beaming-more-than-4-beams-normal-size.ly:7: smoothly as
possible (standard-sized beams).  The outide-staff beams will
On 2020/03/24 18:54:34, lemzwerg wrote:
> s/outide/outside/, and please avoid future tense.

Done. "outside" corrected, "will not" replaced by "do not".

https://codereview.appspot.com/559700043/diff/545760043/input/regression/beaming-more-than-4-beams-normal-size.ly#newcode9
input/regression/beaming-more-than-4-beams-normal-size.ly:9: when it
comes to beam quanting/scoring/positioning."
On 2020/03/24 18:54:34, lemzwerg wrote:
> Please add `/@` after the `/`s to allow line breaks (in the PDF
output).

Good point - done!
But it's @/, --> "quanting/@/scoring/@/positioning".

https://codereview.appspot.com/559700043/



Issue 5036: 128 beaming output not producing output as expected (?) (issue 559700043 by torsten.haemme...@web.de)

2020-03-24 Thread torsten . haemmerle
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 neatly aligned to the staff lines.

If all the beams fall outside the staff (cf. stem shortening), this
grid shift will ensure a consistent vertical alignment of beams.
See regtest beam-shortened-lengths.ly

regression beam-shortened-lengths.ly: new durations 256, 512, and 1024
added.

new regression beaming-more-than-4-beams-normal-size.ly

Please review this at https://codereview.appspot.com/559700043/

Affected files (+66, -7 lines):
  M input/regression/beam-shortened-lengths.ly
  A input/regression/beaming-more-than-4-beams-normal-size.ly
  M lily/beam-quanting.cc
  M lily/include/beam-scoring-problem.hh


Index: input/regression/beam-shortened-lengths.ly
diff --git a/input/regression/beam-shortened-lengths.ly 
b/input/regression/beam-shortened-lengths.ly
index 
057e602963f7386ff6db72911c342d036d7fe799..cc4d5fb78bcd4db0f2f093b08a219cde5beab8ff
 100644
--- a/input/regression/beam-shortened-lengths.ly
+++ b/input/regression/beam-shortened-lengths.ly
@@ -1,4 +1,4 @@
-\version "2.16.0"
+\version "2.21.0"
 
 \header{
   texidoc="Beams in unnatural direction, have shortened stems, but do not look 
too short."
@@ -8,5 +8,5 @@
 
 \relative c'{
   \stemUp
-  f'4  f8[ f]  f16[ f]  f32[ f]  f64[ f]  f128[ f] 
+  f'4  f8[ f]  f16[ f]  f32[ f]  f64[ f]  f128[ f] f256[ f] f512[ f] f1024[ f]
 }
Index: input/regression/beaming-more-than-4-beams-normal-size.ly
diff --git a/input/regression/beaming-more-than-4-beams-normal-size.ly 
b/input/regression/beaming-more-than-4-beams-normal-size.ly
new file mode 100644
index 
..dc63626e771aab501059275441b10f2c016e0427
--- /dev/null
+++ b/input/regression/beaming-more-than-4-beams-normal-size.ly
@@ -0,0 +1,39 @@
+
+\version "2.21.0"
+
+\header{
+texidoc="
+Inside-staff beams should align with staff lines (sit, straddle, hang) as
+smoothly as possible (standard-sized beams).  The outide-staff beams will
+not interfere with staff lines, so the inside-staff beams are more important
+when it comes to beam quanting/scoring/positioning."
+}
+
+\layout {
+  indent = 0
+  line-width = 100
+  ragged-right = ##t
+  \context {
+\Staff
+\omit Clef
+\omit TimeSignature
+  }
+}
+
+testMusic = {
+  \cadenzaOn
+   a8[ 8]
+   16[ 16]
+   32[ 32]
+   64[ 64]
+   128[ 128]
+   256[ 256]
+   512[ 512]
+   1024[ 1024]
+   128[ 64 32 16 8]
+}
+
+<<
+  \new Staff \testMusic
+  \new Staff \transpose a c''' \testMusic
+>>
Index: lily/beam-quanting.cc
diff --git a/lily/beam-quanting.cc b/lily/beam-quanting.cc
index 
8657bac2f7eb66957aaa09fc2fa1112bb8c5d758..852cedf9063b8da4af014ac0d6969d03e6cd7d66
 100644
--- a/lily/beam-quanting.cc
+++ b/lily/beam-quanting.cc
@@ -220,7 +220,8 @@ void Beam_scoring_problem::init_instance_variables (Grob 
*me, Drul_array y
   staff_space_ = Staff_symbol_referencer::staff_space (beam_);
   beam_thickness_ = Beam::get_beam_thickness (beam_) / staff_space_;
   line_thickness_ = Staff_symbol_referencer::line_thickness (beam_) / 
staff_space_;
-
+  length_fraction_
+= robust_scm2double (beam_->get_property ("length-fraction"), 1.0);
   // This is the least-squares DY, corrected for concave beams.
   musical_dy_ = robust_scm2double (beam_->get_property ("least-squares-dy"), 
0);
 
@@ -876,6 +877,17 @@ Beam_scoring_problem::generate_quants 
(vector> *s
   Real hang = 1.0 - (beam_thickness_ - line_thickness_) / 2;
   Real base_quants [] = {straddle, sit, inter, hang};
   int num_base_quants = int (sizeof (base_quants) / sizeof (Real));
+  int max_beam_count = std::max (edge_beam_counts_[LEFT],
+ edge_beam_counts_[RIGHT]);
+
+  /* for normal-sized beams, in case of more than 4 beams, the outer beam
+ used for generating quants will never interfere with staff lines, but
+ prevent the inside-staff beams from being neatly positioned.
+ A correctional grid_shift has to be applied to compensate. */
+  Real grid_shift = 0.0;
+  /* grid shift only makes sense for widened normal-sized beams: */
+  if (!is_knee_ && max_beam_count > 4 && length_fraction_ == 1.0)
+grid_shift = (max_beam_count - 4) * (1.0 - beam_translation_);
 
   /*
 Asymetry ? should run to <= region_size ?
@@ -890,10 +902,17 @@ Beam_scoring_problem::generate_quants 
(vector> *s
   for (vsize i = 0; i < unshifted_quants.size (); i++)
 for (vsize j = 0; j < unshifted_quants.size (); j++)
   {
-auto c
-  = Beam_configuration::new_config (unquanted_y_,
-Interval (unshifted_quants[i],
-  unshifted_quants[j]));
+Interval corr (0.0, 0.0);
+if (grid_shift)
+  for (LEFT_and_RIGHT (d))
+ 

Re: Fix Windows resource to enable manifest (issue 551580044 by truer...@gmail.com)

2020-03-08 Thread torsten . haemmerle
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 of LilyPond and so I'm not using them, thus never having a
chance to discover that all of a sudden, the problem has vanished.

To me, this feels rather like an enhancement/new feature than a bugfix,
as in "LilyPond for Windows can handle UTF-8 filenames now".
I was just suggesting.

https://codereview.appspot.com/551580044/



Fix MinGW UTF-8 locale settings (issue 551580045 by truer...@gmail.com)

2020-03-08 Thread torsten . haemmerle
LGTM.  Arigato!

https://codereview.appspot.com/551580045/



Re: Fix Windows resource to enable manifest (issue 551580044 by truer...@gmail.com)

2020-03-08 Thread torsten . haemmerle
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.

Thanks,
Torsten

https://codereview.appspot.com/551580044/



Issue 5829: Re-indent all mf files (issue 573610048 by torsten.haemme...@web.de)

2020-03-07 Thread torsten . haemmerle
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 13.6 METAFONT formatting rules.

Please review this at https://codereview.appspot.com/573610048/

Affected files (+13300, -13352 lines):
  M Documentation/contributor/emmentaler-font.itexi
  M mf/debugging-settings.mf
  M mf/feta-accidentals.mf
  M mf/feta-accordion.mf
  M mf/feta-alphabet-generic.mf
  M mf/feta-arrow.mf
  M mf/feta-arrowheads.mf
  M mf/feta-autometric.mf
  M mf/feta-braces.mf
  M mf/feta-braces-generic.mf
  M mf/feta-brackettips.mf
  M mf/feta-clefs.mf
  M mf/feta-dots.mf
  M mf/feta-dynamics.mf
  M mf/feta-flags.mf
  M mf/feta-flags-generic.mf
  M mf/feta-flats.mf
  M mf/feta-macros.mf
  M mf/feta-naturals.mf
  M mf/feta-noteheads.mf
  M mf/feta-noteheads-generic.mf
  M mf/feta-numbers.mf
  M mf/feta-other-generic.mf
  M mf/feta-params.mf
  M mf/feta-parenthesis.mf
  M mf/feta-pedals.mf
  M mf/feta-rests.mf
  M mf/feta-scripts.mf
  M mf/feta-sharps.mf
  M mf/feta-ties.mf
  M mf/feta-timesignatures.mf
  M mf/feta-trills.mf
  M mf/parmesan-accidentals.mf
  M mf/parmesan-clefs.mf
  M mf/parmesan-custodes.mf
  M mf/parmesan-dots.mf
  M mf/parmesan-flags.mf
  M mf/parmesan-macros.mf
  M mf/parmesan-noteheads.mf
  M mf/parmesan-noteheads-generic.mf
  M mf/parmesan-other-generic.mf
  M mf/parmesan-rests.mf
  M mf/parmesan-scripts.mf
  M mf/parmesan-timesignatures.mf





Re: Issue 5806: Tweak mf files to avoid FontForge internal overlap error (issue 571780043 by torsten.haemme...@web.de)

2020-03-02 Thread torsten . haemmerle
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/



Issue 5808: Web: Productions - typo fix (issue 557570043 by torsten.haemme...@web.de)

2020-03-01 Thread torsten . haemmerle
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 Documentation/web/introduction.itexi


Index: Documentation/web/introduction.itexi
diff --git a/Documentation/web/introduction.itexi 
b/Documentation/web/introduction.itexi
index 
5a9d0149f34cd5b9bb62c8c36bb373ff3b562011..c55db079039a781cb792d8392c5d22927305e152
 100644
--- a/Documentation/web/introduction.itexi
+++ b/Documentation/web/introduction.itexi
@@ -669,7 +669,7 @@ Benjamin Britten's @emph{Saint Nicolas} which was performed 
in 2011 by
 @item
 @uref{https://unito.academia.edu/LucaRossettoCasel,Luca Rossetto Casel},
 for his Ph.D. thesis, created a critical edition of Tommaso Traetta's
-@emph{Enea nel Lazio (1760)} opera series, with libretto by Vittorio
+@emph{Enea nel Lazio (1760)}, opera seria on a libretto by Vittorio
 Amedeo Cigna-Santi, in four parts:
 
@uref{https://www.academia.edu/1987651/Enea_nel_Lazio_opera_riformata_prima_lazione_poi_le_parole_-_Partitura_1_4_,
 Part One}
 
@uref{https://www.academia.edu/1994533/Enea_nel_Lazio_opera_riformata_prima_lazione_poi_le_parole_-_Partitura_2_4_,
 Part Two}





Issue 5815: Mention Windows 10 in Windows package download (issue 553590043 by torsten.haemme...@web.de)

2020-03-01 Thread torsten . haemmerle
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:   Documentation/ca/web/download.itexi
deleted:Documentation/cs/download.itexi
modified:   Documentation/cs/web/download.itexi
modified:   Documentation/de/web/download.itexi
modified:   Documentation/es/web/download.itexi
modified:   Documentation/fr/web/download.itexi
modified:   Documentation/hu/web/download.itexi
modified:   Documentation/it/web/download.itexi
modified:   Documentation/ja/web/download.itexi
modified:   Documentation/nl/web/download.itexi
modified:   Documentation/web/download.itexi
modified:   Documentation/zh/web/download.itexi

Please review this at https://codereview.appspot.com/553590043/

Affected files (+11, -618 lines):
  M Documentation/ca/web/download.itexi
  D Documentation/cs/download.itexi
  M Documentation/cs/web/download.itexi
  M Documentation/de/web/download.itexi
  M Documentation/es/web/download.itexi
  M Documentation/fr/web/download.itexi
  M Documentation/hu/web/download.itexi
  M Documentation/it/web/download.itexi
  M Documentation/ja/web/download.itexi
  M Documentation/nl/web/download.itexi
  M Documentation/web/download.itexi
  M Documentation/zh/web/download.itexi





Re: Issue 5806: Tweak mf files to avoid FontForge internal overlap error (issue 571780043 by torsten.haemme...@web.de)

2020-03-01 Thread torsten . haemmerle
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/



Re: Issue 5773: Quarter Tones for all Languages incl. MusicXML import (issue 577520043 by torsten.haemme...@web.de)

2020-03-01 Thread torsten . haemmerle
@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 Documentation/es/notation/pitches.itely (right):

https://codereview.appspot.com/577520043/diff/555310043/Documentation/es/notation/pitches.itely#newcode558
Documentation/es/notation/pitches.itely:558: @multitable
{@w{@code{português} o}} {@code{s}/@code{-sharp}}
{@code{f}/@code{-flat}} {@code{ss}/@code{x}/@code{-sharpsharp}}
{@code{ff}/@code{-flatflat}}
On 2020/03/01 19:54:14, dak wrote:
> I am pretty sure that those aren't the Portuguese note names here.

No, definitely not.
BUT (!) the @multitable line is just for column width spacing, thus
mostly containing the column entry meant to determine the minimum column
width.

So the @multitable columns contain the entries determining the width of
the respective column (just for spacing, they will not be printed).

In this case, the longest language column should be as wide as
"portuquês o".
The longest note name column entry in this case will be the English
names, thanks to -flatflat etc.).
In other cases, the widest @multitable spacer entries will actually be
the column title (hence the misunderstanding, I suppose).

The column *titles* to be printed are specified in the @headitem line. 
They will, of course, vary in the different doc language versions.

In summary:  Everything's OK. @multitable != @headitem

Cheers,
Torsten

https://codereview.appspot.com/577520043/



Re: Issue 5806: Tweak mf files to avoid FontForge internal overlap error (issue 571780043 by torsten.haemme...@web.de)

2020-02-29 Thread torsten . haemmerle
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 = 0;
On 2020/02/29 17:23:39, lemzwerg wrote:
> Please use an indentation of 8 spaces for orthogonality.
> 
> We could later decide to reduce the indentation to two (or four)
spaces – which
> I like much better.  However, this would be a lot of work and is
probably not
> worth the trouble.

Done.

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-numbers.mf
File mf/feta-numbers.mf (left):

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-numbers.mf#oldcode467
mf/feta-numbers.mf:467: ..tension 0.9.. {dir (alpha + 10)}z2r
On 2020/02/29 17:23:40, lemzwerg wrote:
> indentation

Thta's the old code (left side).
Indentation changed to strictly 8 characters.

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-numbers.mf
File mf/feta-numbers.mf (right):

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-numbers.mf#newcode467
mf/feta-numbers.mf:467: pata:= subpath (0.1, 5) of z1r{dir (beta)}
On 2020/02/29 17:23:39, lemzwerg wrote:
> s/pata:=/pata :=/

Done.

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-numbers.mf#newcode493
mf/feta-numbers.mf:493: fill subpath (ta,length pata) of pata
On 2020/02/29 17:23:40, lemzwerg wrote:
> space after comma

Done.

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-numbers.mf#newcode494
mf/feta-numbers.mf:494: .. reverse subpath (tb,length patb) of patb
On 2020/02/29 17:23:40, lemzwerg wrote:
> indentation

Done.

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-numbers.mf#newcode750
mf/feta-numbers.mf:750: .. {dir (delta - 90)}z3r
On 2020/02/29 17:23:40, lemzwerg wrote:
> indentation

Done.

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-numbers.mf#newcode793
mf/feta-numbers.mf:793: patb := z13l{dir (180 + beta)}
On 2020/02/29 17:23:40, lemzwerg wrote:
> indentation

Done.

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-scripts.mf
File mf/feta-scripts.mf (left):

https://codereview.appspot.com/571780043/diff/549620043/mf/feta-scripts.mf#oldcode999
mf/feta-scripts.mf:999: .. z2l
On 2020/02/29 17:23:40, lemzwerg wrote:
> indentation

Yeah. But that's the old code on the left side.
Nevertheless, I've changed the indentation to 8 characters.

https://codereview.appspot.com/571780043/



Re: Issue 5773: Quarter Tones for all Languages incl. MusicXML import (issue 577520043 by torsten.haemme...@web.de)

2020-02-29 Thread torsten . haemmerle
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 o in
changes doc.  I'll try to sneak that correction in with another
issue. ;)

Ta,
Torsten


https://codereview.appspot.com/577520043/



Issue 5806: Tweak mf files to avoid FontForge internal overlap error (issue 571780043 by torsten.haemme...@web.de)

2020-02-29 Thread torsten . haemmerle
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 resulting glyphs had been OK, though.
FontForge should be able to cope with this, but, alas, it isn't.

I've now tweaked all the mf files causing these problems so that the
annoying error messages can be avoided without while keeping the
original resulting glyph outlines.

modified:   mf/feta-arrowheads.mf

- former def_set_arrow_paths (returning both open_path and close_path)
  has been replaced by def_set_arrow_path returning one path arrow_path
  depending on additional boolean parameter CLOSED
- No need to draw two overlapping mirrored half-paths anymore.
- Char definitions adapted accordingly
FIXES arrowheads.open.01
  arrowheads.open.0M1
  arrowheads.open.11
  arrowheads.open.1M1

modified:   mf/feta-braces.mf

- now draw one single cyclic path rather than overlaying two
  separate, mirrored half-paths
- using intersection and subpaths to exaclty retain the original
  slopes.
FIXES brace4 - brace507

modified:   mf/feta-clefs.mf

- draw_gclef: just one control point changed (hidden in overlapping
area)
FIXES clefs.G
  clefs.GG
  clefs.tenorG
- draw_tab_B: lower arc of the B in "TAB", angle slighty changed by 1°
FIXES clefs.tab

modified:   mf/feta-numbers.mf

- original paths in char definition re-arranged, intersected, joined,
  in order to create one single outline (two glyphs concerned):
- two char definitions affected:
FIXES two
FIXES seven

modified:   mf/feta-scripts.mf

- draw_turn: "ploop" overlay completely removed, using joined sub-
  paths instead
FIXES scripts.reverseturn
  scripts.turn
  scripts.slashturn
- "Segno" char definition: similar to draw_turn change
FIXES scripts.segno

modified:   mf/feta-timesignatures.mf

- draw_C: .. instead of -- (hidden in overlapping area)
FIXES timesig.C44
  timesig.C22

modified:   mf/parmesan-clefs.mf

- draw_neomensural_c_clef: .. instead of -- (hidden in overlap area)
FIXES clefs.neomensural.c
  clefs.neomensural.c_change
- draw_petrucci_c_clef: .. instead of .. (hidden in overlap area)
FIXES clefs.petrucci.c1
  clefs.petrucci.c2
  clefs.petrucci.c2_change
  clefs.petrucci.c3
  clefs.petrucci.c3_change
  clefs.petrucci.c4
  clefs.petrucci.c5
  clefs.petrucci.c5_change

modified:   mf/parmesan-rests.mf

- "2neomensural" char definition: one additional auxiliary point
  inserted in overlap area
FIXES rests.2neomensural

>>> RESULT: same glyphs, no FontForge errors anymore

Please review this at https://codereview.appspot.com/571780043/

Affected files (+169, -177 lines):
  M mf/feta-arrowheads.mf
  M mf/feta-braces.mf
  M mf/feta-clefs.mf
  M mf/feta-numbers.mf
  M mf/feta-scripts.mf
  M mf/feta-timesignatures.mf
  M mf/parmesan-clefs.mf
  M mf/parmesan-rests.mf





Re: Issue 5788: New French Beamimg Approach (issue 557500043 by torsten.haemme...@web.de)

2020-02-26 Thread torsten . haemmerle
> 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.

The property is a positive value that will be subtracted from the stem's
length.

How about
* beamed-stem-french-adjustment
* french-beaming-stem-adjustment
…? 

> But the main thing is that I'd like its name to contain
"french-beaming" because
> it is not intended to be used for anything else and not guaranteed to
work for
> anything else.

I know it's only internal, but a meaningful and unambiguous property
name would be 
desirable nevertheless.

https://codereview.appspot.com/557500043/



Re: Issue 5788: New French Beamimg Approach (issue 557500043 by torsten.haemme...@web.de)

2020-02-26 Thread torsten . haemmerle
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 (was: french-correction) internal
(DAK)

Corrections to doc text (Werner)

New regression file added directly comparing french beaming to
standard
beaming (Werner)

On my behalf:
Modified changes.tely LilyPond example to show more variations in
less
space so that it will not stick out into the margin in PDF version.

Removed an unneccessary if, just multiplying by int 0 in this case.

Full test successfully performed.


@DAK: Sorry, the french-correction property (now: stem-end-shorten) was
supposed
to be internal (as you guessed correctly in the end).  But I had
initially put it
in the all-user-grob-properties list in an early testing stage and plain
and
simply forgot to move it over to all-internal-grob-properties in time.

Cheers and thanks for reviewing,
Torsten




https://codereview.appspot.com/557500043/



Re: Issue 5788: New French Beamimg Approach (issue 557500043 by torsten.haemme...@web.de)

2020-02-25 Thread torsten . haemmerle
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.  That's usual business for me in
TeX,
but I'll have to admit that I'm not so much into texinfo.
Anyway: I'll use the American version (with comma). 

> 'being' would be correct if you continued the previous sentence. 
However, you
> are starting a new one, so you have to use a verb (in a normal form).

Aaaargh, yes, of course.  That's what's happening when altering a text
back
and forth, and you're perfectly right, of course.  I must have been
blind.

> I was probably unclear.  What I want is an example that shows how the
> 'french-correction' *property* changes the input.  This is (a) to
execute the
> code path, and (b) a visualization of the property change.

Ah, now I see your point.
I'll add a new regression test comparing standard and French beaming.
In the existing "stand-alone" French beaming tests, the up-to-now wrong
stem lengths had always been there, but nobody noticed due to lack of
comparison.

I'll upload a new patch set as soon as the Naming of the Shrew has
finally
been decided upon (french-correction -> stem-end-shorten?,
French-shorten?)

Cheers,
Torsten


https://codereview.appspot.com/557500043/



Re: Issue 5788: New French Beamimg Approach (issue 557500043 by torsten.haemme...@web.de)

2020-02-25 Thread torsten . haemmerle
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):

https://codereview.appspot.com/557500043/diff/551490044/Documentation/changes.tely#newcode67
Documentation/changes.tely:67: @emph{exactly} behaves like standard
(i.e. default) beaming in every respect
On 2020/02/24 05:24:25, lemzwerg wrote:
> s/i.e./i.e.,/

Ah, then I suppose the changes.tely language is American English...
In Europe, i.e. (sic!) British English, there's usually no comma after
i.e. or e.g.
But I'll update this and insert a comma.
Done.

https://codereview.appspot.com/557500043/diff/551490044/Documentation/changes.tely#newcode68
Documentation/changes.tely:68: (beam positioning and placement of any
articulation/fingering/etc.).  The
On 2020/02/24 05:24:25, lemzwerg wrote:
> ... articulation, fingering, etc.

Done.

https://codereview.appspot.com/557500043/diff/551490044/Documentation/changes.tely#newcode69
Documentation/changes.tely:69: only remaining difference being inner
stems not passing through beams.
On 2020/02/24 05:24:25, lemzwerg wrote:
> s/being/are/
I've heard this a lot of times, but, OK, "are" is probably simpler.
Done.

https://codereview.appspot.com/557500043/diff/551490044/input/regression/beam-french.ly
File input/regression/beam-french.ly (right):

https://codereview.appspot.com/557500043/diff/551490044/input/regression/beam-french.ly#newcode23
input/regression/beam-french.ly:23: { d32[^1 e^2 f^3 g^4 a^5] } s64
On 2020/02/24 05:24:25, lemzwerg wrote:
> Please add one or more test cases for your 'french-correction'
property.

My 'french-correction' is an intrinsic part of the new French Beaming
implementation.  So I think the two existing regression files

* beam-french.ly and
* tuplet-number-french-kneed-beams.ly

already handle French beaming quite well and I've even added a few
problematic cases to beam-French.ly.  In the reg diffs (inserted by
James, cf. Issue 5788),  the most important changes can be clearly seen
in both files, even wit the existing examples.

Apart from that, probably hardly anybody will ever use French beaming,
and the most important regression test by far is all the non-French
cases that must not change by my intervention.

What specific French beaming examples are you missing?

https://codereview.appspot.com/557500043/diff/551490044/lily/include/beam.hh
File lily/include/beam.hh (right):

https://codereview.appspot.com/557500043/diff/551490044/lily/include/beam.hh#newcode36
lily/include/beam.hh:36: struct Beam_stem_length
On 2020/02/24 06:44:39, hanwenn wrote:
> Beam_stem_end ?

Agreed. Done.

https://codereview.appspot.com/557500043/



Re: Issue 5788: New French Beamimg Approach (issue 557500043 by torsten.haemme...@web.de)

2020-02-25 Thread torsten . haemmerle
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
"stem-end-shorten", for the following reasons:

The term "end" does not carry any information, because stems can only be
shortened at their end, as their starting point is nailed to a notehead.
;)
There already are stem-shortenish properties in standard stem processing
(beamed-stem-shorten and stem-shorten).

Most importantly, the french-correction property is different from all
the other shortenish propertie in so far as all of them will heavily
influence the layout (affecting overall positioning), but
"french-correction" is the only property that will shorten the stem for
printing only, leaving all positioning aspects untouched.

Therefore, I'll very much like to have it called something with "french"
in it to emphasize the special role it plays in French beaming (and
really *nowhere* else).

Would "french-shorten" be a viable compromise?

Thanks for looking into this,
Torsten

https://codereview.appspot.com/557500043/



Issue 5788: New French Beamimg Approach (issue 557500043 by torsten.haemme...@web.de)

2020-02-23 Thread torsten . haemmerle
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.

==
BASIC IDEA
==

Hypothesis: The *only* difference between standard and French beaming
is that French "inner group" stems will not pass through
all the beams.
THAT'S ALL! Nothing else about it!

The current approach to generally shorten French stems from the very
beginning causes many follow-up positioning problems that have to
be remedied later-on in many different places by neutralizing this
deviation somehow.
Tuplet numbers (w/o tuplet brackets) have already been dealt with, but
many other problems still remain.

GENERAL SOLUTION (in short)

- Junk all exceptions and do not distinguish between French and standard
  beaming at all (quite radical, but extremely helpful)
- That way we can be sure that all positionings will exactly match
  the standard beaming case
- Only at the very end, when it comes to actually printing the stem, it
  has to be shortened by the appropriate amount.

GENERAL SOLUTION (more detailed)

- Junk all (!) existing French beaming special treatment, using standard
  stem lenghts throughout for all calculations so that everything
exactly
  matches the standard beaming case automatically.
- The standard beam length is extremely important for all the
positioning
  calculations in several ways:
  * Special beam properties as all kinds of minimum lengths will only
work
correctly for standard length beams and there's no need at all for
introducing more and more exceptions for French beams
  * Stem ends are crucial for placement of all kinds of grobs such as
articulations, fingerings, text scripts and they *must* behave
as if all the stems had standard length (property unchanged!) for
vertical positioning of grobs aligned to the stem ends.
  * ONLY when it comes to actually printing the stem, French shortening
becomes relevant.
So *only* the ly:stem::pring function will actually need to know
about the new french-correction property, using (standard) "length"
property minus new "french-correction" property.

=
MODIFICATIONS
=

modified:   Documentation/changes.tely

Mention that French beaming will now exactly behave like standard
beaming in all beam positionings and all articulation, fingering,
etc. placements will now be identical to the standard beaming case.
Show an example comparing standard/French beaming (w/o displaying
the coding, though.  It's just about demonstrating the (now) exactly
matching positioning.

modified:   input/regression/beam-french.ly

(existing) regression file. One beam positioning flaw (too far away
from the noteheads) can already be seen in the current test cases,
but I've added a few more examples containing exceptionally critical
cases.

modified:   scm/define-grob-properties.scm

Introduce new property "french-correction"

modified:   lily/include/stem.hh

Additional Real parameter for set_stem_positions to pass over
french_correction in order to be able to leave standard stem length
untouched and set both property "length" and "french-correction".

modified:   lily/stem.cc

- Stem::set_stem_positions
  * additional parameter Real fc for french_correction value
  * Set Stem property "length" as before, but now containing the
unaltered (i.e. "unfrenched") stem length
  * if (and only if) fc is non-zero, set "french-correction" property
of the Stem
- Callback Stem::print
  * retrieve the (now unfrenched) stem-length property
  * retrieve the (new) french-correction property (0 if non-existent)
  * subtract french_correction from the standard stem_length
(for printing the correctly shortened stem)
- Add new property "french-correction" to the interface

modified:   lily/include/beam.hh

- New struct Beam_stem_length
  containing stem_y_ (as before, but unaltered by French beaming)
  plus a french_correction_ containing the Frech beaming stem length
  delta.
- Function Beam::calc_stem_y adaped
  * return Beam_stem_length rather than a mere Real stem_y
  * Boolean parameter "french" replaced by int value french_count
for passing over the individual number of beam translations the
stem has to be shortened by (if applicable;
0 for standard beaming or standard-length outer group French beams).

modified:   lily/beam.cc

- Junk (now) obsolete French beaming special treatment
- Initialize new struct Beam_stem_length
- Adapt Beam::calc_stem_y
  * return new struct Beam_stem_length
  * replace bool french by int french_count
  * in case of non-zero french_count, calculate french_correction
respecting beam_translation and feather_factor
  * pass over (now unaltered!) stem_y + id as 

Re: Issue 5773: Quarter Tones for all Languages incl. MusicXML import (issue 577520043 by torsten.haemme...@web.de)

2020-02-23 Thread torsten . haemmerle
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
> python/musicexp.py:365: "nederlands": pitch_nederlands,
> You are sorting the entries alphabetically except for Dutch.  Is there
a reason
> for this?

Yes, there are several reasons:
* Dutch is LilyPond's mother tongue and thus the default language. 
* This is the sorting order found in define-note-names.scm
* This is the sorting order found in Documentation, too (cf.
pitches.itely)
That's why I put nederlands in the first place.
Off-Topic: This is like in traditional German university curricula:
Theology first, and only after that: all others in alphabetical order.
;)

> 
>
https://codereview.appspot.com/577520043/diff/555310043/scm/define-note-names.scm
> File scm/define-note-names.scm (right):
> 
>
https://codereview.appspot.com/577520043/diff/555310043/scm/define-note-names.scm#newcode1416
> scm/define-note-names.scm:1416: '((català catalan)(español
espanol)(português
> portugues)))
> I would use spaces between ')('.
You're definitely right.
As it is a mere cosmetic change without any technical impact, I've
updated my local branch
accordingly, but will refrain from uploading a new patch here.

Cheers,
Torsten


Description:
Issue 5773: Quarter Tones for all Languages incl. MusicXML import

modified:   scm/define-note-names.scm
- Missing quarter-tone definitions added, now all languages are
complete.
- Slightly changed sort order of norsk so that the commonly used forms
  (the ones with doulbe-s) are in front of the less common forms.
- Proper language names consistently introduced (català and português
added)

modified:   python/musicexp.py
- Complete set of language-specific pitch conversions.
- Order of lanuages corresponding to the "standard" LilyPond order.

modified:   scripts/musicxml2ly.py
- Map input parameter -l (language) values to UTF-8 language names.

modified:   Documentation/changes.tely
- Announce new language names
- Announce complete set of quarter-tone definitions

modified:   Documentation/ca/notation/pitches.itely
modified:   Documentation/de/notation/pitches.itely
modified:   Documentation/es/notation/pitches.itely
modified:   Documentation/fr/notation/pitches.itely
modified:   Documentation/it/notation/pitches.itely
modified:   Documentation/ja/notation/pitches.itely
modified:   Documentation/notation/pitches.itely

Notation
1.1.1 Writing pitches
Note names in other languages
- Tables unified and completed in all languages
- Remarks about missing quarter-tone definitions removed

Please review this at https://codereview.appspot.com/577520043/

Affected files (+730, -381 lines):
  M Documentation/ca/notation/pitches.itely
  M Documentation/changes.tely
  M Documentation/de/notation/pitches.itely
  M Documentation/es/notation/pitches.itely
  M Documentation/fr/notation/pitches.itely
  M Documentation/it/notation/pitches.itely
  M Documentation/ja/notation/pitches.itely
  M Documentation/notation/pitches.itely
  M python/musicexp.py
  M scm/define-note-names.scm
  M scripts/musicxml2ly.py





Re: musicxml2ly: portugues notenames and quarternotes in español (issue 571630043 by d...@gnu.org)

2020-02-15 Thread torsten . haemmerle
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 in the patch, but if people think it should stay
as it is, I'll remove the new names.

Thanks,
Torsten


https://codereview.appspot.com/571630043/



Re: musicxml2ly: portugues notenames and quarternotes in español (issue 571630043 by d...@gnu.org)

2020-02-15 Thread torsten . haemmerle
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 something like
> 
>   # this map should be synchronized with scm/define-note-names.scm

Hello,

I'd propose to leave it as it is for this patch.
But apart from that, I also dislike LilyPond's inconsistent language
naming conventions.

I've prepared a patch finally introducing full quarter tone support for
all languages and this also includes a complete set of fully tested
MusicMXL pitch import rules. And it's about proper language spelling
introducing català and português.

I've dropped a mail in the dev list asking for opinions about consistent
language naming conventions.
In any case, I'll wait with my patch until this one has been pushed.


All the best,
Torsten

https://codereview.appspot.com/571630043/



Re: Fix and align musicxml and input language "deutsch" (issue 547610043 by d...@gnu.org)

2020-02-12 Thread torsten . haemmerle
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/



Re: Fix and align musicxml and input language "deutsch" (issue 547610043 by d...@gnu.org)

2020-02-12 Thread torsten . haemmerle
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 20:39:14, Be-3 wrote:
> > Proposal:
> > keep 1st character and replace 'fq' by 'tq' in the suffix:
> > 
> >   return str[:1] + str[1:].replace ('fq', 'tq')
> 
> Your proposal does not work for sqs.  Did an independent fix, as I
have not seen
> your proposal in time.  Please test since this should also still make
it into
> 2.20.

Yep, I forgot about the quarter tone sharps.

I've now successfully tested your English solution with a complete set
of all notes ranging from c to b using all accidentals in quarter tone
steps from double-flat to double-sharp.

Having a quick look at the other languages, espanol import is still
completely missing out quarter tones, even if there are Spanish quarter
note names. And portugues is completely missing.


def pitch_espanol (pitch):
str = pitch_generic (pitch, ['do', 're', 'mi', 'fa', 'sol', 'la',
'si'], ['b', 'cb', 'cs', 's'])
return str.replace ('bc', 'tc').replace ('sc', 'tc')

and 

def pitch_portugues (pitch):
str = pitch_generic (pitch, ['do', 're', 'mi', 'fa', 'sol', 'la',
'si'], ['b', 'bqt', 'sqt', 's'])
return str.replace ('bbqt', 'btqt').replace ('ssqt', 'stqt')

would do the trick, tested with the complete set.

Torsten


https://codereview.appspot.com/547610043/



Re: Fix and align musicxml and input language "deutsch" (issue 547610043 by d...@gnu.org)

2020-02-11 Thread torsten . haemmerle
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):

https://codereview.appspot.com/547610043/diff/557380043/python/musicexp.py#newcode324
python/musicexp.py:324: return str
Proposal:
keep 1st character and replace 'fq' by 'tq' in the suffix:

  return str[:1] + str[1:].replace ('fq', 'tq')

https://codereview.appspot.com/547610043/



Re: Fix and align musicxml and input language "deutsch" (issue 547610043 by d...@gnu.org)

2020-02-11 Thread torsten . haemmerle
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):

https://codereview.appspot.com/547610043/diff/557380043/python/musicexp.py#newcode324
python/musicexp.py:324: return str
does not work for quartertones yet: THREE-Q-FLAT will come out as -fqf,
but it should be -tqf (like "three quarter flat").

fqf, (F SEMI-FLAT), however, must remain fqf, etc.
So, a substring-recplacement of fqf will not generally work.
It's a mess...

The same holds true for -fqs (THREE-Q-SHARP).

https://codereview.appspot.com/547610043/



issue 3692: Fingering collision with accidentals (issue 341470043 by torsten.haemme...@web.de)

2018-09-08 Thread torsten . haemmerle

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:   ../input/regression/fingering-adjacent-note-chord-new.ly
new file:   ../input/regression/fingering-adjacent-note-chord.ly
  - Adjacent-note chord regression tests for New_fingering_engraver
and Fingering_engraver.

modified:   ../lily/fingering-engraver.cc
  - Instead of aligning the fingering on the first-to-come notehead of
the
chord, now use note-column as parent and take advantage of the
side-position-interface's X-align-on-main-noteheads functionality.
  - The (sometimes) incorrect alignment caused by simply using the the
first chord note entered as parent was, after all, the actual reason
for the accidental collision that never had happened with a proper
fingering alignment.

modified:   ../lily/new-fingering-engraver.cc
  - Making New_fingering_engraver consider the X-align-on-main-noteheads
property and use note-column as parent for up/down orientations.
That
way, up/down fingerings will be aligned in a straight column,
properly
centered over the main noteheads.
  - With "classic" per-note alignment, use  notehead as parent (as it
had
been always done), but now avoid all accidentals in the chord for
up/down placement.
  - If there's only one note, no special alignment/accidental treatment
needed.
  - accidentals_.clear () had been missing after position_all (), so
that
more and more accidentals gathered up from chord to chord.
  - By default, X-align-on-main-noteheads is *not* set to keep
everything
compatible to the previous practice.
  - When setting X-align-on-main-noteheads and using just up/down
orientation,
New_fingering_engraver behaves like Fingering_engraver.

Please review this at https://codereview.appspot.com/341470043/

Affected files (+116, -6 lines):
  M Documentation/changes.tely
  A input/regression/fingering-adjacent-note-chord.ly
  A input/regression/fingering-adjacent-note-chord-new.ly
  M lily/fingering-engraver.cc
  M lily/new-fingering-engraver.cc



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


issue 5413: X-aligning problem with chords containing unisons (issue 369840043 by torsten.haemme...@web.de)

2018-09-05 Thread torsten . haemmerle

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 FIRST unison note (punctum saliens!)
- highest note: return LAST unison note
Separate DOWN/UP processing now (because of < vs. >=).
Result: consistent spacing plus correct first/last/main notehead
detection.

new file:   ../input/regression/chord-X-align-on-main-noteheads.ly
- Regression test added

Please review this at https://codereview.appspot.com/369840043/

Affected files (+24, -7 lines):
  A input/regression/chord-X-align-on-main-noteheads.ly
  M lily/stem.cc


Index: input/regression/chord-X-align-on-main-noteheads.ly
diff --git a/input/regression/chord-X-align-on-main-noteheads.ly  
b/input/regression/chord-X-align-on-main-noteheads.ly

new file mode 100644
index  
..a3fab79f70fb07425e16eb07147437b3b7a35319

--- /dev/null
+++ b/input/regression/chord-X-align-on-main-noteheads.ly
@@ -0,0 +1,14 @@
+\version "2.21.0"
+
+\header {
+  texidoc = "Chords containing unisons or seconds:
+Center articulation marks, dynamics, slurs, etc. on the notehead that
+is on the “correct” side of the stem."
+}
+
+{
+  e''4-^ -^\p -^\f -^
+  (\< )\! \> ~ \!
+  f'-^ -^\p -^\f -^
+  (\< )\! \> ~ \!
+}
Index: lily/stem.cc
diff --git a/lily/stem.cc b/lily/stem.cc
index  
947a429c23f472770474dd10c9fb8abb8a4053e6..239b15b07fd415a2b0e6c6d55d971df8adc8580f  
100644

--- a/lily/stem.cc
+++ b/lily/stem.cc
@@ -220,20 +220,23 @@ Stem::extremal_heads (Grob *me)
   Drul_array exthead (0, 0);
   extract_grob_set (me, "note-heads", heads);

-  for (vsize i = heads.size (); i--;)
+  for (vsize i = 0; i < heads.size (); i++)
 {
   Grob *n = heads[i];
   int p = Staff_symbol_referencer::get_rounded_position (n);

-  for (LEFT_and_RIGHT (d))
+  if (p < extpos[DOWN])   /* < lowest note unison: take FIRST one */
 {
-  if (d * p > d * extpos[d])
-{
-  exthead[d] = n;
-  extpos[d] = p;
-}
+  exthead[DOWN] = n;
+  extpos[DOWN] = p;
+}
+  if (p >= extpos[UP])/* >= highest note unison: take LAST one */
+{
+  exthead[UP] = n;
+  extpos[UP] = p;
 }
 }
+
   return exthead;
 }



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


issue 5393: Another fingering vs. accidental problem (issue 346920043 by torsten.haemme...@web.de)

2018-08-30 Thread torsten . haemmerle

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 critical collision cases.

modified:   ../lily/new-fingering-engraver.cc
- dots of all noteheads considered to avoid collisionts with fingerings
- Y-offset callback had been too late for correct X positioning:
  (A) Y-center stencil for FingeringColumn
  ---or---
  (B) Setting Y-offset (considering Y-offset set by user plus
self-alignment-Y and parent-alignment-Y)
  This is compatible to current behaviour (except for being in time)
- add_string function removed (obsolete, never used)

Please review this at https://codereview.appspot.com/346920043/

Affected files (+28, -6 lines):
  M input/regression/finger-chords-accidental.ly
  M input/regression/finger-chords-dot.ly
  M lily/new-fingering-engraver.cc


Index: input/regression/finger-chords-accidental.ly
diff --git a/input/regression/finger-chords-accidental.ly  
b/input/regression/finger-chords-accidental.ly
index  
c9d3353e6405058ef7761b04735115570f3c898c..d048951999f42f4f250163bc9bdc1741bb8edaf3  
100644

--- a/input/regression/finger-chords-accidental.ly
+++ b/input/regression/finger-chords-accidental.ly
@@ -12,5 +12,11 @@
   r4
   \set fingeringOrientations = #'(left)
   
-  
+   r4
+   
+   
+   
+   
+   
+   
 }
Index: input/regression/finger-chords-dot.ly
diff --git a/input/regression/finger-chords-dot.ly  
b/input/regression/finger-chords-dot.ly
index  
06cbfa0e75f8780269d9469e0218f887ed43a7a2..1275316bf3e8561c1734e914a4dd276721e2c906  
100644

--- a/input/regression/finger-chords-dot.ly
+++ b/input/regression/finger-chords-dot.ly
@@ -7,4 +7,5 @@
 \relative {
   \set fingeringOrientations = #'(right)
   4.. 4. r8.
+  4. r8  4. r8
 }
Index: lily/new-fingering-engraver.cc
diff --git a/lily/new-fingering-engraver.cc b/lily/new-fingering-engraver.cc
index  
06a3a8da969d7ab2d95672f167158887fe1b..1e82ed701c79c83b65df49662a0e53164c4aac59  
100644

--- a/lily/new-fingering-engraver.cc
+++ b/lily/new-fingering-engraver.cc
@@ -78,7 +78,6 @@ protected:
   vector *,
   Stream_event *, Stream_event *);
   void add_script (Grob *, Stream_event *, Stream_event *);
-  void add_string (Grob *, Stream_event *, Stream_event *);
   void position_scripts (SCM orientations, vector *);
 };

@@ -282,11 +281,27 @@ New_fingering_engraver::position_scripts (SCM  
orientations,

   && unsmob (ft.head_->get_object ("accidental-grob")))
 Side_position_interface::add_support (f,
   unsmob  
(ft.head_->get_object ("accidental-grob")));

-  else if (unsmob (ft.head_->get_object ("dot")))
-Side_position_interface::add_support (f,
-  unsmob  
(ft.head_->get_object ("dot")));

+  else if (Rhythmic_head::dot_count (ft.head_))
+for (vsize j = 0; j < heads_.size (); j++)
+   if (Grob *d = unsmob (heads_[j]->get_object ("dot")))
+ Side_position_interface::add_support (f, d);
+
+  if (horiz.size () > 1)  /* -> FingeringColumn */
+{
+  Stencil *fs = f->get_stencil ();
+  fs->align_to (Y_AXIS, CENTER);
+}
+  else
+{
+  SCM self_align_y =
+Self_alignment_interface::aligned_on_parent (f, Y_AXIS);
+  SCM yoff= f->get_property ("Y-offset");
+  if (scm_is_number (yoff))
+self_align_y = scm_from_double (scm_to_double (self_align_y)
+  + scm_to_double (yoff));
+  f->set_property ("Y-offset", self_align_y);
+}

-  Self_alignment_interface::set_aligned_on_parent (f, Y_AXIS);
   Side_position_interface::set_axis (f, X_AXIS);

   f->set_property ("direction", scm_from_int (hordir));



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Issue 5324: \abs-fontsize and set-global-staff-size in books (issue 341450043 by torsten.haemme...@web.de)

2018-08-13 Thread torsten . haemmerle

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.
That's why I used #ly:reset-all-fonts for #set-global-staff-size only.

But I'll definitely check the issues you mentioned.

Cheerio,
Torsten

https://codereview.appspot.com/341450043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Issue 5324: \abs-fontsize and set-global-staff-size in books (issue 341450043 by torsten.haemme...@web.de)

2018-08-13 Thread torsten . haemmerle

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):


https://codereview.appspot.com/341450043/diff/1/input/regression/book-change-global-staffsize-abs-fonts.ly#newcode6

input/regression/book-change-global-staffsize-abs-fonts.ly:6:
@code{\\abs-fontsize}.
A single-word sentence looks strange...


Right!
(That's a single-word sentence that definitely makes more sense than
"\abs-fontsize."
I'll see to it, thanks.

Apart from that: I just wonder if there's another possibility to change
output_scale other than by #set-global-staff-size.  If so, as DAK
implied, we'd better go for a more comprehensive solution.

Thanks,
Torsten

https://codereview.appspot.com/341450043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Issue 5324: \abs-fontsize and set-global-staff-size in books (issue 341450043 by torsten.haemme...@web.de)

2018-08-13 Thread torsten . haemmerle

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 *only* problem this solves is a "successful" try_retrieve that
delivers the wrong (outdated) scale_ factor after
#(set-global-staff-size …).

The other solution I proposed (checking output_scale each time a font is
retrieved from the buffer and eventually re-calculating the scale_
value) would be safer in so far as it ensures the correct scale_ factor
no matter how or why output_scale has changed (not just by
set-global-staff-size).
Would that be of any help? I could provide an alternative patch here.

It'd be good to check the "other fontsize mismatch problems", too, in
any case.

Thanks,
Torsten


Description:
Issue 5324: \abs-fontsize and set-global-staff-size in books

The font buffering hashtable will provide an outdated LilyPond
scale factor after changing global staff size between books. Mainly
\abs-fontsize concerned, because \fontsize will usually not match
a Pango font-size used in a differently scaled book and fresh fonts
will have to be loaded anyway.

The straight-forward solution is to simply apply #ly:restet-all-fonts
from within #set-global-staff-size.

Please review this at https://codereview.appspot.com/341450043/

Affected files (+35, -0 lines):
  A input/regression/book-change-global-staffsize-abs-fonts.ly
  M scm/paper.scm


Index: input/regression/book-change-global-staffsize-abs-fonts.ly
diff --git a/input/regression/book-change-global-staffsize-abs-fonts.ly  
b/input/regression/book-change-global-staffsize-abs-fonts.ly

new file mode 100644
index  
..888b71f4fe96c8f2171b62bf08359bac46f98ecb

--- /dev/null
+++ b/input/regression/book-change-global-staffsize-abs-fonts.ly
@@ -0,0 +1,34 @@
+\version "2.21.0"
+
+\header {
+  texidoc = "Changing @code{global-staff-size} between consecutive
+ @code{\\book}s must not impair font spacing.
+ @code{\\abs-fontsize}.
+ The following output shows a 10pt book after a standard
+ 20pt book:"
+}
+\paper { indent = 0 left-margin = 10 line-width = 75 }
+\header {
+  title = \markup \abs-fontsize #10 "Changing global staff size"
+  subtitle = \markup \abs-fontsize #10 "from 20pt to 10pt in the 2nd book"
+  tagline = ##f
+}
+testMusic = {
+  c'4_\markup \abs-fontsize #10 "\abs-fontsize #10 text"
+ _\markup \abs-fontsize #10 \line { "\abs-fontsize #10
+\dynamic" \dynamic "fff" }
+ ^\markup \fontsize #0 "\fontsize #0"
+ ^\markup \fontsize #6 "\fontsize #6"
+  d' e' f'
+}
+
+
+#(set-global-staff-size 20)
+#(define output-suffix "1")
+\book { \score { \testMusic } }
+
+% This book will be shown in the regression tests/collated files:
+#(set-global-staff-size 10)
+#(define output-suffix #f)
+\book { \score { \testMusic } }
+
Index: scm/paper.scm
diff --git a/scm/paper.scm b/scm/paper.scm
index  
23ec05a70f25715225123160ae71a352d41d6140..029e9c401f6be2da18c0c6ba56a35acb54d04457  
100644

--- a/scm/paper.scm
+++ b/scm/paper.scm
@@ -120,6 +120,7 @@
 (if in-layout?
 (ly:warning (_ "set-global-staff-size: not in toplevel scope")))

+(ly:reset-all-fonts)
 (layout-set-absolute-staff-size-in-module new-scope
   (* sz (eval 'pt new-scope)))
 (module-define! current-mod '$defaultpaper new-paper)))


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Add and use a Transform data type (issue 344970043 by d...@gnu.org)

2018-06-17 Thread torsten . haemmerle



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), robust_scm2double (scm_caddr (rot), 0.0));
+  center[X_AXIS] = s->extent (X_AXIS).linear_combination
(center[X_AXIS]);
+  center[Y_AXIS] = s->extent (Y_AXIS).linear_combination
(center[Y_AXIS]);
(or something similar)

Here, we've got the rotation centre problem as in issue 5346.

Standard regtests can't see it, but PNG based regtests show differences
for
skyline-grob-rotation.ly and
stencil-color-rotation.ly

skyline-boxes-ellipses.ly shows up, too, but there are only minimal
deviations due to rounding issues.

https://codereview.appspot.com/344970043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Fix bad rotation center from issue 5346 (issue 341380043 by d...@gnu.org)

2018-06-16 Thread torsten . haemmerle

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://lists.gnu.org/mailman/listinfo/lilypond-devel


Fix GC issue in skyline rotations (issue 344960043 by d...@gnu.org)

2018-06-16 Thread torsten . haemmerle

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/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: issue #5341: Urgent corrections to stencil-integral.cc (skylines) (issue 341350043 by torsten.haemme...@web.de)

2018-06-12 Thread torsten . haemmerle

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 problem, shouldn't we?



I'm afraid I'll have to take a much more thorough look at this shit

show and

rework its data structures.


That'd be great…



I suggest you remove this change (which would not
even help due to the lack of scm_remember_upto_here_1) from this patch

set,

retaining the cosmetic parts, and I'll try coming up with something

that has a

chance of working regarding the GC parts.


Do you mean "remove the grob rotation coding" (Stencil q = *s;).

The cosmetic part being just < and <= etc...?


Is there a simple (minimal example way) to provoke the garbage
collection problem,
because in my test cases, it (accidentally) worked.

https://codereview.appspot.com/341350043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


issue #5341: Urgent corrections to stencil-integral.cc (skylines) (issue 341350043 by torsten.haemme...@web.de)

2018-06-11 Thread torsten . haemmerle

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 new_s.

Minor changes:
make_round_filled_boxes
line 393: increasing step width in order to skip unneccessary straight
lines.
line 432: replacing a < by a clearer <=

Please review this at https://codereview.appspot.com/341350043/

Affected files (+4, -3 lines):
  M lily/stencil-integral.cc


Index: lily/stencil-integral.cc
diff --git a/lily/stencil-integral.cc b/lily/stencil-integral.cc
index  
bc2bc2b41d363166d77b5bca97d67090f1bbb9b4..7af58d00d3004107887de7f808b42adab957c4bf  
100644

--- a/lily/stencil-integral.cc
+++ b/lily/stencil-integral.cc
@@ -390,7 +390,7 @@ make_round_filled_box_boxes (vector ,
   points.push_back (Offset (-left + radius, top));
   points.push_back (Offset (right - radius, top));

-  for (vsize i = 0; i < (vsize) points.size () - 1; i++)
+  for (vsize i = 0; i < (vsize) points.size () - 1; i+=2)
 {
   Offset p0 = points[i];
   Offset p1 = points[i+1];
@@ -420,7 +420,7 @@ make_round_filled_box_boxes (vector ,
   cy[DOWN]  = -bottom + radius;
   cy[UP]= top - radius;

-  for (vsize i = 0; i < (vsize) quantization + 1; i++)
+  for (vsize i = 0; i <= (vsize) quantization; i++)
 for (DOWN_and_UP(v))
   for (LEFT_and_RIGHT(h))
 {
@@ -1129,7 +1129,8 @@ Stencil::skylines_from_stencil (SCM sten, Real pad,  
SCM rot, Axis a)

   Real x = robust_scm2double (scm_cadr (rot), 0.0);
   Real y = robust_scm2double (scm_caddr (rot), 0.0);
   /* pass over a rotated copy of the stencil */
-  Stencil *q = unsmob (s->smobbed_copy ());
+  SCM new_s = s->smobbed_copy ();
+  Stencil *q = unsmob (new_s);
   q->rotate_degrees (angle, Offset (x, y));
   data = stencil_traverser
(make_transform_matrix (1.0, 0.0, 0.0, 1.0, 0.0, 0.0),



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: issue 5319: Make skylines reflect grob rotation (issue 342060043 by torsten.haemme...@web.de)

2018-05-03 Thread torsten . haemmerle

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

lily/key-signature-interface.cc:114: padding += (intersection

(ht_right,

last_ht_left).length ()
The outermost parentheses on the right side are redundant.


Hi Werner,

the key-signature-interface.cc slipped in only accidentally and does not
belong to this issue. It has disappeared from 2nd patch set. There was
also a mismatch in changes.tely and James aked me to make a new patch
against current master.
Apart from that, yes, the parentheses are redundant, but they were also
there in the original coding and so I kept them...

All the best,
Torsten

https://codereview.appspot.com/342060043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


issue 5319: Make skylines reflect grob rotation (issue 342060043 by torsten.haemme...@web.de)

2018-05-03 Thread torsten . haemmerle

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 grob rotation

modified:   ../lily/include/stencil.hh
Add parameter to skylines_from_stencil for rot (in analogy to pad)

modified:   ../lily/stencil-integral.cc

Stencil::skylines_from_stencil
If new rotation parameter rot is specified, use a rotated copy of
the original stencil to build the skyline transformation matrix.
As the rotation is rarely used, the general (non-rotated) case will
still use the original stencil as before.

Grob::vertical_sklyines_from_grob and
Grob::horizontal_skylines_from_grob
Read "rotation" property an pass it (SCM) to skylines_from_stencil.

modified:   ../lily/accidental.cc

Accidental_interface::horizontal_skylines
Read "rotation" property and pass it (SCM) to skylines_from_stencil.

Announced in ../Documentation/changes.tely

Documentation example slightly adapted (20° to 15°) to new skyline
behaviour (padding will now be respected). In all affected languages:
modified:   ../Documentation/de/notation/changing-defaults.itely
modified:   ../Documentation/es/notation/changing-defaults.itely
modified:   ../Documentation/fr/notation/changing-defaults.itely
modified:   ../Documentation/it/notation/changing-defaults.itely
modified:   ../Documentation/ja/notation/changing-defaults.itely
modified:   ../Documentation/notation/changing-defaults.itely

modified:   ../input/regression/skyline-color-rotation.ly
(20° to 15°, cf. documentation)

new:   ../input/regression/skyline-grob-rotation.ly
New regression test.



Please review this at https://codereview.appspot.com/342060043/

Affected files (+109, -25 lines):
  M Documentation/changes.tely
  M Documentation/de/notation/changing-defaults.itely
  M Documentation/es/notation/changing-defaults.itely
  M Documentation/fr/notation/changing-defaults.itely
  M Documentation/it/notation/changing-defaults.itely
  M Documentation/ja/notation/changing-defaults.itely
  M Documentation/notation/changing-defaults.itely
  A input/regression/skyline-grob-rotation.ly
  M input/regression/stencil-color-rotation.ly
  M lily/accidental.cc
  M lily/include/stencil.hh
  M lily/key-signature-interface.cc
  M lily/stencil-integral.cc


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Clarify notation for slurs and beams (issue 343060043 by carl.d.soren...@gmail.com)

2018-04-30 Thread torsten . haemmerle

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


https://codereview.appspot.com/343060043/diff/1/Documentation/learning/fundamental.itely
File Documentation/learning/fundamental.itely (right):

https://codereview.appspot.com/343060043/diff/1/Documentation/learning/fundamental.itely#newcode487
Documentation/learning/fundamental.itely:487: \absolute
{c'8-1--(~^\markup{"text annotationi"} c' d')}
Shouldn't it be "text annotation"?

https://codereview.appspot.com/343060043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: issue 5312: Key cancellation glyph position inconsistent (issue 343020043 by torsten.haemme...@web.de)

2018-04-24 Thread torsten . haemmerle

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

square/parallelogram

> part of a natural glyph will be one stave-space high independent of

the font

> used, just like a notehead in *any* font will be one stave-space

high.


So it's a reasonable approximation that the box part of the natural

will extend

about 3/4 staff spaces above the staff position of the natural, and

the

descender
will  descend about 1.5 staff spaces below.



Seems like a reasonable estimate to me, even if it's not exact for a

particular

font.
You can't get too far away from that and still fit in a staff.



Thanks,



Carl


Agreed, Carl,

And, IMHO, there is no need for exaggerated exactness: as soon as two
corners get too close (no matter how close exactly), we need a wee bit
of extra padding.
The main reason for the 3/4 staff spaces, to be honest, was that the two

intervals will be just touching (with intersection length 0) for the
corner-to-corner constellation.

All the best,
Torsten

https://codereview.appspot.com/343020043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: issue 5312: Key cancellation glyph position inconsistent (issue 343020043 by torsten.haemme...@web.de)

2018-04-24 Thread torsten . haemmerle

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 FIXME in recognizing this fact.  Or

maybe we

just go as-is.


Hi Carl,

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
square/parallelogram
part of a natural glyph will be one stave-space high independent of the
font
used, just like a notehead in *any* font will be one stave-space high.

Therefore, it will also work for Gonville, Bravura, all of Abraham's
fonts...
Funny enough, it works for the LilyJAZZ font, too, because even in this
pseudo
handwritten font the square/parallelogram part has the same (standard)
height.

All the best,
Torsten

https://codereview.appspot.com/343020043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


issue 5312: Key cancellation glyph position inconsistent (issue 343020043 by torsten.haemme...@web.de)

2018-04-24 Thread torsten . haemmerle

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 representing the "vertical skylines" of two
adjacent natural glyphs. This makes a better model of the actual
glyph shape than the original single point within an inverval.
Now we can distiguish between all three cases:
(1) no overlap -> no additional kerning needed
(2) just touching (overlap, but intesection length 0) -> kerning 0.15
(3) overlapping -> kerning 0.3

Case (2) is the new one that couldn't be handled before:
Just touching at the corners made the glyphs cling together.

Basically same  interval technique as before, but I
renamed the Slices from
  pos and overlap_pos
to
  ht_right and last_ht_left.
Using quarter stave-spaces as to get a better (integer) resolution.

Please review this at https://codereview.appspot.com/343020043/

Affected files (+20, -8 lines):
  M Documentation/changes.tely
  M lily/key-signature-interface.cc


Index: Documentation/changes.tely
diff --git a/Documentation/changes.tely b/Documentation/changes.tely
index  
96748ad84dd8f2d91ae058f6545d1697c793b9c9..defcd4f1daabe11cc5855497e51137290de6297e  
100644

--- a/Documentation/changes.tely
+++ b/Documentation/changes.tely
@@ -62,6 +62,16 @@ which scares away people.
 @end ignore

 @item
+Slight padding added between natural glyphs just touching at the corners
+in key cancellations.
+@lilypond[verbatim,quote]
+{
+  \omit Staff.TimeSignature
+  \key ces \major s1 \key c \major s
+}
+@end lilypond
+
+@item
 Two new ornaments have been added.
 @lilypond[verbatim,quote]
 {
Index: lily/key-signature-interface.cc
diff --git a/lily/key-signature-interface.cc  
b/lily/key-signature-interface.cc
index  
79e2bd99482d25b8e5eb1fb10e9fb1ce2255cae7..0c4180d194a283799ff62398be04a2dda2893377  
100644

--- a/lily/key-signature-interface.cc
+++ b/lily/key-signature-interface.cc
@@ -58,7 +58,7 @@ Key_signature_interface::print (SCM smob)
 the cancellation signature.
   */

-  Slice pos, overlapping_pos;
+  Slice ht_right, last_ht_left; /* ht intervals for natural glyph kerning  
*/

   SCM last_glyph_name = SCM_BOOL_F;
   SCM padding_pairs = me->get_property ("padding-pairs");

@@ -87,14 +87,15 @@ Key_signature_interface::print (SCM smob)
 me->warning (_ ("alteration not found"));
   else
 {
-  pos.set_empty ();
+  ht_right.set_empty ();
   Stencil column;
   for (SCM pos_list =  
Lily::key_signature_interface_alteration_positions

  (scm_car (s), c0s, smob);
scm_is_pair (pos_list); pos_list = scm_cdr (pos_list))
 {
   int p = scm_to_int (scm_car (pos_list));
-  pos.add_point (p);
+  ht_right.add_point (2*p - 6);  /* descender */
+  ht_right.add_point (2*p + 3);  /* upper right corner */
   column.add_stencil (acc.translated (Offset (0, p * inter)));
 }
   /*
@@ -108,14 +109,15 @@ Key_signature_interface::print (SCM smob)
   padding_pairs);
   if (scm_is_pair (handle))
 padding = robust_scm2double (scm_cdr (handle), 0.0);
-  else if (glyph_name == "accidentals.natural"
-   && !intersection (overlapping_pos, pos).is_empty ())
-padding += 0.3;
+  else if (glyph_name == "accidentals.natural")
+if (!intersection (ht_right, last_ht_left).is_empty ())
+  padding += (intersection (ht_right, last_ht_left).length ()
+  ? 0.3 /* edges overlap */
+  : 0.15);  /* just touching at the corners */

   mol.add_at_edge (X_AXIS, LEFT, column, padding);

-  pos.widen (4);
-  overlapping_pos = pos + 2;
+  last_ht_left = ht_right + 3;  /* shift up (change to left side)  
*/

   last_glyph_name = glyph_name_scm;
 }
 }



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: issue 5307: Skyline Refinements (Rounded Boxes and Rotated Ellipses) (issue 341140043 by torsten.haemme...@web.de)

2018-04-21 Thread torsten . haemmerle

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 here).
But you are right, of course, and I've wrapped some lines in my new
resp. changed coding.


Is `max' really the right operation here?  Or to ask differently: Does

a

negative value of `diameter' makes sense?  All other values on the

right side

are non-negative...



Or maybe the first argument of `max' should be 1, as in function
`make_partial_ellipse_boxes'?

Oh, yes, of course.
That was a relic from earlier attempts and does not make too much sense,
indeed.

I've now taken the quantization declaration in front of the "non-simple
box" coding,
omitted the superfluent max operation and *after* quantization has been
computed,
I set the radius value (to 0., if there's no quantization).
That way, an effective skyline radius of 0 will ensure that the straight
lines meet
in the corners and there is no need to draw "rounded" corners with a
quantization of 1.
In contrast to the ellipses, a box will still have an outline even with
sharp corners.

Thanks,
Torsten


https://codereview.appspot.com/341140043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


issue 5307: Skyline Refinements (Rounded Boxes and Rotated Ellipses) (issue 341140043 by torsten.haemme...@web.de)

2018-04-20 Thread torsten . haemmerle

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 affects calculation of quantisation value

function make_round_filled_box_boxes:

(1) remove unintentional additional space between box and skyline

(2) In case of rounded corners (threshold: diameter > 0.5 staff spaces)
or rotation, a detailed skyline will be constructed.
Rotated straight lines are drawn using buildings.
Rounded corners built out of boxes (cf. ellipse).

(3) Draw simple single-box skyline for simple boxes (i.e. in most cases,
e.g. ledger lines).

Please review this at https://codereview.appspot.com/341140043/

Affected files (+145, -13 lines):
  M Documentation/changes.tely
  A input/regression/skyline-boxes-ellipses.ly
  M lily/stencil-integral.cc



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: add glyphs for 256th, 512th and 1024th flags and rests (issue 336590043 by lilyp...@maltemeyn.de)

2018-02-23 Thread torsten . haemmerle

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 "calculated" values weren't correct (even if
the output was OK).

So I agree on 7,8 → 4, but it has to be 10 → 6 ;)


https://codereview.appspot.com/336590043/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: add glyphs for 256th, 512th and 1024th flags and rests (issue 336590043 by lilyp...@maltemeyn.de)

2018-02-23 Thread torsten . haemmerle

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 new rest glyphs)

My suggestion:
- ((= log 7) 4)
+ ((>= log  7) (- log 3));

That way, there's no need to add a separate line for each duration.

Just writing this as a comment because I'm terribly unsure about the
correct usage of rietveld and I don't want to mess with Malte's coding.

All the best,
Torsten

https://codereview.appspot.com/336590043/

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: issue #1493: Problem with horizontal beams (issue 337560043 by torsten.haemme...@web.de)

2018-02-10 Thread torsten . haemmerle

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/10 13:26:34, Dan Eble wrote:

If I interpret this correctly, you're trying to avoid a later division

when the

divisor is large.  That divisor is (concaveness + damping), so perhaps

this

should  be (concaveness + damping) >= threshold.


Hi Dan,

Thanks for the hint.
But while concaveness and damping play the same role in the damping
formula, they are still two separate things and therefore, I have kept
them separate on purpose.

If both concaveness and damping stay below 1, the beams should not
be forced to be horizontal, even if the *sum* of concaveness and damping
exceeds 1.

Torsten

Description:
issue #1493: Problem with horizontal beams

file: lily/beam-quanting.cc

In spite of infinite damping, due to numeric precision problems
(rounding/adding) the resulting beams arbitrarily still had a
remaining non-zero slope, sometimes even in the "wrong" direction.

In good TeX manner and in line with concaveness (already implemented),
we'll use a threshold value of 1 for forcing horizontal beams.

Please review this at https://codereview.appspot.com/337560043/

Affected files (+1, -1 lines):
  M lily/beam-quanting.cc


Index: lily/beam-quanting.cc
diff --git a/lily/beam-quanting.cc b/lily/beam-quanting.cc
index  
f3505772f1a70940c4deb9014d400b9158e485a2..a50b3d2cde329facb134e8242409860ad5d2eeaa  
100644

--- a/lily/beam-quanting.cc
+++ b/lily/beam-quanting.cc
@@ -714,7 +714,7 @@ Beam_scoring_problem::slope_damping ()
   SCM s = beam_->get_property ("damping");
   Real damping = scm_to_double (s);
   Real concaveness = calc_concaveness ();
-  if (concaveness >= 1)
+  if ((concaveness >= 1) || (damping >= 1))
 {
   unquanted_y_[LEFT] = unquanted_y_[RIGHT];
   musical_dy_ = 0;



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


issue #3653: Beam ends not matched to StaffSymbol thickness (issue 339210043 by torsten.haemme...@web.de)

2018-01-12 Thread torsten . haemmerle

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 in C++ coding: stave line thickness lt accidentally taken from

layout line thickness rather than from acutal StaffSymbol line thickness


Please review this at https://codereview.appspot.com/339210043/

Affected files (+1, -1 lines):
  M lily/beam.cc


Index: lily/beam.cc
diff --git a/lily/beam.cc b/lily/beam.cc
index  
a50f2904bc0b00e4a497c16c908b13cabf38d96b..dff62168af0f10a6c8d95dd4048cab42743b8dfa  
100644

--- a/lily/beam.cc
+++ b/lily/beam.cc
@@ -371,7 +371,7 @@ Beam::calc_beam_segments (SCM smob)
   Real gap_length = robust_scm2double (me->get_property ("gap"), 0.0);

   Position_stem_segments_map stem_segments;
-  Real lt = me->layout ()->get_dimension (ly_symbol2scm  
("line-thickness"));

+  Real lt = Staff_symbol_referencer::line_thickness (me);

   /* There are two concepts of "rank" that are used in the following code.
  The beam_rank is the vertical position of the beam (larger numbers are



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel


Re: Issue 2961: \fret-diagram-verbose with capo 1 (issue 6854106)

2012-12-29 Thread torsten . haemmerle

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


Re: Issue 2961: \fret-diagram-verbose with capo 1 (issue 6854106)

2012-12-03 Thread torsten . haemmerle

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
https://lists.gnu.org/mailman/listinfo/lilypond-devel