Fixes footnote automatic numbering.

Fixes footnote automatic numbering.

[MikeSol, 2011-08-11 13:52:49]

This gets the numbering of automatic footnotes correct for systems with more
than one staff.

Cheers,
MS

[Reinhold, 2011-08-11 14:48:42]

Regtest missing.

Much better now, but with my example there are still some inconsistencies. In
particular, the ordering of the six footnotes in each of the systems is:

1st system:
2  3  5
1  4  6

2nd system (start at 7, shown here as 1):
1  4  5
2  3  6

3rd system (2nd page):
1  3  5
2  4  6

So, while the numbers are now sorted by moment, two footnotes at the same moment
are not consistently ordered.

http://codereview.appspot.com/4877041/diff/1/lily/system.cc
File lily/system.cc (right):

http://codereview.appspot.com/4877041/diff/1/lily/system.cc#newcode325
lily/system.cc:325: 
Remove the spaces from that empty line (git complains)

[mike_apollinemike.com, 2011-08-11 15:10:23]

On Aug 11, 2011, at 4:48 PM, reinhold.kainhofer@gmail.com wrote:

> Regtest missing.
> 
> Much better now, but with my example there are still some
> inconsistencies. In particular, the ordering of the six footnotes in
> each of the systems is:
> 
> 1st system:
> 2  3  5
> 1  4  6
> 
> 2nd system (start at 7, shown here as 1):
> 1  4  5
> 2  3  6
> 
> 3rd system (2nd page):
> 1  3  5
> 2  4  6
> 
> So, while the numbers are now sorted by moment, two footnotes at the
> same moment are not consistently ordered.
> 

I've put a new patchset up that should do the trick.
Before I add a regtest, I want to figure out why your example intersects with
the footnotes - that may be a separate bug that I can fix in this patch.

Cheers,
MS

[MikeSol, 2011-10-04 06:30:24]

Hey Reinhold et al,

This newest patch set should do the trick for automatic numbering.

Cheers,
MS

[pkx166h, 2011-10-04 06:48:56]

Passes make but fails make check. Cannot see where I just get

--snip--

job 0 terminated with signal: 11
fatal error: Children (0) exited with errors.
command failed: /home/jlowe/lilypond-git/build/out/bin/lilypond -I
/home/jlowe/lilypond-git/input/regression/ -I ./out-test -I
/home/jlowe/lilypond-git/input -I /home/jlowe/lilypond-git/Documentation -I
/home/jlowe/lily ...

etc.

--snip--

[mike_apollinemike.com, 2011-10-04 06:50:31]

On Oct 4, 2011, at 8:48 AM, pkx166h@gmail.com wrote:

> Passes make but fails make check. Cannot see where I just get
> 
> --snip--
> 
> job 0 terminated with signal: 11
> fatal error: Children (0) exited with errors.
> command failed: /home/jlowe/lilypond-git/build/out/bin/lilypond -I
> /home/jlowe/lilypond-git/input/regression/ -I ./out-test -I
> /home/jlowe/lilypond-git/input -I /home/jlowe/lilypond-git/Documentation
> -I /home/jlowe/lily ...
> 
> etc.
> 
> --snip--
> 

I just got the same thing - sorry about that.  Working on a fix.

Cheers,
MS

[pkx166h, 2011-10-05 06:58:06]

