Re: GUILE 2.2 progress
On Sat, Jan 25, 2020 at 8:16 PM David Kastrup wrote: > > $ gs -q -dSAFER -dDEVICEWIDTHPOINTS=595.28 -dDEVICEHEIGHTPOINTS=841.89 > > -dCompatibilityLevel=1.4 -dNOPAUSE -dBATCH -r1200 -sDEVICE=pdfwrite > > -dAutoRotatePages=/None -dPrinted=false -sOutputFile=mozart-hrn-3.pdf > > -c.setpdfwrite -f/tmp/lilypond-OCyOzh > > Error: /rangecheck in /--.parsecff-- > > Operand stack: > >false --nostringval-- > > Pretty sure this is an encoding problem. the question is how this managed to work on GUILE 2.0. We're embedding the OTF/CFF font (which is binary data) into a string without considering any encoding. The string then gets dumped onto the PS file. -- Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
Re: GUILE 2.2 progress
Han-Wen Nienhuys writes: > On Sat, Jan 25, 2020 at 3:50 PM David Kastrup wrote: >> > That patch is papering over a problem rather than fixing it. It might >> > be a reoccurence of something like >> > >> > commit c01a2a757e3c59727bdfa8d77568bf42525fbe05 >> > Author: Andy Wingo >> > Date: Thu Jun 23 11:47:42 2016 +0200 >> > >> > Fix race between SMOB marking and finalization >> > >> > * libguile/smob.c (clear_smobnum): New helper. >> > (finalize_smob): Re-set the smobnum to the "finalized smob" type >> > before finalizing. Fixes #19883. >> > (scm_smob_prehistory): Pre-register a "finalized smob" type, which >> > has >> > no mark procedure. >> > * test-suite/standalone/test-smob-mark-race.c: New file. >> > * test-suite/standalone/Makefile.am: Arrange to build and run the new >> > test. >> >> Actually, you may also be missing >> commit c9910c604279f438728cd268272e1839cbc53835 >> Author: Andy Wingo >> Date: Mon Mar 13 15:47:51 2017 +0100 > > It must have been another one, v2.2.6 doesn't have the problem anymore, but > > [hanwen@localhost guile]$ git describe --contains > c9910c604279f438728cd268272e1839cbc53835 > v2.2.0~11 > [hanwen@localhost guile]$ git describe --contains > c01a2a757e3c59727bdfa8d77568bf42525fbe05 > v2.1.4~111 Ok, sorry. So they managed to get and fix more problems. Basically your patch replaces malloc/free with a version that does not bother complaining about multiple freeing. But I cannot vouch for our destructors being executable multiple times, so that "fix" is really asking for trouble. > I still get > > input/regression/mozart-hrn-3.ly:31:9: error: not a markup > > #(string-append "It has been typeset and placed in the public " Frankly, that one looks like a nightmare that should not happen. Does it say the same for, say, #"just a string" ? And "just a string" (without hash mark)? > and > > $ gs -q -dSAFER -dDEVICEWIDTHPOINTS=595.28 -dDEVICEHEIGHTPOINTS=841.89 > -dCompatibilityLevel=1.4 -dNOPAUSE -dBATCH -r1200 -sDEVICE=pdfwrite > -dAutoRotatePages=/None -dPrinted=false -sOutputFile=mozart-hrn-3.pdf > -c.setpdfwrite -f/tmp/lilypond-OCyOzh > Error: /rangecheck in /--.parsecff-- > Operand stack: >false --nostringval-- Pretty sure this is an encoding problem. > > those look fixable, though. -- David Kastrup
Some py3 fixes (issue 553430047 by hanw...@gmail.com)
LGTM
https://codereview.appspot.com/553430047/diff/573430051/python/lilylib.py
File python/lilylib.py (right):
https://codereview.appspot.com/553430047/diff/573430051/python/lilylib.py#newcode257
python/lilylib.py:257: sys.stderr.write('command failed: %s\n' % cmd)
With space before the '(' or without?
https://codereview.appspot.com/553430047/
Re: GUILE 2.2 progress
On Sat, Jan 25, 2020 at 3:50 PM David Kastrup wrote: > > That patch is papering over a problem rather than fixing it. It might > > be a reoccurence of something like > > > > commit c01a2a757e3c59727bdfa8d77568bf42525fbe05 > > Author: Andy Wingo > > Date: Thu Jun 23 11:47:42 2016 +0200 > > > > Fix race between SMOB marking and finalization > > > > * libguile/smob.c (clear_smobnum): New helper. > > (finalize_smob): Re-set the smobnum to the "finalized smob" type > > before finalizing. Fixes #19883. > > (scm_smob_prehistory): Pre-register a "finalized smob" type, which has > > no mark procedure. > > * test-suite/standalone/test-smob-mark-race.c: New file. > > * test-suite/standalone/Makefile.am: Arrange to build and run the new > > test. > > Actually, you may also be missing > commit c9910c604279f438728cd268272e1839cbc53835 > Author: Andy Wingo > Date: Mon Mar 13 15:47:51 2017 +0100 It must have been another one, v2.2.6 doesn't have the problem anymore, but [hanwen@localhost guile]$ git describe --contains c9910c604279f438728cd268272e1839cbc53835 v2.2.0~11 [hanwen@localhost guile]$ git describe --contains c01a2a757e3c59727bdfa8d77568bf42525fbe05 v2.1.4~111 I still get input/regression/mozart-hrn-3.ly:31:9: error: not a markup #(string-append "It has been typeset and placed in the public " and $ gs -q -dSAFER -dDEVICEWIDTHPOINTS=595.28 -dDEVICEHEIGHTPOINTS=841.89 -dCompatibilityLevel=1.4 -dNOPAUSE -dBATCH -r1200 -sDEVICE=pdfwrite -dAutoRotatePages=/None -dPrinted=false -sOutputFile=mozart-hrn-3.pdf -c.setpdfwrite -f/tmp/lilypond-OCyOzh Error: /rangecheck in /--.parsecff-- Operand stack: false --nostringval-- those look fixable, though. -- Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
Re: Issue XXXX: Dot_configuration maintenance (issue 577380046 by nine.fierce.ball...@gmail.com)
On Sat, Jan 25, 2020 at 7:37 PM wrote: > > On 2020/01/25 18:24:32, hanwenn wrote: > > > https://codereview.appspot.com/577380046/diff/553430046/lily/dot-configuration.cc > > File lily/dot-configuration.cc (right): > > > > > https://codereview.appspot.com/577380046/diff/553430046/lily/dot-configuration.cc#newcode69 > > lily/dot-configuration.cc:69: auto process_entry = [d, k, _cfg, > > ](const value_type ) > > what does this syntax do? > > https://en.cppreference.com/w/cpp/language/lambda > > It defines a function named "process_entry" taking one argument (const > value_type &). The body of the function can access variables d and k by > value, and variables new_cfg and offset by reference. I think "this" > might also be captured implicitly in this context, but I'd have to go to > the documentation to refresh my memory. > > An alternative to using a lambda would be to move this stuff to a > private member function, or maybe a static function in this file. Thanks for the explanation! -- Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
Re: Issue XXXX: Dot_configuration maintenance (issue 577380046 by nine.fierce.ball...@gmail.com)
On 2020/01/25 18:24:32, hanwenn wrote: > https://codereview.appspot.com/577380046/diff/553430046/lily/dot-configuration.cc > File lily/dot-configuration.cc (right): > > https://codereview.appspot.com/577380046/diff/553430046/lily/dot-configuration.cc#newcode69 > lily/dot-configuration.cc:69: auto process_entry = [d, k, _cfg, > ](const value_type ) > what does this syntax do? https://en.cppreference.com/w/cpp/language/lambda It defines a function named "process_entry" taking one argument (const value_type &). The body of the function can access variables d and k by value, and variables new_cfg and offset by reference. I think "this" might also be captured implicitly in this context, but I'd have to go to the documentation to refresh my memory. An alternative to using a lambda would be to move this stuff to a private member function, or maybe a static function in this file. https://codereview.appspot.com/577380046/
python/langdefs: make print statement py3 compatible (issue 573440043 by hanw...@gmail.com)
LGTM https://codereview.appspot.com/573440043/
Re: Issue XXXX: Dot_configuration maintenance (issue 577380046 by nine.fierce.ball...@gmail.com)
https://codereview.appspot.com/577380046/diff/553430046/lily/dot-configuration.cc File lily/dot-configuration.cc (right): https://codereview.appspot.com/577380046/diff/553430046/lily/dot-configuration.cc#newcode69 lily/dot-configuration.cc:69: auto process_entry = [d, k, _cfg, ](const value_type ) what does this syntax do? https://codereview.appspot.com/577380046/
Re: EXPERIMENTAL: put a reminder of the mm rest on the last page at the top. (issue 553400043 by hanw...@gmail.com)
On Sat, Jan 25, 2020 at 3:57 PM wrote: > > I'm not asking for anyone to review. It's context to the other change > about > > multi-measure-rests. > > OIC. An MMR reminder sounds interesting. Does the problem tie in with > others like repeating the current chord name after a page break? > > https://codereview.appspot.com/553400043/ I think you could build that in a similar fashion, yes. -- Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
Re: Issue 5695: reduce dynamic casting (issue 575530107 by nine.fierce.ball...@gmail.com)
https://codereview.appspot.com/575530107/diff/581530050/lily/spaceable-grob.cc File lily/spaceable-grob.cc (right): https://codereview.appspot.com/575530107/diff/581530050/lily/spaceable-grob.cc#newcode92 lily/spaceable-grob.cc:92: break; That looks still too complicated. Just do return *next_spring; here, no need to break out of the loop. Then you don't need the variable spring at all (since it will be 0 if the loop exits regularly) and can hijack its name for the more cumbersome next_spring. And the final return can just be return Spring (); Oh, and the programming_error can be called unconditionally then. https://codereview.appspot.com/575530107/
Issue 5695: reduce dynamic casting (issue 575530107 by nine.fierce.ball...@gmail.com)
Reviewers: ,
Description:
https://sourceforge.net/p/testlilyissues/issues/5695/
Here are a few miscellaneous things that I've been holding onto for a
while because they would have been out of context in other reviews.
Each file is in a separate commit, though they're squashed in this
review.
1: defer dynamic_cast in Tie_column::add_tie
2: reduce unsmobbing in Spaceable_grob::get_spring
3: reduce unsmobbing in Paper_column::is_musical
Please review this at https://codereview.appspot.com/575530107/
Affected files (+14, -11 lines):
M lily/paper-column.cc
M lily/spaceable-grob.cc
M lily/tie-column.cc
Index: lily/paper-column.cc
diff --git a/lily/paper-column.cc b/lily/paper-column.cc
index
6fc65f27614e3c16cdf63766ee3642514b0afea6..7f17dbf8909dba3406ebd375dd1839c5b637e6c9
100644
--- a/lily/paper-column.cc
+++ b/lily/paper-column.cc
@@ -108,10 +108,9 @@ bool
Paper_column::is_musical (Grob *me)
{
SCM m = me->get_property ("shortest-starter-duration");
- Moment s (0);
- if (unsmob (m))
-s = *unsmob (m);
- return s != Moment (0);
+ if (Moment *s = unsmob (m))
+return *s != Moment (0);
+ return false;
}
bool
Index: lily/spaceable-grob.cc
diff --git a/lily/spaceable-grob.cc b/lily/spaceable-grob.cc
index
dae8db19c81726f9f2dd2d6039e1e230ad1cb358..95b7d10877753bd2c522c5646389b6e3eae6f2d3
100644
--- a/lily/spaceable-grob.cc
+++ b/lily/spaceable-grob.cc
@@ -81,13 +81,17 @@ Spaceable_grob::get_spring (Paper_column *this_col, Grob
*next_col)
Spring *spring = 0;
for (SCM s = this_col->get_object ("ideal-distances");
- !spring && scm_is_pair (s);
- s = scm_cdr (s))
+ scm_is_pair (s); s = scm_cdr (s))
{
if (scm_is_pair (scm_car (s))
- && unsmob (scm_cdar (s)) == next_col
- && unsmob (scm_caar (s)))
-spring = unsmob (scm_caar (s));
+ && unsmob (scm_cdar (s)) == next_col)
+{
+ if (Spring *next_spring = unsmob (scm_caar (s)))
+{
+ spring = next_spring;
+ break;
+}
+}
}
if (!spring)
Index: lily/tie-column.cc
diff --git a/lily/tie-column.cc b/lily/tie-column.cc
index
c3e4ec6068da02d86484217820ea884a759dcaf2..4c1b2d504f0c4b35b4046634b091b1c3fa79e56b
100644
--- a/lily/tie-column.cc
+++ b/lily/tie-column.cc
@@ -37,12 +37,12 @@ using std::vector;
void
Tie_column::add_tie (Grob *tc, Spanner *tie)
{
- Spanner *me = dynamic_cast (tc);
-
if (tie->get_parent (Y_AXIS)
&& has_interface (tie->get_parent (Y_AXIS)))
return;
+ // TODO: Change to a Spanner in the function signature.
+ Spanner *me = dynamic_cast (tc);
if (!me->get_bound (LEFT)
|| (me->get_bound (LEFT)->get_column ()->get_rank ()
> tie->get_bound (LEFT)->get_column ()->get_rank ()))
Re: Issue XXXX: Dot_configuration maintenance (issue 577380046 by nine.fierce.ball...@gmail.com)
https://codereview.appspot.com/577380046/
Doc: Corrected doc string for ly:dimension? (issue 547470049 by davidgrant...@gmail.com)
Reviewers: ,
Message:
It isn't clear to me how a dimension is different from a normal number,
but the current doc string seems to be incorrect all the same.
Description:
Doc: Corrected doc string for ly:dimension?
ly:dimension? is a predicate - it does not return a number.
Please review this at https://codereview.appspot.com/547470049/
Affected files (+1, -1 lines):
M lily/general-scheme.cc
Index: lily/general-scheme.cc
diff --git a/lily/general-scheme.cc b/lily/general-scheme.cc
index
629c85ccaa7483d542d16d54e548279d2a401483..3cad1775d2754768aa3b39f7608f5b83f0d4316e
100644
--- a/lily/general-scheme.cc
+++ b/lily/general-scheme.cc
@@ -260,7 +260,7 @@ LY_DEFINE (ly_unit, "ly:unit", 0, 0, 0, (),
}
LY_DEFINE (ly_dimension_p, "ly:dimension?", 1, 0, 0, (SCM d),
- "Return @var{d} as a number. Used to distinguish length"
+ "Is @var{d} a dimension? Used to distinguish length"
" variables from normal numbers.")
{
return scm_number_p (d);
Re: EXPERIMENTAL: put a reminder of the mm rest on the last page at the top. (issue 553400043 by hanw...@gmail.com)
On 2020/01/24 19:29:56, hanwenn wrote: > I'm not asking for anyone to review. It's context to the other change about > multi-measure-rests. OIC. An MMR reminder sounds interesting. Does the problem tie in with others like repeating the current chord name after a page break? https://codereview.appspot.com/553400043/
Re: GUILE 2.2 progress
David Kastrup writes: > Han-Wen Nienhuys writes: > >> With https://codereview.appspot.com/551390047/ > > That patch is papering over a problem rather than fixing it. It might > be a reoccurence of something like > > commit c01a2a757e3c59727bdfa8d77568bf42525fbe05 > Author: Andy Wingo > Date: Thu Jun 23 11:47:42 2016 +0200 > > Fix race between SMOB marking and finalization > > * libguile/smob.c (clear_smobnum): New helper. > (finalize_smob): Re-set the smobnum to the "finalized smob" type > before finalizing. Fixes #19883. > (scm_smob_prehistory): Pre-register a "finalized smob" type, which has > no mark procedure. > * test-suite/standalone/test-smob-mark-race.c: New file. > * test-suite/standalone/Makefile.am: Arrange to build and run the new > test. Actually, you may also be missing commit c9910c604279f438728cd268272e1839cbc53835 Author: Andy Wingo Date: Mon Mar 13 15:47:51 2017 +0100 Fix finalizer resuscitation causing excessive GC * libguile/finalizers.c (async_gc_finalizer): (scm_i_register_async_gc_callback): Replace "weak gc callback" mechanism with "async gc callback" mechanism. Very similar but the new API is designed to be called a bounded number of times, to avoid running afoul of libgc heuristics. * libguile/weak-list.h: New internal header. * libguile/Makefile.am (noinst_HEADERS): Add weak-list.h. * libguile/weak-set.c (vacuum_all_weak_sets): (scm_c_make_weak_set, scm_init_weak_set): * libguile/weak-table.c (vacuum_all_weak_tables): (scm_c_make_weak_table, scm_init_weak_table): Arrange to vacuum all weak sets from a single async GC callback, and likewise for weak tables. Thanks to Ludovic Courtès for tracking this bug down! in the version you have been using. That one looks like it could cause trouble as well. -- David Kastrup
Re: GUILE 2.2 progress
Jonas Hahnfeld writes: > Am Samstag, den 25.01.2020, 15:26 +0100 schrieb David Kastrup: >> Jonas Hahnfeld via Discussions on LilyPond development >> < >> lilypond-devel@gnu.org >> > writes: >> >> > Am Samstag, den 25.01.2020, 14:45 +0100 schrieb Thomas Morley: >> > > Am Sa., 25. Jan. 2020 um 13:48 Uhr schrieb Han-Wen Nienhuys < >> > > hanw...@gmail.com >> > > >> > > > : >> > > > [...] >> > > > I think it is clear that we should not be targeting GUILE 2.0, but >> > > > GUILE 2.2. >> > > >> > > I dropped any work for guile-2.0 long ago. >> > > For me it's more the question whether we should entirely skip guile-2, >> > > going for guile-3 directly. >> > >> > My 2 cents: Guile 3.0 was only released very recently, it's not even >> > available in rolling release distributions like Arch Linux. Most >> > certainly it will not be part of the upcoming Ubuntu 20.04 release? >> > Requiring Guile 2.2 would exclude (vanilla) Ubuntu 16.04 and CentOS 7 >> > which I think would be acceptable for master and future major versions >> > of LilyPond. >> >> 16.04 is all very nice, but relying on non-updated legacy systems for >> proscribing the dependencies of a hot-of-the-presses release is not >> providing much of value. > > me: "which I think would be acceptable for master and future major > versions" ?!? Ah, "which" is supposed to refer to "would exclude" rather than "Ubuntu 16.04 and CentOS 7". I thought you wanted to avoid excluding the acceptable listed versions. I probably would have understood had you written: Requiring Guile 2.2 would exclude (vanilla) Ubuntu 16.04 and CentOS 7. I think that should be acceptable for master and future major versions of LilyPond. Sorry for the confusion. > Also I'm not proscribing anything, it was just a list of what would be > lost. I now get it. -- David Kastrup
Re: GUILE 2.2 progress
Am Samstag, den 25.01.2020, 15:26 +0100 schrieb David Kastrup: > Jonas Hahnfeld via Discussions on LilyPond development > < > lilypond-devel@gnu.org > > writes: > > > Am Samstag, den 25.01.2020, 14:45 +0100 schrieb Thomas Morley: > > > Am Sa., 25. Jan. 2020 um 13:48 Uhr schrieb Han-Wen Nienhuys < > > > hanw...@gmail.com > > > > > > > : > > > > [...] > > > > I think it is clear that we should not be targeting GUILE 2.0, but > > > > GUILE 2.2. > > > > > > I dropped any work for guile-2.0 long ago. > > > For me it's more the question whether we should entirely skip guile-2, > > > going for guile-3 directly. > > > > My 2 cents: Guile 3.0 was only released very recently, it's not even > > available in rolling release distributions like Arch Linux. Most > > certainly it will not be part of the upcoming Ubuntu 20.04 release? > > Requiring Guile 2.2 would exclude (vanilla) Ubuntu 16.04 and CentOS 7 > > which I think would be acceptable for master and future major versions > > of LilyPond. > > 16.04 is all very nice, but relying on non-updated legacy systems for > proscribing the dependencies of a hot-of-the-presses release is not > providing much of value. me: "which I think would be acceptable for master and future major versions" ?!? Also I'm not proscribing anything, it was just a list of what would be lost. Jonas signature.asc Description: This is a digitally signed message part
Re: GUILE 2.2 progress
Jonas Hahnfeld via Discussions on LilyPond development writes: > Am Samstag, den 25.01.2020, 14:45 +0100 schrieb Thomas Morley: >> Am Sa., 25. Jan. 2020 um 13:48 Uhr schrieb Han-Wen Nienhuys < >> hanw...@gmail.com >> >: >> > [...] >> > I think it is clear that we should not be targeting GUILE 2.0, but GUILE >> > 2.2. >> >> I dropped any work for guile-2.0 long ago. >> For me it's more the question whether we should entirely skip guile-2, >> going for guile-3 directly. > > My 2 cents: Guile 3.0 was only released very recently, it's not even > available in rolling release distributions like Arch Linux. Most > certainly it will not be part of the upcoming Ubuntu 20.04 release? > Requiring Guile 2.2 would exclude (vanilla) Ubuntu 16.04 and CentOS 7 > which I think would be acceptable for master and future major versions > of LilyPond. 16.04 is all very nice, but relying on non-updated legacy systems for proscribing the dependencies of a hot-of-the-presses release is not providing much of value. -- David Kastrup
Re: Issue 4550: Avoid "using namespace std; " in included files (Take 2) (issue 579240043 by nine.fierce.ball...@gmail.com)
Am Samstag, den 25.01.2020, 14:01 +0100 schrieb Han-Wen Nienhuys: > I've pushed all my local branches to > https://github.com/hanwen/lilypond > , which make the rebasing and such > easier. > > How does the pushing process go? Even though I am busy, maybe Jonas is > right that it's easier for everyone if I push directly. I mean everyone is busy... The process is here: http://lilypond.org/doc/v2.19/Documentation/contributor/pushing-to-staging In short, go to the branch with the commit to push, rebase it to origin/staging and then $ git push origin HEAD:staging. I myself usually do a $ git push -n origin HEAD:staging first (dry-run) and check that it's indeed the commits that counted down... Jonas signature.asc Description: This is a digitally signed message part
Re: Issue 4550: Avoid "using namespace std; " in included files (Take 2) (issue 579240043 by nine.fierce.ball...@gmail.com)
Dan Eble writes: > On Jan 25, 2020, at 07:26, David Kastrup wrote: >> >> - Interval hex = head->extent (common_[X_AXIS], X_AXIS); >> + Interval head_ext = head->extent (common_[X_AXIS], X_AXIS); > ... >> >> That last part applies part of a patch from an unrelated issue of >> Han-Wen. Please don't do stuff like that, if necessary using > > It does not apply a patch from Han-Wen. It necessarily renames that > variable prior to a scripted replacement of "hex" with "std::hex". I > wonder whether the task would have been easier if I had used a > different name for the variable. Well, then I'd have had something looking like an actual conflict in my rejected parts of the patches. It would likely have been a bit more work to resolve and quite a bit less to hunt down. > I don't understand from this description exactly problem you > experienced and where in my development process you would have wanted > me to use that command. It wasn't clear that this partial overlap with a patch of Han-Wen was independently arrived at and manually created rather than an artifact. The problem I experienced is that a partially applying patch apparently was partly applied previously. That this was not a problem of what I was doing took some time to find out. Of course, resetting would indeed not have helped since my guess at the discrepancy's origin was wrong. -- David Kastrup
Re: GUILE 2.2 progress
Am Samstag, den 25.01.2020, 14:45 +0100 schrieb Thomas Morley: > Am Sa., 25. Jan. 2020 um 13:48 Uhr schrieb Han-Wen Nienhuys < > hanw...@gmail.com > >: > > [...] > > I think it is clear that we should not be targeting GUILE 2.0, but GUILE > > 2.2. > > I dropped any work for guile-2.0 long ago. > For me it's more the question whether we should entirely skip guile-2, > going for guile-3 directly. My 2 cents: Guile 3.0 was only released very recently, it's not even available in rolling release distributions like Arch Linux. Most certainly it will not be part of the upcoming Ubuntu 20.04 release? Requiring Guile 2.2 would exclude (vanilla) Ubuntu 16.04 and CentOS 7 which I think would be acceptable for master and future major versions of LilyPond. Jonas signature.asc Description: This is a digitally signed message part
Re: GUILE 2.2 progress
Am Sa., 25. Jan. 2020 um 15:03 Uhr schrieb Han-Wen Nienhuys : > > On Sat, Jan 25, 2020 at 2:45 PM Thomas Morley > wrote: > > Compiling the same file with my slow loptop and > > released 2.19.83 > > > .. > > Without the error with string-append > > interesting. I was on 2.2.4; maybe these were bugs fixed in 2.2.6 or > beyond. Let me see if I can upgrade things. Iirc, the fix is already in our remote branch dev/guile-v2-work. I may be wrong, though worth a look. Cheers, Harm
Re: Issue 4550: Avoid "using namespace std; " in included files (Take 2) (issue 579240043 by nine.fierce.ball...@gmail.com)
Han-Wen Nienhuys writes: > I've pushed all my local branches to > https://github.com/hanwen/lilypond , which make the rebasing and such > easier. > > How does the pushing process go? Even though I am busy, maybe Jonas is > right that it's easier for everyone if I push directly. Current batch is through. Whatever rocks your boat. -- David Kastrup
Re: GUILE 2.2 progress
On Sat, Jan 25, 2020 at 2:45 PM Thomas Morley wrote: > Compiling the same file with my slow loptop and > released 2.19.83 > .. > Without the error with string-append interesting. I was on 2.2.4; maybe these were bugs fixed in 2.2.6 or beyond. Let me see if I can upgrade things. > > These timings are internally consistent: if we can do byte-compilation > > on the .scm files (which would make the SCM reading instantaneous), > > and get the GC overhead down, we'd be at the same performance level of > > GUILE 1.8. > > > > What do folks think is more important? Reducing GC overhead is a win > > on large scores, while byte-compilation will make compile/edit/debug > > cycles faster. > > Having a slow laptop, I'm always in favor of speed, though I didn't > understand what benefit you expect by "reducing GC overhead". > Could you explain? Compiling the .scm files happens once, and is a fixed cost. The fixed cost is large when you process a tiny .ly file, but it is neglible if the file is large. The overhead of the new Garbage Collector is likely proportional to the size of the input, and is the cause that GUILE 2.2/3 will be about 50% slower than GUILE 1.8. -- Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
Re: GUILE 2.2 progress
Han-Wen Nienhuys writes: > With https://codereview.appspot.com/551390047/ That patch is papering over a problem rather than fixing it. It might be a reoccurence of something like commit c01a2a757e3c59727bdfa8d77568bf42525fbe05 Author: Andy Wingo Date: Thu Jun 23 11:47:42 2016 +0200 Fix race between SMOB marking and finalization * libguile/smob.c (clear_smobnum): New helper. (finalize_smob): Re-set the smobnum to the "finalized smob" type before finalizing. Fixes #19883. (scm_smob_prehistory): Pre-register a "finalized smob" type, which has no mark procedure. * test-suite/standalone/test-smob-mark-race.c: New file. * test-suite/standalone/Makefile.am: Arrange to build and run the new test. > it looks like I can run LilyPond against GUILE 2.2 mostly: > > > $ time lilypond input/regression/mozart-hrn-3.ly > GNU LilyPond 2.21.0 > Import (ice-9 threads) to have access to `call-with-new-thread'. > Import (ice-9 threads) to have access to `current-thread'. > Processing `input/regression/mozart-hrn-3.ly' > Parsing... > input/regression/mozart-hrn-3.ly:31:9: error: not a markup > > #(string-append "It has been typeset and placed in the public " Looks like it should be a markup. > Interpreting > music...[8][16][24][32][40][48][56][64][72][80][88][96][104][112][120] > Preprocessing graphical objects... > Interpreting music... > Interpreting music... > Interpreting music...[8][16][24][32][40][48] > Preprocessing graphical objects... > Interpreting music... > Interpreting > music...[8][16][24][32][40][48][56][64][72][80][88][96][104][112][120][128][136][144] > Preprocessing graphical objects... > MIDI output to `mozart-hrn-3.midi'... > MIDI output to `mozart-hrn-3-1.midi'... > MIDI output to `mozart-hrn-3-2.midi'... > Finding the ideal number of pages... > Fitting music on 3 or 4 pages... > Drawing systems... > Layout output to `/tmp/lilypond-A2ssuI'... > Converting to `mozart-hrn-3.pdf'... > Deleting `/tmp/lilypond-A2ssuI'... > fatal error: failed files: "input/regression/mozart-hrn-3.ly" > > real 0m7.239s > user 0m8.637s > sys 0m0.121s > > (this disables the GS step, as something is munging a string , causing > GS to barf.) Likely an encoding problem. > 7.2 seconds end-to-end includes 1.7s of GC, and 2.0s of reading/compiling SCM. > > On guile 1.8, with GS disabled, the end to end runtime is > > 3.568s including 0.33s for GC and 0.32s for reading the SCM. > > These timings are internally consistent: if we can do byte-compilation > on the .scm files (which would make the SCM reading instantaneous), > and get the GC overhead down, we'd be at the same performance level of > GUILE 1.8. > > What do folks think is more important? Reducing GC overhead is a win > on large scores, while byte-compilation will make compile/edit/debug > cycles faster. Byte compilation is sort of a big thing since it needs to find places for the .go files to go in a LilyPond installation/distribution without messing up future compile/edit/debug cycles. If you offer to do either, my vote would be on that as the clearly more tedious one. > Even if we wouldn't recoup all the overhead of GC, I think the reduced > complexity of using BGC for GC might be worth it. > > I think it is clear that we should not be targeting GUILE 2.0, but > GUILE 2.2. Well, not much of a point in targeting an old stable version except possibly for checking the new stable for regressions. -- David Kastrup
Re: mirroring lilypond on github
> I'm going to call > > git push --mirror g...@github.com:lilypond/lilypond.git > > to update the github mirror. However, this will cause the following > changes: I've now done a push, and it seems to have worked correctly. Please check. Werner
Re: GUILE 2.2 progress
Am Sa., 25. Jan. 2020 um 13:48 Uhr schrieb Han-Wen Nienhuys : > > With https://codereview.appspot.com/551390047/ > it looks like I can run LilyPond against GUILE 2.2 mostly: > > > $ time lilypond input/regression/mozart-hrn-3.ly > GNU LilyPond 2.21.0 > Import (ice-9 threads) to have access to `call-with-new-thread'. > Import (ice-9 threads) to have access to `current-thread'. > Processing `input/regression/mozart-hrn-3.ly' > Parsing... > input/regression/mozart-hrn-3.ly:31:9: error: not a markup > > #(string-append "It has been typeset and placed in the public " > Interpreting > music...[8][16][24][32][40][48][56][64][72][80][88][96][104][112][120] > Preprocessing graphical objects... > Interpreting music... > Interpreting music... > Interpreting music...[8][16][24][32][40][48] > Preprocessing graphical objects... > Interpreting music... > Interpreting > music...[8][16][24][32][40][48][56][64][72][80][88][96][104][112][120][128][136][144] > Preprocessing graphical objects... > MIDI output to `mozart-hrn-3.midi'... > MIDI output to `mozart-hrn-3-1.midi'... > MIDI output to `mozart-hrn-3-2.midi'... > Finding the ideal number of pages... > Fitting music on 3 or 4 pages... > Drawing systems... > Layout output to `/tmp/lilypond-A2ssuI'... > Converting to `mozart-hrn-3.pdf'... > Deleting `/tmp/lilypond-A2ssuI'... > fatal error: failed files: "input/regression/mozart-hrn-3.ly" > > real 0m7.239s > user 0m8.637s > sys 0m0.121s > > (this disables the GS step, as something is munging a string , causing > GS to barf.) > > 7.2 seconds end-to-end includes 1.7s of GC, and 2.0s of reading/compiling SCM. > > On guile 1.8, with GS disabled, the end to end runtime is > > 3.568s including 0.33s for GC and 0.32s for reading the SCM. Compiling the same file with my slow loptop and released 2.19.83 real 0m13,777s user 0m12,731s sys 0m0,855s with my own guile-3 setup (none of your recent patches applied): real 0m22,384s user 0m24,366s sys 0m0,693s Without the error with string-append > > These timings are internally consistent: if we can do byte-compilation > on the .scm files (which would make the SCM reading instantaneous), > and get the GC overhead down, we'd be at the same performance level of > GUILE 1.8. > > What do folks think is more important? Reducing GC overhead is a win > on large scores, while byte-compilation will make compile/edit/debug > cycles faster. Having a slow laptop, I'm always in favor of speed, though I didn't understand what benefit you expect by "reducing GC overhead". Could you explain? > > Even if we wouldn't recoup all the overhead of GC, I think the reduced > complexity of using BGC for GC might be worth it. > > I think it is clear that we should not be targeting GUILE 2.0, but GUILE 2.2. I dropped any work for guile-2.0 long ago. For me it's more the question whether we should entirely skip guile-2, going for guile-3 directly. > -- > Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen > Best, Harm
Re: Issue 4550: Avoid "using namespace std; " in included files (Take 2) (issue 579240043 by nine.fierce.ball...@gmail.com)
On Jan 25, 2020, at 07:26, David Kastrup wrote: > > - Interval hex = head->extent (common_[X_AXIS], X_AXIS); > + Interval head_ext = head->extent (common_[X_AXIS], X_AXIS); ... > > That last part applies part of a patch from an unrelated issue of > Han-Wen. Please don't do stuff like that, if necessary using It does not apply a patch from Han-Wen. It necessarily renames that variable prior to a scripted replacement of "hex" with "std::hex". I wonder whether the task would have been easier if I had used a different name for the variable. > Please don't do stuff like that, if necessary using > > git reset --hard I don't understand from this description exactly problem you experienced and where in my development process you would have wanted me to use that command. — Dan
Re: Issue 4550: Avoid "using namespace std; " in included files (Take 2) (issue 579240043 by nine.fierce.ball...@gmail.com)
(and in the process, I erroneously clobbered
https://github.com/lilypond/lilypond, which I am fixing now.
On Sat, Jan 25, 2020 at 2:01 PM Han-Wen Nienhuys wrote:
>
> I've pushed all my local branches to
> https://github.com/hanwen/lilypond , which make the rebasing and such
> easier.
>
> How does the pushing process go? Even though I am busy, maybe Jonas is
> right that it's easier for everyone if I push directly.
>
> On Sat, Jan 25, 2020 at 1:26 PM David Kastrup wrote:
> >
> > d...@gnu.org writes:
> >
> > > On 2020/01/24 15:26:06, dak wrote:
> > >
> > > What I meant to say: I guess I should be able to handle those
> > > comparatively obvious merge conflicts.
> > >
> > > https://codereview.appspot.com/579240043/
> >
> > But frankly, I have not been reckoning with having to deal with the ilk
> > of
> >
> > commit 6f4128e1f359daf38a3caf2c0e4ad68e16c10540
> > Author: Dan Eble
> > Date: Thu Jan 9 13:04:18 2020 -0500
> >
> > Issue 4550/2: Avoid "using namespace std;" in included files
> >
> > These are manual changes in preparation for an automated removal of
> > "using namespace std;".
> >
> > Mostly, these are additions of using-declarations for commonly used
> > types and containers (e.g. std::string, std::vector) to *.cc files so
> > that they will continue to build after the big removal.
> >
> > diff --git a/lily/slur-scoring.cc b/lily/slur-scoring.cc
> > index bb779a040e..262b41a677 100644
> > --- a/lily/slur-scoring.cc
> > +++ b/lily/slur-scoring.cc
> > @@ -45,6 +45,9 @@
> > #include "stem.hh"
> > #include "warn.hh"
> >
> > +using std::string;
> > +using std::vector;
> > +
> > /*
> >TODO:
> >
> > @@ -122,12 +125,12 @@ Slur_score_state::get_encompass_info (Grob *col) const
> >
> >if (Grob *head = Note_column::first_head (col))
> > {
> > - Interval hex = head->extent (common_[X_AXIS], X_AXIS);
> > + Interval head_ext = head->extent (common_[X_AXIS], X_AXIS);
> >// FIXME: Is there a better option than setting to 0?
> > - if (hex.is_empty ())
> > + if (head_ext.is_empty ())
> > ei.x_ = 0;
> >else
> > -ei.x_ = hex.center ();
> > +ei.x_ = head_ext.center ();
> > }
> >else
> > ei.x_ = col->extent (common_[X_AXIS], X_AXIS).center ();
> >
> > That last part applies part of a patch from an unrelated issue of
> > Han-Wen. Please don't do stuff like that, if necessary using
> >
> > git reset --hard
> >
> > Took me about half an hour of head-scratching to figure out why the
> > diffs would differ here.
> >
> > --
> > David Kastrup
> >
>
>
> --
> Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
--
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
Re: Issue 4550: Avoid "using namespace std; " in included files (Take 2) (issue 579240043 by nine.fierce.ball...@gmail.com)
I've pushed all my local branches to
https://github.com/hanwen/lilypond , which make the rebasing and such
easier.
How does the pushing process go? Even though I am busy, maybe Jonas is
right that it's easier for everyone if I push directly.
On Sat, Jan 25, 2020 at 1:26 PM David Kastrup wrote:
>
> d...@gnu.org writes:
>
> > On 2020/01/24 15:26:06, dak wrote:
> >
> > What I meant to say: I guess I should be able to handle those
> > comparatively obvious merge conflicts.
> >
> > https://codereview.appspot.com/579240043/
>
> But frankly, I have not been reckoning with having to deal with the ilk
> of
>
> commit 6f4128e1f359daf38a3caf2c0e4ad68e16c10540
> Author: Dan Eble
> Date: Thu Jan 9 13:04:18 2020 -0500
>
> Issue 4550/2: Avoid "using namespace std;" in included files
>
> These are manual changes in preparation for an automated removal of
> "using namespace std;".
>
> Mostly, these are additions of using-declarations for commonly used
> types and containers (e.g. std::string, std::vector) to *.cc files so
> that they will continue to build after the big removal.
>
> diff --git a/lily/slur-scoring.cc b/lily/slur-scoring.cc
> index bb779a040e..262b41a677 100644
> --- a/lily/slur-scoring.cc
> +++ b/lily/slur-scoring.cc
> @@ -45,6 +45,9 @@
> #include "stem.hh"
> #include "warn.hh"
>
> +using std::string;
> +using std::vector;
> +
> /*
>TODO:
>
> @@ -122,12 +125,12 @@ Slur_score_state::get_encompass_info (Grob *col) const
>
>if (Grob *head = Note_column::first_head (col))
> {
> - Interval hex = head->extent (common_[X_AXIS], X_AXIS);
> + Interval head_ext = head->extent (common_[X_AXIS], X_AXIS);
>// FIXME: Is there a better option than setting to 0?
> - if (hex.is_empty ())
> + if (head_ext.is_empty ())
> ei.x_ = 0;
>else
> -ei.x_ = hex.center ();
> +ei.x_ = head_ext.center ();
> }
>else
> ei.x_ = col->extent (common_[X_AXIS], X_AXIS).center ();
>
> That last part applies part of a patch from an unrelated issue of
> Han-Wen. Please don't do stuff like that, if necessary using
>
> git reset --hard
>
> Took me about half an hour of head-scratching to figure out why the
> diffs would differ here.
>
> --
> David Kastrup
>
--
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
Re: Define operator new/delete for smobs (issue 551390047 by hanw...@gmail.com)
On 2020/01/25 12:50:15, hanwenn wrote: > On 2020/01/25 12:37:32, dak wrote: > > That seems weird for me. The topic states "this provides a way to run > LilyPond > > with Guile 2.2" but garbage collection with Guile 2.2 works out of the box > > already. This patch only causes extra work and will slow down garbage > > collection further. > > > > The only way in which it could make sense is if in return some other GC hooks > > and/or work would be disabled. > > running against > > $ guile2.2 --version > guile (GNU Guile) 2.2.4 > > I get > > Import (ice-9 threads) to have access to `call-with-new-thread'. > Import (ice-9 threads) to have access to `current-thread'. > Processing `input/regression/mozart-hrn-3.ly' > Parsing...double free or corruption (!prev) > > Thread 5 "lilypond" received signal SIGABRT, Aborted. > [Switching to Thread 0x7fffeea92700 (LWP 31206)] > 0x77526e35 in raise () from /lib64/libc.so.6 > Missing separate debuginfos, use: dnf debuginfo-install > guile22-2.2.4-3.fc30.x86_64 libunistring-0.9.10-5.fc30.x86_64 > (gdb) up > #1 0x77511895 in abort () from /lib64/libc.so.6 > (gdb) > #2 0x7756a08f in __libc_message () from /lib64/libc.so.6 > (gdb) > #3 0x7757140c in malloc_printerr () from /lib64/libc.so.6 > (gdb) > #4 0x775731bc in _int_free () from /lib64/libc.so.6 > (gdb) > #5 0x004dd2c4 in Simple_smob::free_smob (obj=) at > /usr/include/c++/9/bits/basic_string.h:2300 > 2300c_str() const _GLIBCXX_NOEXCEPT > (gdb) > #6 0x77b8b4a5 in GC_invoke_finalizers () at finalize.c:1216 > 1216 (*(curr_fo -> fo_fn))((ptr_t)(curr_fo -> fo_hidden_base), > (gdb) down > #5 0x004dd2c4 in Simple_smob::free_smob (obj=) at > /usr/include/c++/9/bits/basic_string.h:2300 > 2300c_str() const _GLIBCXX_NOEXCEPT > (gdb) quit we could make this conditional on GUILEV2 obviously. Unfortunately, we can't as yet get rid fo the mark functions, because we need a vector with a custom allocator first (Grob_array!). https://codereview.appspot.com/551390047/
Re: Define operator new/delete for smobs (issue 551390047 by hanw...@gmail.com)
Reviewers: dak,
Message:
On 2020/01/25 12:37:32, dak wrote:
> That seems weird for me. The topic states "this provides a way to run
LilyPond
> with Guile 2.2" but garbage collection with Guile 2.2 works out of the
box
> already. This patch only causes extra work and will slow down garbage
> collection further.
>
> The only way in which it could make sense is if in return some other
GC hooks
> and/or work would be disabled.
running against
$ guile2.2 --version
guile (GNU Guile) 2.2.4
I get
Import (ice-9 threads) to have access to `call-with-new-thread'.
Import (ice-9 threads) to have access to `current-thread'.
Processing `input/regression/mozart-hrn-3.ly'
Parsing...double free or corruption (!prev)
Thread 5 "lilypond" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffeea92700 (LWP 31206)]
0x77526e35 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install
guile22-2.2.4-3.fc30.x86_64 libunistring-0.9.10-5.fc30.x86_64
(gdb) up
#1 0x77511895 in abort () from /lib64/libc.so.6
(gdb)
#2 0x7756a08f in __libc_message () from /lib64/libc.so.6
(gdb)
#3 0x7757140c in malloc_printerr () from /lib64/libc.so.6
(gdb)
#4 0x775731bc in _int_free () from /lib64/libc.so.6
(gdb)
#5 0x004dd2c4 in Simple_smob::free_smob (obj=) at /usr/include/c++/9/bits/basic_string.h:2300
2300 c_str() const _GLIBCXX_NOEXCEPT
(gdb)
#6 0x77b8b4a5 in GC_invoke_finalizers () at finalize.c:1216
1216(*(curr_fo -> fo_fn))((ptr_t)(curr_fo -> fo_hidden_base),
(gdb) down
#5 0x004dd2c4 in Simple_smob::free_smob (obj=) at /usr/include/c++/9/bits/basic_string.h:2300
2300 c_str() const _GLIBCXX_NOEXCEPT
(gdb) quit
Description:
Define operator new/delete for smobs
This marks the memory for tracing with BDW GC in GUILE >= 2.0
automatically. This provides a way to run LilyPond against GUILE 2.2.
Please review this at https://codereview.appspot.com/551390047/
Affected files (+9, -0 lines):
M lily/include/smobs.hh
Index: lily/include/smobs.hh
diff --git a/lily/include/smobs.hh b/lily/include/smobs.hh
index
5fd1ed660f134bf841708a7271f0b3b792ad48db..b75f9fb928fd03024d3349e1df663f6737592acb
100644
--- a/lily/include/smobs.hh
+++ b/lily/include/smobs.hh
@@ -175,6 +175,15 @@ class Smob_base
static Scm_init scm_init_;
static void init (void);
static std::string smob_name_;
+
+public:
+ void *operator new(size_t sz) {
+return scm_gc_malloc(sz, "smob");
+ }
+ void operator delete(void *p) {
+scm_gc_free(p, 0, "smob");
+ }
+
protected:
static Super *unchecked_unsmob (SCM s)
{
GUILE 2.2 progress
With https://codereview.appspot.com/551390047/ it looks like I can run LilyPond against GUILE 2.2 mostly: $ time lilypond input/regression/mozart-hrn-3.ly GNU LilyPond 2.21.0 Import (ice-9 threads) to have access to `call-with-new-thread'. Import (ice-9 threads) to have access to `current-thread'. Processing `input/regression/mozart-hrn-3.ly' Parsing... input/regression/mozart-hrn-3.ly:31:9: error: not a markup #(string-append "It has been typeset and placed in the public " Interpreting music...[8][16][24][32][40][48][56][64][72][80][88][96][104][112][120] Preprocessing graphical objects... Interpreting music... Interpreting music... Interpreting music...[8][16][24][32][40][48] Preprocessing graphical objects... Interpreting music... Interpreting music...[8][16][24][32][40][48][56][64][72][80][88][96][104][112][120][128][136][144] Preprocessing graphical objects... MIDI output to `mozart-hrn-3.midi'... MIDI output to `mozart-hrn-3-1.midi'... MIDI output to `mozart-hrn-3-2.midi'... Finding the ideal number of pages... Fitting music on 3 or 4 pages... Drawing systems... Layout output to `/tmp/lilypond-A2ssuI'... Converting to `mozart-hrn-3.pdf'... Deleting `/tmp/lilypond-A2ssuI'... fatal error: failed files: "input/regression/mozart-hrn-3.ly" real 0m7.239s user 0m8.637s sys 0m0.121s (this disables the GS step, as something is munging a string , causing GS to barf.) 7.2 seconds end-to-end includes 1.7s of GC, and 2.0s of reading/compiling SCM. On guile 1.8, with GS disabled, the end to end runtime is 3.568s including 0.33s for GC and 0.32s for reading the SCM. These timings are internally consistent: if we can do byte-compilation on the .scm files (which would make the SCM reading instantaneous), and get the GC overhead down, we'd be at the same performance level of GUILE 1.8. What do folks think is more important? Reducing GC overhead is a win on large scores, while byte-compilation will make compile/edit/debug cycles faster. Even if we wouldn't recoup all the overhead of GC, I think the reduced complexity of using BGC for GC might be worth it. I think it is clear that we should not be targeting GUILE 2.0, but GUILE 2.2. -- Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen
Define operator new/delete for smobs (issue 551390047 by hanw...@gmail.com)
That seems weird for me. The topic states "this provides a way to run LilyPond with Guile 2.2" but garbage collection with Guile 2.2 works out of the box already. This patch only causes extra work and will slow down garbage collection further. The only way in which it could make sense is if in return some other GC hooks and/or work would be disabled. https://codereview.appspot.com/551390047/
Re: Issue 4550: Avoid "using namespace std; " in included files (Take 2) (issue 579240043 by nine.fierce.ball...@gmail.com)
d...@gnu.org writes:
> On 2020/01/24 15:26:06, dak wrote:
>
> What I meant to say: I guess I should be able to handle those
> comparatively obvious merge conflicts.
>
> https://codereview.appspot.com/579240043/
But frankly, I have not been reckoning with having to deal with the ilk
of
commit 6f4128e1f359daf38a3caf2c0e4ad68e16c10540
Author: Dan Eble
Date: Thu Jan 9 13:04:18 2020 -0500
Issue 4550/2: Avoid "using namespace std;" in included files
These are manual changes in preparation for an automated removal of
"using namespace std;".
Mostly, these are additions of using-declarations for commonly used
types and containers (e.g. std::string, std::vector) to *.cc files so
that they will continue to build after the big removal.
diff --git a/lily/slur-scoring.cc b/lily/slur-scoring.cc
index bb779a040e..262b41a677 100644
--- a/lily/slur-scoring.cc
+++ b/lily/slur-scoring.cc
@@ -45,6 +45,9 @@
#include "stem.hh"
#include "warn.hh"
+using std::string;
+using std::vector;
+
/*
TODO:
@@ -122,12 +125,12 @@ Slur_score_state::get_encompass_info (Grob *col) const
if (Grob *head = Note_column::first_head (col))
{
- Interval hex = head->extent (common_[X_AXIS], X_AXIS);
+ Interval head_ext = head->extent (common_[X_AXIS], X_AXIS);
// FIXME: Is there a better option than setting to 0?
- if (hex.is_empty ())
+ if (head_ext.is_empty ())
ei.x_ = 0;
else
-ei.x_ = hex.center ();
+ei.x_ = head_ext.center ();
}
else
ei.x_ = col->extent (common_[X_AXIS], X_AXIS).center ();
That last part applies part of a patch from an unrelated issue of
Han-Wen. Please don't do stuff like that, if necessary using
git reset --hard
Took me about half an hour of head-scratching to figure out why the
diffs would differ here.
--
David Kastrup
Re: Simplify and speed up uniquify (issue 583390043 by hanw...@gmail.com)
On 2020/01/24 15:50:22, Dan Eble wrote: > On 2020/01/24 15:17:02, dak wrote: > > On 2020/01/24 14:11:59, Dan Eble wrote: > > > On 2020/01/24 14:06:22, hanwenn wrote: > > > > If the allocation cost becomes problematic, we can use another hashmap > > > instead. > > > > > > "We" could profile it and know before pushing whether it is worth it. > > > > I don't think that this is really performance-critical. > > I accept your informed intuition, but I also have more suggestions. > > Don't assume the hash function is perfect: create the table with more buckets, > say 2*size. > > Instead of calling find() then insert(), just call insert() and check whether it > worked, something like this (untested): > > if (seen.insert(grobs[i]).second) > grobs[j++] = grobs[i]; > > https://en.cppreference.com/w/cpp/container/unordered_set/insert Done. https://codereview.appspot.com/583390043/
Re: Clean up and document include file searching (issue 573400043 by hanw...@gmail.com)
https://codereview.appspot.com/573400043/diff/549420043/lily/sources.cc File lily/sources.cc (right): https://codereview.appspot.com/573400043/diff/549420043/lily/sources.cc#newcode60 lily/sources.cc:60: Sources::find_full_path(string file_string, string const _dir) On 2020/01/21 13:42:27, Dan Eble wrote: > Is there any reason not to make this const? Done. https://codereview.appspot.com/573400043/
PATCHES - Countdown for January 25th
Hello, Here is the current patch countdown list. The next countdown will be on January 27th A quick synopsis of all patches currently in the review process can be found here: http://philholmes.net/lilypond/allura/ Push: 5676 remove obsolete lines from lily-guile-macros.hh - hanwen https://sourceforge.net/p/testlilyissues/issues/5676 http://codereview.appspot.com/555170043 5675 Document C++ structs for slur scoring - hanwen https://sourceforge.net/p/testlilyissues/issues/5675 http://codereview.appspot.com/571380043 5673 Remove implicit evaluation of the ".twy" file - hanwen https://sourceforge.net/p/testlilyissues/issues/5673 http://codereview.appspot.com/551380043 5671 lily/page-breaking: pass vector by reference - hanwen https://sourceforge.net/p/testlilyissues/issues/5671 http://codereview.appspot.com/581510043 5669 Document why System::rank_type is int16 - hanwen https://sourceforge.net/p/testlilyissues/issues/5669 http://codereview.appspot.com/559370043 5668 Documentation: fix typo in German spacing.itely file - hanwen https://sourceforge.net/p/testlilyissues/issues/5668 http://codereview.appspot.com/581490043 5667 Documentation: fix typos in tool naming - hanwen https://sourceforge.net/p/testlilyissues/issues/5667 http://codereview.appspot.com/581480043 5666 ly_scm_write_string: call scm_write directly - hanwen https://sourceforge.net/p/testlilyissues/issues/5666 http://codereview.appspot.com/545450043 Countdown: 5672 Clean up and document include file searching - hanwen https://sourceforge.net/p/testlilyissues/issues/5672 http://codereview.appspot.com/573400043 Review: 5689 Refactor enforcement of a single Score context - Dan Eble https://sourceforge.net/p/testlilyissues/issues/5689 http://codereview.appspot.com/551390046 5688 Announce end of multi-measure-rest - hanwen https://sourceforge.net/p/testlilyissues/issues/5688 http://codereview.appspot.com/561310045 5687 Remove dead code throughout - hanwen https://sourceforge.net/p/testlilyissues/issues/5687 http://codereview.appspot.com/547470044 5686 Simplify and speed up uniquify - hanwen https://sourceforge.net/p/testlilyissues/issues/5686 http://codereview.appspot.com/583390043 5685 Remove outdated comment about resizable hash tables - hanwen https://sourceforge.net/p/testlilyissues/issues/5685 http://codereview.appspot.com/549460043 5684 Explain dead code for GUILE2 - hanwen https://sourceforge.net/p/testlilyissues/issues/5684 http://codereview.appspot.com/547470043 5683 GUILE2: generate internals doc in UTF-8 - hanwen https://sourceforge.net/p/testlilyissues/issues/5683 http://codereview.appspot.com/575540045 5682 Only print out open type font substitution if there was a change - hanwen https://sourceforge.net/p/testlilyissues/issues/5682 http://codereview.appspot.com/577390043 5678 l -> lilne - hanwen https://sourceforge.net/p/testlilyissues/issues/5678 http://codereview.appspot.com/581470047 5670 lily: fix some type conversion warnings - hanwen https://sourceforge.net/p/testlilyissues/issues/5670 http://codereview.appspot.com/557190043 New: 5690 Tiny fixes for "Extract PDFmark" - Jonas Hahnfeld https://sourceforge.net/p/testlilyissues/issues/5690 http://codereview.appspot.com/571420055 5680 Provide --loglevel=PROGRESS for make VERBOSE=1 - hanwen https://sourceforge.net/p/testlilyissues/issues/5680 http://codereview.appspot.com/575540044 5646 Switch to Python 3.x - Jonas Hahnfeld https://sourceforge.net/p/testlilyissues/issues/5646 http://codereview.appspot.com/545370043 *** Regards, James
Re: french beaming incorrectly makes stems longer
Hello,
On 25/01/2020 09:43, Malte Meyn wrote:
Am 25.01.20 um 08:12 schrieb Werner LEMBERG:
Before adding it to the bug tracker I want to check this group's
knowledge whether there is a reason that French beams are longer than
normal:
\relative c'' {
r16 f d f
\override Stem.french-beaming = ##t
r16 f d f
}
Looking into scores from French publishers this behaviour seems to be
incorrect.
Another problem that has to do with stem lengths in french beaming:
articulations use the shorter beam length for positioning, resulting
in collisions:
%
\version "2.19.83"
\relative c'' {
\voiceTwo
r16 f-> d-> f->
\override Stem.french-beaming = ##t
r16 f-> d-> f->
}
%
Both beam and articulation positions should be the same for default
and french beams. Probably the easiest solution would be to have
shorter stem stencils without decreasing the stems’ Y extents.
Added as
https://sourceforge.net/p/testlilyissues/issues/5691/
James
Re: french beaming incorrectly makes stems longer
Am 25.01.20 um 08:12 schrieb Werner LEMBERG:
Before adding it to the bug tracker I want to check this group's
knowledge whether there is a reason that French beams are longer than
normal:
\relative c'' {
r16 f d f
\override Stem.french-beaming = ##t
r16 f d f
}
Looking into scores from French publishers this behaviour seems to be
incorrect.
Another problem that has to do with stem lengths in french beaming:
articulations use the shorter beam length for positioning, resulting in
collisions:
%
\version "2.19.83"
\relative c'' {
\voiceTwo
r16 f-> d-> f->
\override Stem.french-beaming = ##t
r16 f-> d-> f->
}
%
Both beam and articulation positions should be the same for default and
french beams. Probably the easiest solution would be to have shorter
stem stencils without decreasing the stems’ Y extents.
Re: Tiny fixes for "Extract PDFmark" (issue 571420055 by jonas.hahnf...@gmail.com)
Reviewers: lemzwerg, https://codereview.appspot.com/571420055/diff/571400046/configure.ac File configure.ac (right): https://codereview.appspot.com/571420055/diff/571400046/configure.ac#newcode391 configure.ac:391: ["(Using Ghostscript >= 9.20 together with Extract PDFmark"]) On 2020/01/25 09:15:10, lemzwerg wrote: > I suggest to use the program's name. > > s/Extract PDFmark/extractpdfmark/ I wasn't sure either, but https://github.com/trueroad/extractpdfmark says "Extract PDFmark" it's the full name of the project. But maybe we should just change it for consistency. Description: Tiny fixes for "Extract PDFmark" Individual changes: 1) Fix extractpdfmark not coming from PATH If STEPMAKE_PROGS cannot find an optional program, it returns "false". Checking for this value is better than requiring exactly "extractpdfmark" which does not hold if the program is not in PATH and the user explicitly sets the environment variable EXTRACTPDFMARK. 2) Fix extractpdfmark with GhostScript 9.50 GhostScript 9.50 defaults to -dSAFE, so the build system needs to pas in -dNOSAFE explicitly. 3) Update message for optional extractpdfmark >From my experiments, enabling Extract PDFmark rather increases the required disk space. This is probably because the process produces additional intermediate files. It only benefits the final PDF docs. Please review this at https://codereview.appspot.com/571420055/ Affected files (+6, -5 lines): M configure.ac M make/lilypond-book-rules.make M stepmake/stepmake/texinfo-rules.make Index: configure.ac diff --git a/configure.ac b/configure.ac index c6261aeb6abc224490b52458578dfe947133e37c..0cbd0c26b1e3ea6ebaa57cab801b151e18c37e11 100644 --- a/configure.ac +++ b/configure.ac @@ -382,17 +382,15 @@ if test "$GS920_VERSION" -lt "$req"; then GS920= USE_EXTRACTPDFMARK=no fi -if test "$EXTRACTPDFMARK" != "extractpdfmark"; then +if test "$EXTRACTPDFMARK" == "false"; then EXTRACTPDFMARK= USE_EXTRACTPDFMARK=no fi if test "$USE_EXTRACTPDFMARK" != "yes"; then STEPMAKE_ADD_ENTRY(OPTIONAL, -["(Optionally using Ghostscript >= 9.20 together with"]) +["(Using Ghostscript >= 9.20 together with Extract PDFmark"]) STEPMAKE_ADD_ENTRY(OPTIONAL, -[" Extract PDFmark can significantly reduce the disk space required"]) -STEPMAKE_ADD_ENTRY(OPTIONAL, -[" for building the documentation and the final PDF files.)"]) +[" can significantly reduce the size of the final PDF files.)"]) fi STEPMAKE_PROGS(MAKEINFO, makeinfo, REQUIRED, 6.1) Index: make/lilypond-book-rules.make diff --git a/make/lilypond-book-rules.make b/make/lilypond-book-rules.make index f914bcc37574048bdeb463e6e9f9853547611e9a..98265fec230d36fdd35e07200c0eb6f3a3adaa0d 100644 --- a/make/lilypond-book-rules.make +++ b/make/lilypond-book-rules.make @@ -43,6 +43,7 @@ $(outdir)/%.pdf: $(outdir)/%.tex ifeq ($(USE_EXTRACTPDFMARK),yes) $(EXTRACTPDFMARK) -o $(outdir)/$*.pdfmark $(outdir)/$*.build/$*.pdf $(GS920) -dBATCH \ + -dNOSAFER \ -dNOPAUSE \ $(LILYPOND_BOOK_GS_QUIET) \ -sDEVICE=pdfwrite \ @@ -94,6 +95,7 @@ $(outdir)/%.pdf: $(outdir)/%.xml ifeq ($(USE_EXTRACTPDFMARK),yes) $(EXTRACTPDFMARK) -o $(outdir)/$*.pdfmark $(outdir)/$*.tmp.pdf $(GS920) -dBATCH \ + -dNOSAFER \ -dNOPAUSE \ $(LILYPOND_BOOK_GS_QUIET) \ -sDEVICE=pdfwrite \ Index: stepmake/stepmake/texinfo-rules.make diff --git a/stepmake/stepmake/texinfo-rules.make b/stepmake/stepmake/texinfo-rules.make index f6398ae05c80b6d3d12e2e5c191420ce13844c69..26b392fc68282394f0da14111254aea6bea6c05a 100644 --- a/stepmake/stepmake/texinfo-rules.make +++ b/stepmake/stepmake/texinfo-rules.make @@ -97,6 +97,7 @@ $(outdir)/%.pdf: $(outdir)/%.texi $(outdir)/version.itexi $(outdir)/%.pdf.omf $( ifeq ($(USE_EXTRACTPDFMARK),yes) $(EXTRACTPDFMARK) -o $(outdir)/$*.pdfmark $(outdir)/$*.tmp.pdf $(GS920) -dBATCH \ + -dNOSAFER \ -dNOPAUSE \ $(TEXINFO_GS_QUIET) \ -sDEVICE=pdfwrite \
Tiny fixes for "Extract PDFmark" (issue 571420055 by jonas.hahnf...@gmail.com)
LGTM, thanks https://codereview.appspot.com/571420055/diff/571400046/configure.ac File configure.ac (right): https://codereview.appspot.com/571420055/diff/571400046/configure.ac#newcode391 configure.ac:391: ["(Using Ghostscript >= 9.20 together with Extract PDFmark"]) I suggest to use the program's name. s/Extract PDFmark/extractpdfmark/ https://codereview.appspot.com/571420055/
Re: gub fail/local-build-script/new bug?
Thomas Morley writes: > Am Fr., 24. Jan. 2020 um 22:28 Uhr schrieb Thomas Morley > : >> >> Am Fr., 24. Jan. 2020 um 22:16 Uhr schrieb Dan Eble : >> > >> > On Jan 24, 2020, at 15:13, David Kastrup wrote: >> > >> > >> > Thomas Morley writes: >> > >> > ... >> > >> > In member function 'std::string Interval_t::to_string() const': >> > /home/hermann/gub/target/freebsd-x86/src/lilypond-git.sv.gnu.org--lilypond.git-master/flower/include/interval.tcc:142:14: >> > error: 'std::to_string' has not been declared >> > >> > ... >> > >> > Probably: >> > >> > commit 9cf6b20aefd79be13c7678d4cc834434b7ca000d >> > Author: Dan Eble >> > Date: Sat Jan 11 08:55:36 2020 -0500 >> > >> >Issue 5659/8: Remove Interval_t::T_to_string () >> > >> > >> > This is probably not the root cause, for though this tries to use >> > std::to_string(), the reason it does so is because several >> > preceding commits that removed ::to_string() overloads that were >> > duplicating the function of std::to_string(). I think if you >> > reverted just this commit, you'd hit an undefined std::to_string() >> > elsewhere. >> > >> > Our current source code assumes more or less C++11 I think, and GUB >> > compilers might not be up to it? >> > >> > >> > This seems likely. (Have I mentioned how tiresome GUB is? I know >> > it's been a while since anyone complained about it.) >> > >> > Can we upgrade GUB, or should I start working to restore the >> > global ::to_string() functions? Newer systems are better off with >> > things the way they are, but I don't see a third option. >> > — >> > Dan >> > >> >> Well, I can't do any reasonable GUB-fixing, it's out of my depth. >> >> Also, your relevant patches are out of my depth as well. Though, >> nobody objected during review, thus I think we should keep them. >> >> But I happily test things :) >> Right now I try a GUB-build from a local branch containing nothing >> else than already released 2.19.83. (with said gublbb and most recent >> GUB) >> If it fails (it worked half a year ago), than I suspect a GUB-problem. >> And I may try to bisect GUB, although this will be very tedious... >> If success I may try going lilypond-upstream. >> >> Cheers, >> Harm > > This GUB-build succeded. > Since I used the most up to date GUB, I suspect the encountered > problem somewhere in LilyPond not in GUB. > I'll first make a patched windows-installer (see above) and then I'll > try to go upstream. Dan just a few days ago committed a change requiring C++11 to work. I thought we called g++ using -std=c++11 options, but maybe that does still not work with older compiler/library versions? -- David Kastrup
Re: french beaming incorrectly makes stems longer
On Sat, Jan 25, 2020 at 09:27:07AM +0100, Thomas Morley wrote:
> Am Sa., 25. Jan. 2020 um 08:58 Uhr schrieb Kevin Barry :
> >
> > The minimum length of the stem of the middle note is forcing the beams down.
>
> Not here. It's more the relevant value of
> beamed-extreme-minimum-free-lengths. See:
>
> \relative c'' {
> r16 f d f
> \override Stem.details.beamed-extreme-minimum-free-lengths =
> #'(2.0 0.8) %% original: '(2.0 1.25)
> \override Stem.french-beaming = ##t
> r16 f d f
> }
OK I see. It seems the value is hard coded now, but in the case of
French beaming it would need to be different depending on how many beams
there are (the above code works for semiquavers, but I think the value
needs to be smaller for demisemiquavers).
I am happy to try patching it, but I wouldn't know where to start.
>
> Cheers,
> Harm
Re: gub fail/local-build-script/new bug?
Am Fr., 24. Jan. 2020 um 22:28 Uhr schrieb Thomas Morley : > > Am Fr., 24. Jan. 2020 um 22:16 Uhr schrieb Dan Eble : > > > > On Jan 24, 2020, at 15:13, David Kastrup wrote: > > > > > > Thomas Morley writes: > > > > ... > > > > In member function 'std::string Interval_t::to_string() const': > > /home/hermann/gub/target/freebsd-x86/src/lilypond-git.sv.gnu.org--lilypond.git-master/flower/include/interval.tcc:142:14: > > error: 'std::to_string' has not been declared > > > > ... > > > > Probably: > > > > commit 9cf6b20aefd79be13c7678d4cc834434b7ca000d > > Author: Dan Eble > > Date: Sat Jan 11 08:55:36 2020 -0500 > > > >Issue 5659/8: Remove Interval_t::T_to_string () > > > > > > This is probably not the root cause, for though this tries to use > > std::to_string(), the reason it does so is because several preceding > > commits that removed ::to_string() overloads that were duplicating the > > function of std::to_string(). I think if you reverted just this commit, > > you'd hit an undefined std::to_string() elsewhere. > > > > Our current source code assumes more or less C++11 I think, and GUB > > compilers might not be up to it? > > > > > > This seems likely. (Have I mentioned how tiresome GUB is? I know it's > > been a while since anyone complained about it.) > > > > Can we upgrade GUB, or should I start working to restore the global > > ::to_string() functions? Newer systems are better off with things the way > > they are, but I don't see a third option. > > — > > Dan > > > > Well, I can't do any reasonable GUB-fixing, it's out of my depth. > > Also, your relevant patches are out of my depth as well. Though, > nobody objected during review, thus I think we should keep them. > > But I happily test things :) > Right now I try a GUB-build from a local branch containing nothing > else than already released 2.19.83. (with said gublbb and most recent > GUB) > If it fails (it worked half a year ago), than I suspect a GUB-problem. > And I may try to bisect GUB, although this will be very tedious... > If success I may try going lilypond-upstream. > > Cheers, > Harm This GUB-build succeded. Since I used the most up to date GUB, I suspect the encountered problem somewhere in LilyPond not in GUB. I'll first make a patched windows-installer (see above) and then I'll try to go upstream. Cheers, Harm
Re: french beaming incorrectly makes stems longer
Am Sa., 25. Jan. 2020 um 08:58 Uhr schrieb Kevin Barry :
>
> The minimum length of the stem of the middle note is forcing the beams down.
Not here. It's more the relevant value of
beamed-extreme-minimum-free-lengths. See:
\relative c'' {
r16 f d f
\override Stem.details.beamed-extreme-minimum-free-lengths =
#'(2.0 0.8) %% original: '(2.0 1.25)
\override Stem.french-beaming = ##t
r16 f d f
}
Cheers,
Harm
Re: french beaming incorrectly makes stems longer
Am 25. Januar 2020 08:56:44 MEZ schrieb Werner LEMBERG : > >> What is "french beaming" supposed to do/be? > >No stems within the beam. Ah, yes. Sorry for forgetting that... > > >Werner -- Diese Nachricht wurde von meinem Android-Gerät mit K-9 Mail gesendet.
Re: french beaming incorrectly makes stems longer
> The minimum length of the stem of the middle note is forcing the > beams down. Sounds sensible. Can you provide a patch to fix that? The minimum stem length for french beams must be handled identically to the non-french case. Werner
