Re: Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

2020-05-09 Thread dak
On 2020/05/09 16:19:59, hanwenn wrote: > On Sat, May 9, 2020 at 4:56 PM <mailto:d...@gnu.org> wrote: > > > > On 2020/05/09 12:00:01, hanwenn wrote: > > > On 2020/05/09 11:03:12, dak wrote: > > > > Rationale? This negates work done as part of issue 5347 wi

Re: Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

2020-05-09 Thread dak
On 2020/05/09 12:00:01, hanwenn wrote: > On 2020/05/09 11:03:12, dak wrote: > > Rationale? This negates work done as part of issue 5347 with the long-term > goal > > of making transforms a better integrated and accessible part of stencils. > This > > will be increasi

Stop smobifying Transform (issue 567580043 by hanw...@gmail.com)

2020-05-09 Thread dak
Rationale? This negates work done as part of issue 5347 with the long-term goal of making transforms a better integrated and accessible part of stencils. This will be increasingly important when we move to Cairo since Cairo cannot represent rotations by multiples of 90 degrees cleanly other than

Re: define-markup-command-internal -> module-define-markup-command! (issue 547920045 by d...@gnu.org)

2020-05-08 Thread dak
Reviewers: hanwenn, Message: On 2020/05/08 17:12:41, hanwenn wrote: > I don't understand how this approach could ever help byte-compiling the markup > scheme files. This still uses module-define! , so the guile2 compilation step > will be oblivious to markup functions. It makes it easier to crea

Re: Split glyph contours in up/down segments for skylines (issue 569700043 by hanw...@gmail.com)