Passes make and make check looks ok, I get something on page
../regression/page-turn-page-breaking.log and I'm attaching the graphviz.log
output (as I don't know if it is significant). But nothing else pops up.

See http://code.google.com/p/lilypond/issues/detail?id=1791#c5

(ignore the pdf I mistakenly attached if it is still there on the tracker - I
did delete it - the png is the reg test).

[mike_apollinemike.com, 2011-10-05 07:10:09]

On Oct 5, 2011, at 8:58 AM, pkx166h@gmail.com wrote:

> Passes make and make check looks ok, I get something on page
> ../regression/page-turn-page-breaking.log and I'm attaching the
> graphviz.log output (as I don't know if it is significant). But nothing
> else pops up.
> 

Hey all,

I need to look into page-turn-page-breaking to see what's up there.  What's odd
is that the log changed w/o changing the file: usually, a new warning about page
overflow means that the visual output will be lower on the page, which is not
the case in the regtests.  What makes this even more bizarre is that the patch
ostensibly has nothing to do with this part of the code base (although I've
learned by now that things I think are totally unconnected often are in subtle
ways).  Anyway, thoughts would be appreciated!

Cheers,
MS

> See http://code.google.com/p/lilypond/issues/detail?id=1791#c5
> 
> (ignore the pdf I mistakenly attached if it is still there on the
> tracker - I did delete it - the png is the reg test).
> 
> http://codereview.appspot.com/4877041/

[pkx166h, 2011-10-14 07:30:11]

Hello, (still) getting

--snip--
@ -4,6 +4,8 @@
 Preprocessing graphical objects...
 Calculating page and line breaks (3 possible page breaks)...[1][2][3]
 Drawing systems...
+warning: cannot fit music on page: overflow is 0.351333
+warning: compressing music to fit
 Writing header field `texidoc' to
`/home/jlowe/lilypond-git/build/out/lybook-testdb/16/lily-4d2fa43b.texidoc'...
 Layout output to
`/home/jlowe/lilypond-git/build/out/lybook-testdb/16/lily-4d2fa43b.eps'...
 Layout output to
`/home/jlowe/lilypond-git/build/out/lybook-testdb/16/lily-4d2fa43b-1.eps'...

--snip-

on /input/regression/page-turn-page-breaking.log

and similar on

input/regression/page-breaking-end-of-score.log

James

[mike_apollinemike.com, 2011-10-14 10:41:21]

On Oct 14, 2011, at 9:30 AM, pkx166h@gmail.com wrote:

> Hello, (still) getting
> 
> --snip--
> @ -4,6 +4,8 @@
> Preprocessing graphical objects...
> Calculating page and line breaks (3 possible page breaks)...[1][2][3]
> Drawing systems...
> +warning: cannot fit music on page: overflow is 0.351333
> +warning: compressing music to fit
> Writing header field `texidoc' to
> `/home/jlowe/lilypond-git/build/out/lybook-testdb/16/lily-4d2fa43b.texidoc'...
> Layout output to
> `/home/jlowe/lilypond-git/build/out/lybook-testdb/16/lily-4d2fa43b.eps'...
> Layout output to
> `/home/jlowe/lilypond-git/build/out/lybook-testdb/16/lily-4d2fa43b-1.eps'...
> 
> --snip-
> 
> on /input/regression/page-turn-page-breaking.log
> 
> and similar on
> 
> input/regression/page-breaking-end-of-score.log
> 
> James


James - if you have a minute, could you please run page-turn-breaking.ly with
this patch applied and with the regtest headers included:

%% Generated by lilypond-book.py
%% Options: [exampleindent=10.16\mm,indent=0\mm,line-width=160\mm]
\include "lilypond-book-preamble.ly"

\version "2.14.0"
\paper {
  indent = 0\mm
  line-width = 160\mm
  % offset the left padding, also add 1mm as lilypond creates cropped
  % images with a little space on the right
  line-width = #(- line-width (* mm  3.000000) (* mm 1))
}

\layout {}

And with -dcheck-internal-types and see if you get this error.  I don't, which
is troubling, as I'm not sure what else in the running of the regtests could
trigger this type of error short of some memleak in another regtest that
trickles down.

Cheers,
MS

[hanwenn, 2011-10-15 21:41:20]

http://codereview.appspot.com/4877041/diff/17001/lily/system.cc
File lily/system.cc (left):

http://codereview.appspot.com/4877041/diff/17001/lily/system.cc#oldcode321
lily/system.cc:321: return &footnote_grobs_;
I think you can do this more concisely with the swap() method from the stl
container.

http://codereview.appspot.com/4877041/diff/17001/lily/system.cc
File lily/system.cc (right):

http://codereview.appspot.com/4877041/diff/17001/lily/system.cc#newcode251
lily/system.cc:251: Grob *at_bat = footnote_grobs_[i];
(I have no idea what a 'bat' is supposed to mean.)

http://codereview.appspot.com/4877041/diff/17001/lily/system.cc#newcode322
lily/system.cc:322: grob_2D_less (Grob *g1, Grob *g2)
Describe what this function is doing.  It's not clear from the implementation. 
Better variable names below would also help.

[mike_apollinemike.com, 2011-10-16 10:06:13]

On Oct 15, 2011, at 11:41 PM, hanwenn@gmail.com wrote:

> 
> http://codereview.appspot.com/4877041/diff/17001/lily/system.cc
> File lily/system.cc (left):
> 
> http://codereview.appspot.com/4877041/diff/17001/lily/system.cc#oldcode321
> lily/system.cc:321: return &footnote_grobs_;
> I think you can do this more concisely with the swap() method from the
> stl container.

Ok - will change.

> 
> http://codereview.appspot.com/4877041/diff/17001/lily/system.cc
> File lily/system.cc (right):
> 
> http://codereview.appspot.com/4877041/diff/17001/lily/system.cc#newcode251
> lily/system.cc:251: Grob *at_bat = footnote_grobs_[i];
> (I have no idea what a 'bat' is supposed to mean.)
> 

From American baseball - at bat is the person who will bat.  I find that there
is not enough baseball lingo in the source, which is a shame, as it is the sport
par excellence from which all other sports derive their legitimacy.

> http://codereview.appspot.com/4877041/diff/17001/lily/system.cc#newcode322
> lily/system.cc:322: grob_2D_less (Grob *g1, Grob *g2)
> Describe what this function is doing.  It's not clear from the
> implementation.  Better variable names below would also help.
> 

Will do.  The function determines which grob is higher on the sytem.

> http://codereview.appspot.com/4877041/

[Graham Percival, 2011-10-16 16:20:15]

On Sun, Oct 16, 2011 at 12:06:05PM +0200, mike@apollinemike.com wrote:
>    On Oct 15, 2011, at 11:41 PM, hanwenn@gmail.com wrote:
> 
>     
http://codereview.appspot.com/4877041/diff/17001/lily/system.cc#newcode251
>      lily/system.cc:251: Grob *at_bat = footnote_grobs_[i];
>      (I have no idea what a 'bat' is supposed to mean.)
> 
>    From American baseball - at bat is the person who will bat.  I find that
>    there is not enough baseball lingo in the source, which is a shame, as it
>    is the sport par excellence from which all other sports derive their
>    legitimacy.

By contrast, I feel that our stem collision code is in desperate
need of some football terminology -- if we tossed in some
red_flag = 0; yellow_flag = true;   variables into there, we could
really have some fun!

inb4 "that's not real football"

Cheers,
- Graham

[mail_philholmes.net, 2011-10-16 17:21:34]

----- Original Message ----- 
From: "Graham Percival" <graham@percival-music.ca>
To: <mike@apollinemike.com>
Cc: <reply@codereview.appspotmail.com>; <mtsolo@gmail.com>; 
<reinhold.kainhofer@gmail.com>; <lilypond-devel@gnu.org>; 
<hanwenn@gmail.com>
Sent: Sunday, October 16, 2011 5:20 PM
Subject: Re: Fixes footnote automatic numbering. (issue 4877041)


> On Sun, Oct 16, 2011 at 12:06:05PM +0200, mike@apollinemike.com wrote:
>>    On Oct 15, 2011, at 11:41 PM, hanwenn@gmail.com wrote:
>>
>> 
>> http://codereview.appspot.com/4877041/diff/17001/lily/system.cc#newcode251
>>      lily/system.cc:251: Grob *at_bat = footnote_grobs_[i];
>>      (I have no idea what a 'bat' is supposed to mean.)
>>
>>    From American baseball - at bat is the person who will bat.  I find 
>> that
>>    there is not enough baseball lingo in the source, which is a shame, as 
>> it
>>    is the sport par excellence from which all other sports derive their
>>    legitimacy.
>
> By contrast, I feel that our stem collision code is in desperate
> need of some football terminology -- if we tossed in some
> red_flag = 0; yellow_flag = true;   variables into there, we could
> really have some fun!
>
> inb4 "that's not real football"
>
> Cheers,
> - Graham

ITYM "red card" and "yellow card".  Flags are so gridiron.


--
Phil Holmes

[dak, 2011-11-10 05:00:43]

http://codereview.appspot.com/4877041/diff/25001/lily/grob.cc
File lily/grob.cc (right):

http://codereview.appspot.com/4877041/diff/25001/lily/grob.cc#newcode639
lily/grob.cc:639: if (!vag)
How is vag supposed to have become false again after being true two lines
before?

[pkx166h, 2011-11-10 07:11:18]

See

http://code.google.com/p/lilypond/issues/detail?id=1791#c10

Patch fails make check

James

[pkx166h, 2011-11-14 11:05:44]

Passes make but fails make check see:

http://code.google.com/p/lilypond/issues/detail?id=1791#c13

James

[mike_apollinemike.com, 2011-11-14 11:13:59]

On Nov 14, 2011, at 12:05 PM, pkx166h@gmail.com wrote:

> Passes make but fails make check see:
> 
> http://code.google.com/p/lilypond/issues/detail?id=1791#c13
> 
> James
> 
> http://codereview.appspot.com/4877041/


Thanks James.
As I feared, the memleak is still there.
I've combed the code several times but cannot find where it's leaky.  It's of
course possible that the leak is not caused by the code per se but rather
another structure in LilyPond that I'm using incorrectly (or that has a bug).

The valgrind backtrace isn't showing any memory leaks (meaning nothing beyond
the use of `new' in several functions in grob.cc, but these come up all over).

The only semi-fancy memory things I'm doing are the use of an array in
grob_2D_less and the creation/modification of grob arrays.  But I don't think
that either of these are done fishily.  Does anyone have any intuition in
reading over the code as to places where memory may be sapped?

Cheers,
MS

[dak, 2011-11-14 11:34:24]

http://codereview.appspot.com/4877041/diff/26012/lily/system.cc
File lily/system.cc (right):

http://codereview.appspot.com/4877041/diff/26012/lily/system.cc#newcode248
lily/system.cc:248: if (s->original ())
You don't check for success of the dynamic_cast before using it.  Is that a
problem?

http://codereview.appspot.com/4877041/diff/26012/lily/system.cc#newcode367
lily/system.cc:367: Grob *me = unsmob_grob (smob);
According to the compiler, getting the pointer me is the last operation for
which smob is needed.  After that, it is free for garbage collection.

If you are not sure that other references to smob keep it from being collected,
you need to write
scm_remember_upto_here_1 (smob)
at the place where garbage collecting smob will no longer be a problem.

That's probably after all the relevant info is entered in grobs_scm.

http://codereview.appspot.com/4877041/diff/26012/lily/system.cc#newcode383
lily/system.cc:383: Spanner *sys_span = unsmob_spanner (smob);
See above regarding garbage collection. sys_span is not likely to be kept around
long enough when optimizing.

[dak, 2011-11-14 11:43:11]

lily/grobarray.cc contains the following:

SCM
Grob_array::mark_smob (SCM s)
{
  (void) s;

#if 0  /* see System::derived_mark () const */
  Grob_array *ga = unsmob_grob_array (s);
  for (vsize i = 0; i < ga->grobs_.size (); i++)
    scm_gc_mark (ga->grobs_[i]->self_scm ());
#endif
  return SCM_UNDEFINED;
}

Consequently, elements of a grob array are not protected from garbage collection
when no other references to the grobs remain.

This is rather the reverse of a memory leak, however.

[mike_apollinemike.com, 2011-11-14 13:23:18]

Thanks for the feedback!  A couple questions below.

On Nov 14, 2011, at 12:34 PM, dak@gnu.org wrote:

> 
> http://codereview.appspot.com/4877041/diff/26012/lily/system.cc#newcode248
> lily/system.cc:248: if (s->original ())
> You don't check for success of the dynamic_cast before using it.  Is
> that a problem?
> 
> http://codereview.appspot.com/4877041/diff/26012/lily/system.cc#newcode367
> lily/system.cc:367: Grob *me = unsmob_grob (smob);
> According to the compiler, getting the pointer me is the last operation
> for which smob is needed.  After that, it is free for garbage
> collection.
> 
> If you are not sure that other references to smob keep it from being
> collected, you need to write
> scm_remember_upto_here_1 (smob)
> at the place where garbage collecting smob will no longer be a problem.
> 
> That's probably after all the relevant info is entered in grobs_scm.
> 

How is this function different from other callback functions that use the
convention Grob *me = unsmob_smob (smob); ?  I understand what you're saying
with scm_remember_upto_here, but I don't see why this function would need it
where other callback functions don't.

Also, is it safe to put a smob up for garbage collection after it is put into a
grob array?  That is, the next time that the grob array is unsmobed, will it
potentially contain a smob that has been garbage collected?

[mike_apollinemike.com, 2011-11-14 13:25:05]

On Nov 14, 2011, at 12:43 PM, dak@gnu.org wrote:

> lily/grobarray.cc contains the following:
> 
> SCM
> Grob_array::mark_smob (SCM s)
> {
>  (void) s;
> 
> #if 0  /* see System::derived_mark () const */
>  Grob_array *ga = unsmob_grob_array (s);
>  for (vsize i = 0; i < ga->grobs_.size (); i++)
>    scm_gc_mark (ga->grobs_[i]->self_scm ());
> #endif
>  return SCM_UNDEFINED;
> }
> 
> Consequently, elements of a grob array are not protected from garbage
> collection when no other references to the grobs remain.
> 
> This is rather the reverse of a memory leak, however.
> 

Agreed.
Reinhold - how did you do the memory profiling on all of the regtests?  I can
figure out how to do it for a single file, but not the regtests in succession. 
This'd help me figure out where/why lilypond is fizzling out.

Cheers,
MS

[dak, 2011-11-14 13:31:52]

"mike@apollinemike.com" <mike@apollinemike.com> writes:

> Thanks for the feedback!  A couple questions below.
>
> On Nov 14, 2011, at 12:34 PM, dak@gnu.org wrote:
>
>> 
>> http://codereview.appspot.com/4877041/diff/26012/lily/system.cc#newcode248
>> lily/system.cc:248: if (s->original ())
>> You don't check for success of the dynamic_cast before using it.  Is
>> that a problem?
>> 
>> http://codereview.appspot.com/4877041/diff/26012/lily/system.cc#newcode367
>> lily/system.cc:367: Grob *me = unsmob_grob (smob);
>> According to the compiler, getting the pointer me is the last operation
>> for which smob is needed.  After that, it is free for garbage
>> collection.
>> 
>> If you are not sure that other references to smob keep it from being
>> collected, you need to write
>> scm_remember_upto_here_1 (smob)
>> at the place where garbage collecting smob will no longer be a problem.
>> 
>> That's probably after all the relevant info is entered in grobs_scm.
>> 
>
> How is this function different from other callback functions that use
> the convention Grob *me = unsmob_smob (smob); ?  I understand what
> you're saying with scm_remember_upto_here, but I don't see why this
> function would need it where other callback functions don't.

I was just doing peep-hole review.  I have no idea about the general
conventions and code quality, or whether there is a contract about which
grobs can be relied on to have references when in callbacks, and for
which grobs the callback might be the last user.

> Also, is it safe to put a smob up for garbage collection after it is
> put into a grob array?  That is, the next time that the grob array is
> unsmobed, will it potentially contain a smob that has been garbage
> collected?

Any sane structure marks its contents during garbage collection as used.

The respective code in grob_array has been commented out by Jan.  The
commit logs of the relevant commits give no clue whatsoever why he would
do something like that.

So no, having a smobbed grob array is no guarantee that its contents
have not already been collected.

That's entirely insane.  There might be a reason for this, but it has
not been documented.

Ask Jan.

-- 
David Kastrup

[reinhold_fam.tuwien.ac.at, 2011-11-14 13:32:51]

On 2011-11-14 14:24, mike@apollinemike.com wrote:
> Reinhold - how did you do the memory profiling on all of the regtests? 
> I can figure out how to do it for a single file, but not the regtests 
> in succession. This'd help me figure out where/why lilypond is 
> fizzling out. 

cd input/regression/
lilypond *.ly

(and then get the values in a separate shell by "ps aux|grep 
lilypond|grep -v grep")

Cheers,
Reinhold

-- 
------------------------------------------------------------------
Reinhold Kainhofer, reinhold@kainhofer.com, http://reinhold.kainhofer.com/
  * Financial&  Actuarial Math., Vienna Univ. of Technology, Austria
  * http://www.fam.tuwien.ac.at/, DVR: 0005886
  * LilyPond, Music typesetting, http://www.lilypond.org

[mike_apollinemike.com, 2011-11-14 14:59:04]

On Nov 14, 2011, at 2:32 PM, Reinhold Kainhofer wrote:

> On 2011-11-14 14:24, mike@apollinemike.com wrote:
>> Reinhold - how did you do the memory profiling on all of the regtests? I can
figure out how to do it for a single file, but not the regtests in succession.
This'd help me figure out where/why lilypond is fizzling out. 
> 
> cd input/regression/
> lilypond *.ly
> 
> (and then get the values in a separate shell by "ps aux|grep lilypond|grep -v
grep")
> 
> Cheers,
> Reinhold

Some more info.
With the most recent patch set, when I comment out the footnote engraver and the
grob array callbacks for system, the segfault still happens.
So it is likely coming from something I'm stashing in probs.  I'll try to figure
out what it is...

Cheers,
MS

[Graham Percival, 2011-11-17 22:11:59]

On Thu, Nov 17, 2011 at 08:57:26AM +0100, mike@apollinemike.com wrote:
> Of course, it'd be great if during the compilation stage some
> tool could do a scan for all get/set property/object calls in
> LilyPond (in .ly, .scm, and .cc documents) and crash the
> compilation if called properties lack interfaces and/or
> docstrings.  This seems like a separate (and less pressing)
> issue from the memleak, however.

I wouldn't be surprised if some of the warnings from gcc and/or
clang and/or valgrind point to such a problem.  Of course,
actually cleaning up all those warnings in all those programs
could take anywhere from 10 to 100 hours -- such a task is likely
to involve a lot of plumbing in the dark and wishfully-forgotten
parts of lilypond (notably stuff in flower/ ).  It would be great
if somebody wanted to undertake such archeology, though.

- Graham

[mike_apollinemike.com, 2011-11-17 22:34:41]

On Nov 14, 2011, at 3:58 PM, mike@apollinemike.com wrote:

> Some more info.
> With the most recent patch set, when I comment out the footnote engraver and
the grob array callbacks for system, the segfault still happens.
> So it is likely coming from something I'm stashing in probs.  I'll try to
figure out what it is...
> 
> Cheers,
> MS

So I figured it out.

The two grob arrays were not declared in any interface, nor were they
documented.  I fixed this and now there's no memory leak.

This may be a valuable piece of information for future research on memory leaks
in LilyPond.  For whatever reason, when a property is looked up and/or passed
around and does not belong to an interface nor is documented, LilyPond will
compile clean but leak memory.

Of course, it'd be great if during the compilation stage some tool could do a
scan for all get/set property/object calls in LilyPond (in .ly, .scm, and .cc
documents) and crash the compilation if called properties lack interfaces and/or
docstrings.  This seems like a separate (and less pressing) issue from the
memleak, however.

Cheers,
MS

[mike_apollinemike.com, 2011-11-18 16:37:46]

On Nov 17, 2011, at 1:05 PM, Graham Percival wrote:

> On Thu, Nov 17, 2011 at 08:57:26AM +0100, mike@apollinemike.com wrote:
>> Of course, it'd be great if during the compilation stage some
>> tool could do a scan for all get/set property/object calls in
>> LilyPond (in .ly, .scm, and .cc documents) and crash the
>> compilation if called properties lack interfaces and/or
>> docstrings.  This seems like a separate (and less pressing)
>> issue from the memleak, however.
> 
> I wouldn't be surprised if some of the warnings from gcc and/or
> clang and/or valgrind point to such a problem.  Of course,
> actually cleaning up all those warnings in all those programs
> could take anywhere from 10 to 100 hours -- such a task is likely
> to involve a lot of plumbing in the dark and wishfully-forgotten
> parts of lilypond (notably stuff in flower/ ).  It would be great
> if somebody wanted to undertake such archeology, though.
> 
> - Graham

As part of the solution, I've put together a patch that does documenting of some
undocumented objects.  I'll post this in the not-too-distant future.

Here are all of the explicitly referenced properties and objects that I could
find that have no documentation.  Not surprisingly, most of them are for Probs,
which for some reason are not documented at all (is this Prob-lematic?).

PROBS
foot-stencil
first-markup-line
bottom-space
last-markup-line
system-grob
footnotes
lines
number
head-stencil
penalty
last-in-score
configuration
tight-spacing
staff-refpoint-extent
is-title

GROBS
ties
forced-spacing
melody-spanner
note-collision

MUSIC
class
context
id
events
moment
property-path
ops
music-cause

Cheers,
MS