Re: Issue 5036: 128 beaming output not producing output as expected (?) (issue 559700043 by torsten.haemme...@web.de)
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)
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)
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)
LGTM. Arigato! https://codereview.appspot.com/551580045/
Re: Fix Windows resource to enable manifest (issue 551580044 by truer...@gmail.com)
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)
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)
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)
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)
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)
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)
@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)
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)
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)
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)
> 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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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)
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