2020-05-08 Thread dak
On 2020/05/08 14:14:47, hahnjo wrote: > assert(false) that none of the previous cases was true? That should detect a > breaking change in FT_Outline pretty quickly. else if (condition) { ... } else assert(false); seems to make the assertion output comparatively useless. Wouldn't it be better to

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-03 Thread dak
On 2020/05/03 10:46:49, hahnjo wrote: > On 2020/05/03 10:40:25, dak wrote: > > On 2020/05/03 09:46:56, hahnjo wrote: > > > rebase + allow -dgs-api=#f to still fork > > > > I decided to look up the terms of the Affero GPL version 3. The relevant > > difference

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-03 Thread dak
On 2020/05/02 10:22:15, hahnjo wrote: > https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc > File lily/general-scheme.cc (right): > > https://codereview.appspot.com/548030043/diff/559960055/lily/general-scheme.cc#newcode778 > lily/general-scheme.cc:778: free (a); > On 2

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-03 Thread dak
On 2020/05/03 09:46:56, hahnjo wrote: > rebase + allow -dgs-api=#f to still fork I decided to look up the terms of the Affero GPL version 3. The relevant differences are: > Notwithstanding any other provision of this License, if you modify the Program, your modified version must prominently offe

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-02 Thread dak
On 2020/05/02 09:49:44, hahnjo wrote: > On 2020/05/01 06:28:56, hanwenn wrote: > > I suggest we make this an option that you have enable explicitly. > > done > > > If it is enabled, we'd have to change the --license output to say AGPL as > well. > > I thought about this and decided against it: >

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 22:19:38, hanwenn wrote: > On Fri, May 1, 2020 at 11:58 PM <mailto:v.villen...@gmail.com> wrote: > > > > On 2020/05/01 12:12:40, dak wrote: > > > That being said, the situation regarding Scorio, a proprietary entity > > using Free > > >

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 11:44:05, hahnjo wrote: > On 2020/05/01 11:41:13, dak wrote: > > On 2020/05/01 11:18:11, hahnjo wrote: > > > > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc > > > File lily/general-scheme.cc (right): > > > >

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 11:18:11, hahnjo wrote: > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc > File lily/general-scheme.cc (right): > > https://codereview.appspot.com/548030043/diff/583830043/lily/general-scheme.cc#newcode783 > lily/general-scheme.cc:783: command += "(" +

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 11:15:29, hanwenn wrote: > On 2020/04/30 07:58:09, dak wrote: > > On 2020/04/30 07:42:16, hahnjo wrote: > > > On 2020/04/27 11:58:11, dak wrote: > > > > Tracker issue: 5946 > (https://sourceforge.net/p/testlilyissues/issues/5946/) > &g

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-05-01 Thread dak
On 2020/05/01 07:58:32, hahnjo wrote: > disclaimer: I'm not a lawyer, this is just my understanding of the licenses. > > On 2020/05/01 06:28:56, hanwenn wrote: > > Technologically speaking, this is a brilliant idea, and I am all in favor it. > > > > However, I think we can't enable this by defaul

Re: Use GhostScript API instead of forking (issue 548030043 by jonas.hahnf...@gmail.com)

2020-04-30 Thread dak
On 2020/04/30 19:15:23, hahnjo wrote: > This probably goes without saying, but touching such core functionality should > receive very rigid testing. I hope I got most things covered, but especially > manual feedback would be very much appreciated! Sounds pretty good. A great long-term perspective

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-04-30 Thread dak
On 2020/04/30 07:42:16, hahnjo wrote: > On 2020/04/27 11:58:11, dak wrote: > > Tracker issue: 5946 (https://sourceforge.net/p/testlilyissues/issues/5946/) > > Rietveld issue: 577840053 (https://codereview.appspot.com/577840053) > > Issue description: > > Use Sc

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-04-28 Thread dak
Regarding the "Get rid of unused function ly:lexer-keywords": that one was added with commit 1ae6f5710e714f13da1bfbfbd840835316618d1f Author: Han-Wen Nienhuys Date: Thu Dec 14 14:10:56 2006 +0100 add ly:lexer-keywords to make keyword extraction easier and exact. but there appears to be no

Issue 5934: remove Repeated_music::folded_music_length (issue 561670043 by nine.fierce.ball...@gmail.com)

2020-04-28 Thread dak
Interesting. In 2.18 it appears to be used as length-callback in TremoloRepeatedMusic. Was that wrong or did something change in unfolded_music_length ? https://codereview.appspot.com/561670043/

Re: Use Scheme_hash_table for keyword handling (issue 577840053 by d...@gnu.org)

2020-04-27 Thread dak
https://codereview.appspot.com/577840053/diff/561760044/lily/lexer.ll File lily/lexer.ll (right): https://codereview.appspot.com/577840053/diff/561760044/lily/lexer.ll#newcode928 lily/lexer.ll:928: // use more SCM for this. On 2020/04/27 23:24:19, Dan Eble wrote: > Is this what you've just done,

Re: Use Scheme_hash_table for keyword handling (issue 577840053 by d...@gnu.org)

2020-04-27 Thread dak
On 2020/04/27 23:07:17, dak wrote: > On 2020/04/27 22:57:06, dak wrote: > > Move keytable init out of constructor. Frankly, I don't consider this an > > improvement. > > As anecdotal data, it took me about as long to get this working as the whole > other patch, and I

Re: Use Scheme_hash_table for keyword handling (issue 577840053 by d...@gnu.org)

2020-04-27 Thread dak
On 2020/04/27 22:57:06, dak wrote: > Move keytable init out of constructor. Frankly, I don't consider this an > improvement. As anecdotal data, it took me about as long to get this working as the whole other patch, and I had it segfault about half a dozen times and had another

Re: Use Scheme_hash_table for keyword handling (issue 577840053 by d...@gnu.org)

2020-04-27 Thread dak
https://codereview.appspot.com/577840053/diff/561760044/lily/lily-lexer.cc File lily/lily-lexer.cc (right): https://codereview.appspot.com/577840053/diff/561760044/lily/lily-lexer.cc#newcode97 lily/lily-lexer.cc:97: Protected_scm Lily_lexer::keytable_; On 2020/04/27 21:22:59, hanwenn wrote: > co

Re: Thread skyline construction through stencil interpretation (issue 581960043 by hanw...@gmail.com)

2020-04-27 Thread dak
That's the kind of stuff that warrants compiling a few scores with -ddebug-skylines and looking rather closely for kinks. https://codereview.appspot.com/581960043/diff/561710044/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/581960043/diff/56171004

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-04-27 Thread dak
On 2020/04/27 09:11:16, dak wrote: > Han-Wen Nienhuys <mailto:hanw...@gmail.com> writes: > > > On Mon, Apr 27, 2020 at 10:59 AM <mailto:d...@gnu.org> wrote: > >> When the parser sees some \blabla it will generally first have to check > >> for a keyword a

Re: Use a hash table for the lexer keywords (issue 549920043 by hanw...@gmail.com)

2020-04-27 Thread dak
Without having looked all that much at the code: When the parser sees some \blabla it will generally first have to check for a keyword and (when it has no match) afterwards for a variable with that name, and a lot of those are actually music functions these days that used to be keyword. Wouldn't

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

2020-04-26 Thread dak
On 2020/04/24 13:15:10, dak wrote: > mailto:hanw...@gmail.com writes: > > > Here is how I have experienced this discussion: > > > > DAK: this is micro-optimization that causes memory fragmentation. > > > > HW: No, it's not; here is a benchmark th

Re: Add a script for running timing benchmarks (issue 545950043 by hanw...@gmail.com)

2020-04-25 Thread dak
On 2020/04/25 17:07:17, hahnjo wrote: > I strongly object to adding more random scripts to the source tree. There are > already far too many unmaintained in scripts/auxiliar/ with no documentation at > all. How about approaching this in a different manner then? Adding instructions to the CG about

Re: Fix warnings related to po (issue 551830043 by jonas.hahnf...@gmail.com)

2020-04-25 Thread dak
On 2020/04/25 08:36:02, hahnjo wrote: > On 2020/04/24 20:14:36, hahnjo wrote: > > On 2020/04/24 18:25:33, lemzwerg wrote: > > > LGTM, thanks a lot! > > > > > > https://codereview.appspot.com/551830043/diff/573740043/po/GNUmakefile > > > File po/GNUmakefile (right): > > > > > > > > > https://coder

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

2020-04-25 Thread dak
On 2020/04/25 07:29:06, hanwenn wrote: > On 2020/04/24 22:04:52, dak wrote: > > On 2020/04/24 21:33:12, Carl wrote: > > > On 2020/04/24 21:19:57, dak wrote: > > > > On 2020/04/24 21:18:12, dak wrote: > > > > > > > > > > > > > >

Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

2020-04-24 Thread dak
On 2020/04/19 22:35:39, dak wrote: > On 2020/04/19 21:43:26, hanwenn wrote: > > (this still needs some work, but the speedup might be worth an early look) > > * Buildings store Y coordinate of the left edge, rather than the > intercept at x==0.0. This avoid excessive roundi

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

2020-04-24 Thread dak
On 2020/04/24 21:33:12, Carl wrote: > On 2020/04/24 21:19:57, dak wrote: > > On 2020/04/24 21:18:12, dak wrote: > > > > > > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc > > > File lily/stencil-integral.

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

2020-04-24 Thread dak
https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc#newcode465 lily/stencil-integral.cc:465: // more convoluted, but it's fairly hot path. Sorry fo

Re: Minor cleanups in stencil-integral.cc (issue 579630043 by hanw...@gmail.com)

2020-04-24 Thread dak
On 2020/04/24 21:18:12, dak wrote: > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc > File lily/stencil-integral.cc (right): > > https://codereview.appspot.com/579630043/diff/555740043/lily/stencil-integral.cc#newcode465 > lily/stencil-integral.

Re: make_draw_bezier_boxes: save work if thickness == 0.0 (issue 551730043 by hanw...@gmail.com)

2020-04-24 Thread dak
https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/551730043/diff/583770043/lily/stencil-integral.cc#newcode496 lily/stencil-integral.cc:496: break; I think that is less readable than desirable.

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

2020-04-23 Thread dak
On 2020/04/23 16:01:20, dak wrote: > On 2020/04/22 07:43:17, hanwenn wrote: > > * The linked list has an overhead of 2 pointers, on a 4x Real > > datastructure, ie. 50% overhead. By the way: C++11 has Forward_list which would be another plugin container type with less overhead.

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

2020-04-23 Thread dak
On 2020/04/22 07:43:17, hanwenn wrote: > current master: > > 4f906b95aea99f2a47d5ba037f7421e5bb933c42 (origin/master) Issue 5908: > get_path_list: use loop instead of tail recursion > > Command being timed: "out/bin/lilypond.4f906b95ae -I carver MSDM" > User time (seconds): 34.30 > Maximum reside

Re: Use vectors rather than lists for skylines. (issue 583750043 by hanw...@gmail.com)

2020-04-21 Thread dak
I am rather skeptical about the usefulness of such microoptimizations without influence on algorithmic complexity. In return for better memory locality you buy into quite larger memory fragmentation, and we have scores of comparatively modest size already exhausting memory. All that exhausted mem

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-20 Thread dak
On 2020/04/21 00:43:02, dak wrote: > On 2020/04/20 23:26:07, Dan Eble wrote: > > On 2020/04/20 19:36:39, dak wrote: > > > Polish and extend tools in callback.hh > > > > LGTM. > > > > I have a hunch that the MFPn_WRAP macros could be turned into some

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-20 Thread dak
On 2020/04/20 23:26:07, Dan Eble wrote: > On 2020/04/20 19:36:39, dak wrote: > > Polish and extend tools in callback.hh > > LGTM. > > I have a hunch that the MFPn_WRAP macros could be turned into something more > typical of C++, but I don't blame you for resting at t

Issue #1204: fix font-name-add-files regtest (issue 573730044 by v.villen...@gmail.com)

2020-04-20 Thread dak
https://codereview.appspot.com/573730044/diff/583810043/input/regression/font-name-add-files.ly File input/regression/font-name-add-files.ly (right): https://codereview.appspot.com/573730044/diff/583810043/input/regression/font-name-add-files.ly#newcode25 input/regression/font-name-add-files.ly:

Re: Rewrite Skyline code (issue 547980044 by hanw...@gmail.com)

2020-04-19 Thread dak
On 2020/04/19 21:43:26, hanwenn wrote: > (this still needs some work, but the speedup might be worth an early look) * Buildings store Y coordinate of the left edge, rather than the intercept at x==0.0. This avoid excessive rounding problems when X is large, and lets us compute building.height

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-19 Thread dak
On 2020/04/19 11:03:35, hahnjo wrote: > On 2020/04/19 10:55:52, dak wrote: > I have the vague > > impression that the ability to do template specialisation on typedefs would > also > > be able to solve part of the job here in a more elegant manner but right now > my >

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-19 Thread dak
On 2020/04/19 09:55:03, hahnjo wrote: > On 2020/04/19 09:39:00, dak wrote: > > > https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh > > File lily/include/callback.hh (right): > > > > > https://codereview.appspot.com/551780043/diff/5

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-19 Thread dak
https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh File lily/include/callback.hh (right): https://codereview.appspot.com/551780043/diff/573720045/lily/include/callback.hh#newcode166 lily/include/callback.hh:166: typedef typename remove_pointer (nullptr)))>::type typ

Re: Obviate method_finder methods (issue 551780043 by d...@gnu.org)

2020-04-19 Thread dak
Reviewers: lemzwerg, Message: On 2020/04/19 05:53:34, lemzwerg wrote: > Nice! I don't really understand the C++ wizardry, but having to type less > macros in the normal code is certainly beneficial. The wizardry is comparatively confined and mainly consists of a more polished version of the mfp_

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 14:43:24, Dan Eble wrote: > On 2020/04/18 14:31:33, dak wrote: > > If we consider type_traits as a sunk cost, I'll see whether I can find > anything > > in it to address the inheritance wart you complained of. > > As you already put in some time there,

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
https://codereview.appspot.com/549890043/diff/573710043/lily/global-context.cc File lily/global-context.cc (right): https://codereview.appspot.com/549890043/diff/573710043/lily/global-context.cc#newcode49 lily/global-context.cc:49: event_source ()->add_listener (GET_LISTENER (static_cast (this),

Re: get_path_list: use loop instead of tail recursion (issue 579570046 by d...@gnu.org)

2020-04-18 Thread dak
Reviewers: hanwenn, Message: On 2020/04/17 09:12:18, hanwenn wrote: > LGTM > > https://codereview.appspot.com/579570046/diff/579580044/lily/stencil-integral.cc > File lily/stencil-integral.cc (right): > > https://codereview.appspot.com/579570046/diff/579580044/lily/stencil-integral.cc#newcode111

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 10:30:27, hanwenn wrote: > https://codereview.appspot.com/549890043/diff/579610043/lily/include/listener.hh > File lily/include/listener.hh (left): > > https://codereview.appspot.com/549890043/diff/579610043/lily/include/listener.hh#oldcode140 > lily/include/listener.hh:140: #define

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 10:32:50, lemzwerg wrote: > > I agree with the sentiment, but I fail to come up > > with a naming choice that does not make the cure > > worse than the problem. > > I would simply use GET_LISTENERX (or GET_LISTENER1) for the longer call. That's being obtuse. Requires learning two t

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 10:26:01, dak wrote: > On 2020/04/18 05:40:53, lemzwerg wrote: > > > What about using two macros instead of one? The first would take a context as a > > first argument (as it does now), the second one uses 'this' in the macro body, > > omitting tha

Re: Make GET_LISTENER take a pointer arg and deduce its type (issue 549890043 by d...@gnu.org)

2020-04-18 Thread dak
On 2020/04/18 05:40:53, lemzwerg wrote: > LGTM - but I don't know the internals well enough to really decide that. It's really not as much about the internals rather than about how to express their use. GET_LISTENER provides an SCM-callable memoized function port into a C++ member function. The

Re: Use Interval for representing skyline X coordinates (issue 571980043 by hanw...@gmail.com)

2020-04-16 Thread dak
Purpose is not clear since the code does not appear to use anything other than the explicit endpoints. Also, intervals are indexed by LEFT/RIGHT rather than START/STOP. Values may be the same, but the former is a whole lot more mnemonic. https://codereview.appspot.com/571980043/

Re: Rewrite Mutable_properties based on vector (issue 575990043 by hanw...@gmail.com)

2020-04-14 Thread dak
On 2020/04/13 17:43:17, hanwenn wrote: > > Actually, it looks like I have been measuring noise. Thermal throttling? > > It looks like I can get reproducible timings if I downclock the CPU from 2.6 to > 2.0Ghz Put the following line in the config /etc/modprobe.d/acpi.conf:options thinkpad-acpi

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-13 Thread dak
On 2020/04/13 23:07:57, Dan Eble wrote: > This change per se LGTM. I remember discussing this syntactic change briefly on > the list a few(?) months ago, so this is not surprising. > > I'm quite pleased with this change, actually. I remember how I felt the first > time I came across klass->a_mac

Re: Rewrite Mutable_properties based on vector (issue 575990043 by hanw...@gmail.com)

2020-04-13 Thread dak
On 2020/04/13 21:13:42, hanwenn wrote: > > > > You need to sort the C++ structures potentially accessed by the GC hook into a > > Preinit class (grep for Preinit for existing uses) and inherit from that > before > > inheriting from whatever operates the Smob. Then C++ guarantees an > > initialis

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread dak
On 2020/04/13 20:27:34, hanwenn wrote: > On 2020/04/13 20:10:35, dak wrote: > > > https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh > > File lily/include/mutable-properties.hh (right): > > > > > https://codereview.appspot.

Re: Rewrite Mutable_properties based on vector (issue 575990043 by hanw...@gmail.com)

2020-04-13 Thread dak
On 2020/04/13 20:21:29, dak wrote: > https://codereview.appspot.com/575990043/diff/577740043/lily/mutable-properties.cc#newcode109 > lily/mutable-properties.cc:109: Mutable_properties::mark () const > This can segfault in GUILEv2 and/or depending on some other circumstances sinc

Re: Rewrite Mutable_properties based on vector (issue 575990043 by hanw...@gmail.com)

2020-04-13 Thread dak
https://codereview.appspot.com/575990043/diff/577740043/lily/mutable-properties.cc File lily/mutable-properties.cc (right): https://codereview.appspot.com/575990043/diff/577740043/lily/mutable-properties.cc#newcode50 lily/mutable-properties.cc:50: if (SCM_EQ_P (key (i), k)) scm_is_eq https://co

Re: Abstract Grob property storage into Mutable_properties. (issue 561640043 by hanw...@gmail.com)

2020-04-13 Thread dak
https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh File lily/include/mutable-properties.hh (right): https://codereview.appspot.com/561640043/diff/565900043/lily/include/mutable-properties.hh#newcode31 lily/include/mutable-properties.hh:31: class Iterator {

Re: Move get_normal to Offset::normal (issue 576000043 by hanw...@gmail.com)

2020-04-13 Thread dak
.hh:123: Offset normal() const { > On 2020/04/13 18:24:47, dak wrote: > > It's kind of unusual to make a constructing function a (non-static) member > > function unless this is necessitated for operator semantics, because different > > conversion rules apply and there

Move get_normal to Offset::normal (issue 576000043 by hanw...@gmail.com)

2020-04-13 Thread dak
https://codereview.appspot.com/57643/diff/577750046/flower/include/offset.hh File flower/include/offset.hh (right): https://codereview.appspot.com/57643/diff/577750046/flower/include/offset.hh#newcode123 flower/include/offset.hh:123: Offset normal() const { It's kind of unusual to make a

Optimize get_path_list. (issue 579570043 by hanw...@gmail.com)

2020-04-13 Thread dak
There is scm_c_memq for avoiding hare&tortoise, but for this use case, the explicit code seems like the saner option. Even without hare&tortoise, the list would be consed together each time without further measures. https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral.cc

Re: Use composition for embedding PangoMatrix in Transform (issue 555610043 by hanw...@gmail.com)

2020-04-13 Thread dak
"PangoMatrix is a struct, so there is no value in using inheritance" is a non-sequitur. A struct just is a class with public members. The motivation for choosing inheritance here was that this allows passing Transform directly into Pango for operations. This advantage is not realised in our curr

Re: Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

2020-04-12 Thread dak
On 2020/04/12 10:59:21, hanwenn wrote: > On 2020/03/30 12:20:17, dak wrote: > > > > Ugly and a maintenance burden since the code is used twice. Anything > > > > wrong with my proposal? > > > > > > I didn't understand your proposal. >

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-12 Thread dak
On 2020/04/12 10:35:24, hanwenn wrote: > On Sat, Apr 11, 2020 at 3:55 PM David Kastrup wrote: > > >> You haven't understood the scheme. A grob only contains the properties > > >> seminal to its type. That's why there is a double indirection. A full > > >> array of 416 entri

Re: Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
On 2020/04/11 17:44:11, hanwenn wrote: > On 2020/04/11 17:19:19, dak wrote: > > > I thought of removing the other function too, and I agree in principle, but > it > > > can only break more user files. As long as we're not considering > reorganizing > > >

Re: Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
On 2020/04/11 16:38:28, hanwenn wrote: > On 2020/04/11 16:02:31, dak wrote: > > The description says "[ly:grob-properties] will not return the properties that > > were \overridden." > > > > Aren't you confusing this with ly:grob-basic-properties ? I

Re: Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
On 2020/04/11 16:32:58, thomasmorley651 wrote: > On 2020/04/11 16:31:48, thomasmorley651 wrote: > > > Btw, it's used in > > input/regression/multi-measure-rest-reminder.ly > > input/regression/scheme-text-spanner.ly > > as well. > > No, that's ly:grob-properties? > Bad grepping ... Also icky nam

Re: Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
The description says "[ly:grob-properties] will not return the properties that were \overridden." Aren't you confusing this with ly:grob-basic-properties ? I think ly:grob-properties will actually _only_ return the properties that were overriden. https://codereview.appspot.com/549840044/

Remove ly:grob-properties (issue 549840044 by hanw...@gmail.com)

2020-04-11 Thread dak
https://codereview.appspot.com/549840044/diff/567430043/lily/grob-scheme.cc File lily/grob-scheme.cc (left): https://codereview.appspot.com/549840044/diff/567430043/lily/grob-scheme.cc#oldcode327 lily/grob-scheme.cc:327: LY_DEFINE (ly_grob_basic_properties, "ly:grob-basic-properties", There is r

Re: Reimplement Scheme_hash_table using linear probing. (issue 559790043 by hanw...@gmail.com)

2020-04-11 Thread dak
On 2020/04/11 11:36:16, hanwenn wrote: > On Sat, Apr 11, 2020 at 1:05 PM wrote: > > > > On 2020/04/11 05:37:39, hanwenn wrote: > > > > In addition, I don't think that it is used to a degree where it > > would > > > significantly affect LilyPond's performance. > >

Re: Reimplement Scheme_hash_table using linear probing. (issue 559790043 by hanw...@gmail.com)

2020-04-11 Thread dak
https://codereview.appspot.com/559790043/diff/563830043/input/regression/scheme-unit-test.ly#newcode6 > input/regression/scheme-unit-test.ly:6: #(ly:test-scheme-hash-table) > On 2020/04/10 21:43:28, dak wrote: > > This seems like a rather opaque way of testing functionality. > &g

Re: Fix line comments for new snippets (issue 549820043 by d...@gnu.org)

2020-04-11 Thread dak
https://codereview.appspot.com/549820043/diff/565880043/scripts/auxiliar/makelsr.py File scripts/auxiliar/makelsr.py (right): https://codereview.appspot.com/549820043/diff/565880043/scripts/auxiliar/makelsr.py#newcode38 scripts/auxiliar/makelsr.py:38: new_lys_marker = "%% generated from %s" % ne

Reimplement Scheme_hash_table using linear probing. (issue 559790043 by hanw...@gmail.com)

2020-04-10 Thread dak
I don't see the point in reinventing the wheel. This functionality is provided both by Guile (where an improved implementation could be submitted) and C++'s STL. In addition, I don't think that it is used to a degree where it would significantly affect LilyPond's performance. Reworking this to a

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-09 Thread dak
On 2020/04/09 17:07:46, hanwenn wrote: > I'm curious about these optimizations. Can you say more? Properties are currently stored in alists. They can be stored in vectors instead. Give all grob properties a number, then have an array per grob type mapping this number to an index into an array.

Re: Fix line comments for new snippets (issue 549820043 by d...@gnu.org)

2020-04-06 Thread dak
Reviewers: lemzwerg, Message: On 2020/04/06 12:40:50, lemzwerg wrote: > LGTM, and please push directly. I'd rather not have 2.21.0 in a state inconsistent with its makelsr program. So if I were to push, I'd need to push the results of a new makelsr run as well. Description: Fix line comments fo

Re: Remove all uses of markup macro from scm/*.scm (issue 575930043 by d...@gnu.org)

2020-04-03 Thread dak
On 2020/04/03 22:22:31, hanwenn wrote: > note that this still doesn't fix the next error message for compilation which is > > > > fatal error: Not a markup command: line > > which is triggered by the *definition* of the markup macro: > > (defmacro*-public markup (#:rest body) > .. > (car (co

Re: output-distance: write HTML as UTF-8 (issue 563810043 by hanw...@gmail.com)

2020-04-03 Thread dak
https://codereview.appspot.com/563810043/diff/545810044/scripts/build/output-distance.py File scripts/build/output-distance.py (right): https://codereview.appspot.com/563810043/diff/545810044/scripts/build/output-distance.py#newcode1328 scripts/build/output-distance.py:1328: system (u'echo HEAD

output-distance: write HTML as UTF-8 (issue 563810043 by hanw...@gmail.com)

2020-04-03 Thread dak
Is this likely related to the problems in `make check` that James currently experiences? https://codereview.appspot.com/563810043/

Re: mf: use python scripting for generating Emmentaler fonts (issue 553580043 by hanw...@gmail.com)

2020-04-02 Thread dak
On 2020/04/02 21:09:57, thomasmorley651 wrote: > On 2020/04/02 21:04:06, dak wrote: > > On 2020/04/02 20:32:39, hanwenn wrote: > > > Jonas, what is the status of 2.21.0 ? > > > > I wanted to give Phil the go-ahead tomorrow, once some backdated convert-ly > rule &g

Re: mf: use python scripting for generating Emmentaler fonts (issue 553580043 by hanw...@gmail.com)

2020-04-02 Thread dak
On 2020/04/02 20:32:39, hanwenn wrote: > Jonas, what is the status of 2.21.0 ? I wanted to give Phil the go-ahead tomorrow, once some backdated convert-ly rule is in. Probably depends on how well the LSR import works whether this gets out then. https://codereview.appspot.com/553580043/

Re: Refactor \markup \pattern (issue 549780045 by d...@gnu.org)

2020-03-31 Thread dak
Reviewers: hanwenn, https://codereview.appspot.com/549780045/diff/30045/scm/define-markup-commands.scm File scm/define-markup-commands.scm (right): https://codereview.appspot.com/549780045/diff/30045/scm/define-markup-commands.scm#newcode4636 scm/define-markup-commands.scm:4636: (space (

Re: Remove all uses of markup macro from scm/*.scm (issue 575930043 by d...@gnu.org)

2020-03-30 Thread dak
https://codereview.appspot.com/575930043/diff/561620044/scm/time-signature.scm File scm/time-signature.scm (right): https://codereview.appspot.com/575930043/diff/561620044/scm/time-signature.scm#newcode23 scm/time-signature.scm:23: (mup (if (procedure? proc) On 2020/03/30 21:15:15, hanwenn wrote

Re: Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

2020-03-30 Thread dak
On 2020/03/29 14:35:43, hanwenn wrote: > On Sun, Mar 29, 2020 at 4:12 PM wrote: > > > > On 2020/03/29 13:55:41, hanwenn wrote: > > > On 2020/03/29 13:52:48, hanwenn wrote: > > > > retain existing > > > > > > how's this? This leaves things backward compatible. > > > > Ugly and

Re: Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

2020-03-29 Thread dak
On 2020/03/29 13:55:41, hanwenn wrote: > On 2020/03/29 13:52:48, hanwenn wrote: > > retain existing > > how's this? This leaves things backward compatible. Ugly and a maintenance burden since the code is used twice. Anything wrong with my proposal? It does not have duplicate code, makes define-

Re: Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

2020-03-29 Thread dak
On 2020/03/29 13:55:41, hanwenn wrote: > On 2020/03/29 13:52:48, hanwenn wrote: > > retain existing > > how's this? This leaves things backward compatible. > > Note that we're currently incoherent, because you can't do > > (let ((n 0)) (define sym val)) "incoherent" means in conflict with one

Re: Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

2020-03-28 Thread dak
On 2020/03/29 00:35:02, dak wrote: > Retaining define-markup-command-internal in order to allow defining markups in > LilyPond syntax in a manner that stops being supported from Scheme seems like > incoherent design. > > markup-lambdas can be created from within LilyPond using \

Define Scheme markups using define-public (issue 577720043 by hanw...@gmail.com)

2020-03-28 Thread dak
Retaining define-markup-command-internal in order to allow defining markups in LilyPond syntax in a manner that stops being supported from Scheme seems like incoherent design. markup-lambdas can be created from within LilyPond using \markup ... \etc . Having them unusable from Scheme for the purp

Re: Add dynamic-interface to keepAliveInterfaces (issue 553760043 by j...@abou-samra.fr)

2020-03-26 Thread dak
What I am worried about is a partitura that puts common dynamics on every staff, including staves without notes. That would keep those from being kept alive. So the question is whether we should not possibly create a different keepAliveInterfaces for the Dynamics context? Opinions? https://code

Re: Issue 5862: Prefer astyle 3.1 and update clang-format options (issue 551640043 by nine.fierce.ball...@gmail.com)

2020-03-26 Thread dak
On 2020/03/26 01:25:57, Dan Eble wrote: > It's worth emphasizing this: even with these improvements to the clang-format > configuration, those who use clang-format will introduce a truckload of > differences from the traditional indentation. Our own fixcc script introduces some modifications to na

Re: Remove trailing whitespace {python,scm,lily,scripts}. (issue 547810043 by hanw...@gmail.com)

2020-03-21 Thread dak
On 2020/03/21 14:17:03, dak wrote: > On 2020/03/21 13:14:50, hahnjo wrote: > > Sending to lilypond-devel for broader notice. This is likely to give > conflicts, > > maybe we can combine this with running fixcc.py? > > Ah right, that one was still pending anyway. I thi

Re: Remove trailing whitespace {python,scm,lily,scripts}. (issue 547810043 by hanw...@gmail.com)

2020-03-21 Thread dak
On 2020/03/21 13:14:50, hahnjo wrote: > Sending to lilypond-devel for broader notice. This is likely to give conflicts, > maybe we can combine this with running fixcc.py? Ah right, that one was still pending anyway. I think it would take care of trailing spaces in the C++ files. https://coderevi

Re: aclocal.m4: Support GUILE_CONFIG, document GUILE_FLAVOR (issue 575860044 by d...@gnu.org)

2020-03-21 Thread dak
https://codereview.appspot.com/575860044/diff/547790043/aclocal.m4 File aclocal.m4 (right): https://codereview.appspot.com/575860044/diff/547790043/aclocal.m4#newcode625 aclocal.m4:625: AC_ARG_VAR(GUILE_FLAVOR, AS_HELP_STRING([], On 2020/03/21 05:41:05, lemzwerg wrote: > What about breaking the

Re: aclocal.m4: Support GUILE_CONFIG, document GUILE_FLAVOR (issue 575860044 by d...@gnu.org)

2020-03-20 Thread dak
On 2020/03/20 14:54:36, hahnjo wrote: > Generally looks fine to me, you only might want to revisit indention: aclocal.m4 > uses 4 spaces instead of tabs. Uh, right. Done. https://codereview.appspot.com/575860044/

Re: scripts/build/scan-mf-deps: script to generate MF dependencies (issue 553700043 by hanw...@gmail.com)

2020-03-17 Thread dak
https://codereview.appspot.com/553700043/diff/567370043/mf/invoke-mf2pt.sh File mf/invoke-mf2pt.sh (right): https://codereview.appspot.com/553700043/diff/567370043/mf/invoke-mf2pt.sh#newcode4 mf/invoke-mf2pt.sh:4: realpath() { On 2020/03/17 07:41:25, hahnjo wrote: > AFAIK sh syntax would be > re

Re: Remove compat hack for Solaris7 and HP-UX (issue 579480047 by hanw...@gmail.com)

2020-03-15 Thread dak
https://codereview.appspot.com/579480047/diff/547750043/aclocal.m4 File aclocal.m4 (right): https://codereview.appspot.com/579480047/diff/547750043/aclocal.m4#newcode694 aclocal.m4:694: AC_PATH_PROG(BASH, bash, $SHELL) This sets BASH with a fallback to /bin/sh , meaning that $BASH is not guarant

Re: scripts/build/scan-mf-deps: script to generate MF dependencies (issue 553700043 by hanw...@gmail.com)

2020-03-14 Thread dak
Why would we want to move away from GNU make as a build system? https://codereview.appspot.com/553700043/

Re: Address output-distance problems: (issue 563730043 by hanw...@gmail.com)

2020-03-12 Thread dak
On 2020/03/12 09:52:31, hahnjo wrote: > On 2020/03/12 09:33:43, hahnjo wrote: > > On 2020/03/12 09:22:09, dak wrote: > > > On 2020/03/12 08:01:03, hahnjo wrote: > > > > This looks like bash-ism which might explain why it works for Han-Wen and > > me. > >

Re: Address output-distance problems: (issue 563730043 by hanw...@gmail.com)

2020-03-12 Thread dak
On 2020/03/12 08:01:03, hahnjo wrote: > On 2020/03/11 23:49:23, dak wrote: > > [...] > > GNU LilyPond 2.21.0 > > cp: cannot stat '19.sub{-*.signature,.ly,-1.eps,.log,.profile}': No such file > or > > directory > > test results in ./out/test-output

  1   2   3   4   5   6   7   8   9   10   >