Re: [PATCH] Re: Re: not-so-ancient flags

[Reinhold Kainhofer]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am Freitag, 22. August 2008 schrieb Han-Wen Nienhuys:
> +  LY_ASSERT_TYPE (scm_is_number, spos, 1);
> +  Grob *g = unsmob_grob (sg);
> +  Grob *st = Staff_symbol_referencer::get_staff_symbol (g);
>
> what happens if st is null?

The next line is st?(...):false, which makes sense, because if there is no 
staff, the position cannot be on a staff line. This code is, BTW, taken 
verbatim from C++ and simply re-written in scheme.

> Stem::flag after this rewrite should just look like
>
>
> Stencil
> Stem::flag(Grob *g)
> {
>   if (Stencil *flag = unsmob_stencil (get_property("flag"))) {
>     return *flag;
>   }
>   return Stencil();
> }
>
> then there should be Stem::calc_flag which contains logic for deciding
> which font symbol to get, and constructing the stencil.
>
> Overrides are done with
>
>   \override Stem #'flag = #my-stencil-creation-function
>
> (default for this prop is ly:stem::calc-flag )
>
> Do we have a 'style in the stem grob?  If no, please use the existing
> 'style property rather than flag-style.

Well, I don't think that the 'style of the stem should be used for the flag 
style. In particular, for me Stem 'style means the style of the stem and not 
the style of the flag. Stem 'flag-style on the other hand clearly says that 
it is the style for the flag and not the stem. Although we currently don't 
have different stem styles, this would make it rather cumbersome to add 
different stem styles later on. The other advantage of 'flag-style is that 
all current code would still work, so users don't have to deal with yet 
another syntax change (which are really annoying, I completely agree with 
anyone complaining...).

So, to summarize what you suggest:
- -) Stem 'flag property: Always a scheme function returning the complete 
stencil; The default scheme function calls some C++ function, which does 
exactly what we have in Stem::flag right now (i.e. lookup a glyph from the 
font, using the 'flag-style property for the name).
- -) Stem 'flag-style property: The name of the style, used only by the 
default 'flag function.
- -) C++ function Stem::calc_flag: Contains the current code for flags will be 
called by the default function of Stem 'flag.

> +;; (define-public (straight-flag flag-thickness flag-spacing
> +;;                        upflag-angle upflag-length
> +;;                        downflag-angle downflag-length)
> +;;   (lambda (stem-grob)
>
> you can write this as
>
>   (define ((f inner-arg inner-arg2) outer-arg1 outer-arg2)

Ah, right, thanks.

> +         "flag-styles.scm"
>
> Looks like overkill - I would suggest a section in output-lib.scm

Considering that output-lib.scm has just over 600 lines and flag-styles.scm 
has already 120 lines, without straight lines being implemented, that would 
mean that after straight flags are implemented, about a quarter of 
output-lib.scm would be dedicated to flags. Thus I thought I'd move that code 
to is own file.

> > 1) I have the impression that ly:stencil-rotate (which is not used
> > anywhere else in lilypond) does not correctly rotate the stencil extents.
> > In particular, look at the inverted-normal-flag style in the example
> > above and you'll notice the large white space between the staff and the
> > mark. I also printed out the stencil extents to the console and they
> > seemed pretty strange to me...
>
> could be.

I'll look into this.

> > 2) I've moved the whole functionality to create flags to scheme. The
> > advantage is that you can call the built-in flag styles from your own
> > styles. The other possibility would be to handle 'mensural and 'no-flag
> > with the current C++ code and only use scheme if the flag-style property
> > is set to a function (i.e. if get_property already returns a stencil, use
> > that, otherwise continue with the C++ code in current master). The
> > drawback of that approach is that there would either be some code
> > duplication between C++ and scheme, or that you would not be able to
> > modify existing flag styles easily. The output will be exactly the same
> > in both cases.
>
> I think the existing code should remain C++ as it is executed a lot,
> so there is a speed advantage.

To be honest, I doubt that there is much difference. As a "quick" test I ran 
lilypond on one of my 120 pages full scores, once with current master (flag 
generated in C++) and once with the patch applied (all flags generated in 
scheme). That's the results:

- -) With the patch (i.e. everything in scheme):
Konvertierung nach »./Schubert_StabatMater_D383_LongScore.pdf«...
real    32m36.546s
user    32m11.217s
sys     0m13.005s

- -) With current master (i.e. everything in C++):
Konvertierung nach »./Schubert_StabatMater_D383_LongScore.pdf«...
real    32m53.452s
user    32m10.501s
sys     0m13.569s

So, basically there's no difference to be seen.
I got similar results with a small test file (1.5 seconds), where there is 
also no difference in the time.

Cheers,
Reinhold

- -- 
- ------------------------------------------------------------------
Reinhold Kainhofer, Vienna University of Technology, Austria
email: address@hidden, http://reinhold.kainhofer.com/
 * Financial and Actuarial Mathematics, TU Wien, http://www.fam.tuwien.ac.at/
 * K Desktop Environment, http://www.kde.org, KOrganizer maintainer
 * Chorvereinigung "Jung-Wien", http://www.jung-wien.at/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFIrwIITqjEwhXvPN0RAohOAJ9Td2E+aa2QnrwcXbx3sB8SfxgWBACfd0DZ
yNbtYjoVugjsk+ZQg17dMhA=
=rVVz
-----END PGP SIGNATURE-----