Re: C++ question on wrapper API for setting Guile fluids

[Dan Eble]

On Apr 22, 2022, at 05:39, Jean Abou Samra  wrote:
> 
> You both made a good point. My conclusion is that I should stick to
> 
>   {
> Local_assumption la (Lily::prebreak_estimate, SCM_BOOL_T);
> ...
>   }
> 
> where the constructor does everything, included scm_dynwind_begin ()
> and scm_dynwind_fluid (). That's safer and clearer, and never mind that
> you have to construct 3 contexts if you need 3 assumptions. I expect
> this to be very rare anyway.

Fine.  At the time someone wants more than one assumption, we might be able to 
add

Local_assumptions la {{fluid1, value1},
^ {fluid2, value2} /* etc. */ };

where the constructor accepts std::initializer_list> but 
I don't know if there are any problems related to GC (and I'm not planning to 
work through that now).
— 
Dan

Re: C++ question on wrapper API for setting Guile fluids

[David Kastrup]

Jean Abou Samra  writes:

> Re call/cc: I would actually like to use
> static_cast (0)
> and not SCM_F_WIND_EXPLICITLY. I have no idea whether the stuff I write
> is going to interact gracefully with rewinding the stack via continuations,
> and I honestly don't want to wonder.We're using C++ (on contrary to Guile,
> which is primarily tailored for C), so most allocations (e.g., vectors) are
> managed via C++ RAII. This means that C++'s stack unwinding and
> Guile's don't
> interact, so you can easily crash LilyPond with continuations and
> such, by making
> some expression return twice and arranging for it to cause double free. This
> is the first example that came to my mind:
>
> \version "2.23.7"
>
> {
>   \override NoteHead.X-extent =
>     #(let ((cont #f))
>    (lambda (grob)
>  (if cont
>  (cont '(0 . 0))
>  (call/cc
>   (lambda (c)
>     (set! cont c)
>     '(0 . 0))
>   c d
> }
>
>
>
> Frankly, I don't think we need to bother at all about such stuff. If
> the user is willing this hard to shoot themselves in the foot, we
> can't prevent them.

It's worth keeping in mind that dynamic programming (like trying to
figure out the best breakpoint sequence) naturally lends itself to
juggling a set of best continuations so far (a continuation is more
versatile to build upon than a static description of a partial
solution).

But the main control flow strategies are certainly maintained by
LilyPond itself, so we are mainly shooting ourselves in the foot by
blocking certain programming practices that make your head ache but may
actually be a good fit for "under the hood".  So it's stuff that there
is no point in bending over backwards to get right, but if we see a
reasonably easy and affordable way to do it "continuation-safe", it's
probably worth doing it and thus keeping more options easily accessible
for the future.

One example of "weird control flow" I had to revert to is
map-markup-commands even if it does not go to the depths of restarting
continuations but "only" throws parts of a solution.

-- 
David Kastrup

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Le 22/04/2022 à 09:52, Luca Fascione a écrit :

[snipped]
Where the problem is that dwc.free(p) is actually effectively acting 
as if it was dwc2.free(p); because the API doesn't pass around the 
context like the C++ wrappers appear to do, rather it statefully "just 
goes for it".
This is a design decision of Guile, obviously. However, it seems to me 
this has possibly uncommon semantics even if it were implemented on 
the scheme-language side of things anyways, doesn't it? I guess I'm 
asking whether people would want/need to do this on purpose, and why. 
It seems to me to achieve this behaviour one would have to capture the 
dwc context and then invoke dwc.free() on that context from deeper 
inside its own stack, and I'm not clear what this means for the frames 
intervening inbetween the calling frame and the callee frame, being a 
current parent of it, maybe it's ok, I normally think of continuations 
as things to deal with a stack that has been unwound, not deeper parts 
of a stack that is still being executed.




I guess it would be possible to make it work, using 
scm_current_dynamic_state
and scm_set_current_dynamic_state. I don't have a need to implement it, 
though.
Assumptions should always be introduced just after creating the dynamic 
context.




[...]

I think the first observation is very good: making it easiest to spot 
wrong code without paying large amounts of attention has been a very 
good strategy for me in the past.
And for the same reason, assuming the scm_dynwind_XXX set of calls is 
small, I would either wrap all of them (because they're all small and 
easy) or very clearly document the wrapper class as "if you need this 
method, add it here and this is how": you definitely don't want to 
find yourself in a halfway house where there are all sorts of 
exceptions that "yes these three methods could have been wrapped but 
we didn't" and now nobody remembers any longer why a decision was 
taken one way or the other. It's a small cost now and can save many 
headaches later.
Obv this works if the set of scm_dynwind_XXX is small (maybe up to 
10-15 calls?), if you have several dozens, you'd definitely wrap a few 
important ones that you need, and leave the rest for another day (with 
the comment inviting the next person along to help with the effort).






You both made a good point. My conclusion is that I should stick to

  {
    Local_assumption la (Lily::prebreak_estimate, SCM_BOOL_T);
    ...
  }

where the constructor does everything, included scm_dynwind_begin ()
and scm_dynwind_fluid (). That's safer and clearer, and never mind that
you have to construct 3 contexts if you need 3 assumptions. I expect
this to be very rare anyway.



As to the exchange Jean and I were having about non-local control 
flow, I was worried about scm_dynwind_end() being called one extra 
time (rather than not being called enough, as your example 
demonstrates) and how to detect that, but looking at how 
scm_dynamic_wind() is implemented, maybe from the C side this can't 
happen. I'm asking: is there a case where evaluating the "main", 
non-guard thunk implies a call to scm_dynwind_end(), so that the outer 
C code doesn't need to? And I think the answer to that is no. So I 
guess as long as we can guarantee that the think doesn't use call/cc 
(or that we always use SCM_F_DYNSTACK_FRAME_REWINDABLE) this will be ok?




Re "can scm_dynwind_end () be called unpaired in the thunk": if the callee
respects programming contracts, no. The principle of scm_dynwind_begin 
() and
scm_dynwind_end () is that they should always be paired under normal 
circumstances

(when no Guile non-local control flow happens).

Re call/cc: I would actually like to use 
static_cast (0)

and not SCM_F_WIND_EXPLICITLY. I have no idea whether the stuff I write
is going to interact gracefully with rewinding the stack via continuations,
and I honestly don't want to wonder.We're using C++ (on contrary to Guile,
which is primarily tailored for C), so most allocations (e.g., vectors) are
managed via C++ RAII. This means that C++'s stack unwinding and Guile's 
don't
interact, so you can easily crash LilyPond with continuations and such, 
by making

some expression return twice and arranging for it to cause double free. This
is the first example that came to my mind:

\version "2.23.7"

{
  \override NoteHead.X-extent =
    #(let ((cont #f))
   (lambda (grob)
 (if cont
 (cont '(0 . 0))
 (call/cc
  (lambda (c)
    (set! cont c)
    '(0 . 0))
  c d
}



Frankly, I don't think we need to bother at all about such stuff. If the 
user

is willing this hard to shoot themselves in the foot, we can't prevent them.

Echoing Han-Wen's fear about the problems linked with dynamic states, 
the reason
to use fluids, apart from consistency with the existing (*parser*) and 
(*location*),

is to be a little cooperative with basic and actually useful non-local flow
such as (mock 

Re: C++ question on wrapper API for setting Guile fluids

[Luca Fascione]

On Thu, Apr 21, 2022 at 11:46 PM Jean Abou Samra  wrote:

> Well, the C++ and Scheme interfaces can feel different and idiomatic
> in their respective languages as long as they share the same
> underlying implementation.
>

I think this is a super important goal. In fact, I'd upgrade 'can' to
'should' :-)


> In the same vein, although Guile has scm_dynamic_wind (), the
> recommended C interface is scm_dynwind_begin () / scm_dynwind_end ()
> because that is more scalable in C code.
>

Yes, and that supports your idea of using RAII to bookend it correctly in
the face of exceptions and such things.
Which is very idiomatic in modern C++ and feels quite natural to me.

Dan had an objection about this case:

{
  Dynwind_context dwc;
  // . . .

  {
Dynwind_context dwc2;
// . . .
dwc.free (p);  // not what it seems
// . . .
  }
}

Where the problem is that dwc.free(p) is actually effectively acting as if
it was dwc2.free(p); because the API doesn't pass around the context like
the C++ wrappers appear to do, rather it statefully "just goes for it".
This is a design decision of Guile, obviously. However, it seems to me this
has possibly uncommon semantics even if it were implemented on the
scheme-language side of things anyways, doesn't it? I guess I'm asking
whether people would want/need to do this on purpose, and why. It seems to
me to achieve this behaviour one would have to capture the dwc context and
then invoke dwc.free() on that context from deeper inside its own stack,
and I'm not clear what this means for the frames intervening inbetween the
calling frame and the callee frame, being a current parent of it, maybe
it's ok, I normally think of continuations as things to deal with a stack
that has been unwound, not deeper parts of a stack that is still being
executed. Whereas in a way this is what {} scoping does, it's weird that
there could be intervening function calls and this would still be allowed,
because this second scenario is closer to implementing TCL's upvar/uplevel
mechanism, which I've learned Schemers really don't like. For these reasons
I suspect that Dan's example can easily happen by mistake, but wouldn't be
something that folks have a legitimate, every-day use for, or is it?

Anyways I was reading the source of Guile from the repo, and I found that
dynwind_begin() does this:

void
scm_dynwind_begin (scm_t_dynwind_flags flags)
{
  scm_thread *thread = SCM_I_CURRENT_THREAD;
  scm_dynstack_push_frame (>dynstack, flags);
}


So I'm thinking that (maybe in some appropriate debug mode) if
Dynwind_context were to capture a copy of >dynstack, it could then
check if the stack has moved or not,
and error out in some useful way if the (dyn)stack pointer has changed.
Given C++ semantics, I suspect it can only change by being deeper, but one
way or the other, this could work.
Mind you, as much as the proposed RAII idea is kinda slightly dangerous, I
don't think we should protect too much for folks doing irresponsible things
with the stack, as long as
the documentation is clear about how the wrapper works, this should be
fine.

On the other hand, if this concept of messing with dwc from a stack frame
deeper inside is important, dynstack.h has all controls for capturing and
standing up local stacks, so one could instead do a full capture in the
constructor (say calling scm_dynstack_capture_all, or something like that)
to allow for this behaviour (and then the forwarding methods would adjust
the dynstack to always be pointed as expected).
It seems a bit overkill to me, because I'm not used to the semantics
implied by doing this, but all the same it appears this might work.

-> footnote: these arguably all appear to be internal API entrypoints, I'm
not sure what this means in terms of it being a polite thing or not to call
them from lilypond's codebase.

Earlier in the thread Jean said:

> Making a method of the context is an attractive proposal, as
> it prevents from forgetting to introduce a context.
>
> Actually, the specific use case is much narrower than this particular
> set of methods: I don't need C++ wrappers for all dynwind-related Guile
> APIs, I just have one need for fluids. The example in the first email was
> a straw man for the sake of explanation.

I think the first observation is very good: making it easiest to spot wrong
code without paying large amounts of attention has been a very good
strategy for me in the past.
And for the same reason, assuming the scm_dynwind_XXX set of calls is
small, I would either wrap all of them (because they're all small and easy)
or very clearly document the wrapper class as "if you need this method, add
it here and this is how": you definitely don't want to find yourself in a
halfway house where there are all sorts of exceptions that "yes these three
methods could have been wrapped but we didn't" and now nobody remembers any
longer why a decision was taken one way or the other. It's a small cost now

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Le 21/04/2022 à 23:35, David Kastrup a écrit :

Properties will be coded a lot in Scheme.  So the interface needs to map
naturally to Scheme programming.



Well, the C++ and Scheme interfaces can feel different and idiomatic
in their respective languages as long as they share the same
underlying implementation.

In the same vein, although Guile has scm_dynamic_wind (), the
recommended C interface is scm_dynwind_begin () / scm_dynwind_end ()
because that is more scalable in C code.

Re: C++ question on wrapper API for setting Guile fluids

[David Kastrup]

Jean Abou Samra  writes:

> Le 21/04/2022 à 13:10, Dan Eble a écrit :
>> That is probably mostly due to your early remark that your
>> control-flow options are limited in the case of a with_fluid wrapper
>> taking a lambda. (It definitely steered my answer.)
>
>
> Well, I find it better if it doesn't require reworking the control
> flow logic to introduce assumptions in existing code. Also, that
> should facilitate the refactoring of unpure/pure containers into
> assumptions.

Properties will be coded a lot in Scheme.  So the interface needs to map
naturally to Scheme programming.

-- 
David Kastrup

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Le 21/04/2022 à 13:10, Dan Eble a écrit :
That is probably mostly due to your early remark that your 
control-flow options are limited in the case of a with_fluid wrapper 
taking a lambda. (It definitely steered my answer.)



Well, I find it better if it doesn't require reworking the control
flow logic to introduce assumptions in existing code. Also, that
should facilitate the refactoring of unpure/pure containers into 
assumptions.

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Le 21/04/2022 à 12:58, David Kastrup a écrit :

My personal take on this would move the magic out of the normal reach of
users.  You get (*start-column*) and (*end-column*) which you can use
for accessing the respective fluids but the function *start-column* does
not merely access %start-column but it also registers the associated
impurity for the sake of caching.



Absolutely, I don't want to make the fluids accessible to
anything outside of the property infrastructure. *start-column*
(or whatever name) is an Assumption object. Assumption is
a smob type defining a call () smob procedure. That way, you
can have

(*start-column*)

and

(under-assumptions ((*start-column* 0))
  ...)

and both of these use the fluid as well as other internal members
of the assumption without ever making them accessible. For the
latter, the under-assumptions macro invocation translates to

(ly:run-under-assumption *start-column* 0 (lambda () ...))

which makes the interface completely opaque/encapsulated.

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Le 21/04/2022 à 11:18, Han-Wen Nienhuys a écrit :

OK. I'm curious to see how that turns out.



I am too :-)

I've been intermittently working on this project for more than
6 months now. When I do an experimental refactoring, I make
a new branch from the current one to avoid losing the previous work.
Currently I am working in a branch called 'purity5' ...



Does every call to
get_property() will have to check the fluid as well to see if it
should cache the computed value?




Yes. I'm trying to make it such that this check is fast, because
get_property without purity at all is the frequent case.



How does the caching framework know if the computation depends on the
assumption?




In my current design, Output_def gains a new member, which is a
stack (SCM list). Each element of the stack corresponds to a
property callback that is currently being made. The element is
a list of assumptions that have been used for this call. Suppose,
for example, that you have the call chain

VerticalAxisGroup.adjacent-pure-heights
[introduces *prebreak-estimate*, *prebreak-estimate-start*, 
*prebreak-estimate-end*]

-> PaperColumn.Y-extent
-> NoteColumn.Y-extent
-> Stem.Y-extent

Each get_property call pushes a frame on the Output_def's stack.
During the Stem.Y-extent callback, the stack looks like

(() ;; Stem.Y-extent
 () ;; NoteColumn.Y-extent
 () ;; PaperColumn.Y-extent
 () ;; VerticalAxisGroup.adjacent-pure-heights
 ...)


Moreover, when the *prebreak-estimate* assumption is introduced,
it records the state of the stack. So *prebreak-estimate* remembers
the SCM sublist that corresponds to the part

(() ;; VerticalAxisGroup.adjacent-pure-heights
 ...)

(Equivalently, it could also remember the stack's depth.)

Now, Stem.Y-extent accesses *prebreak-estimate*. This records
in the stack that callbacks inside the scope where the assumption
has been introduced need to get special caching. More precisely,
the stack is walked until the sublist recorded by
*prebreak-estimate* is found. This makes it look like


((#) ;; Stem.Y-extent
 (#) ;; NoteColumn.Y-extent
 (#) ;; PaperColumn.Y-extent
 () ;; VerticalAxisGroup.adjacent-pure-heights
 ...)


At the end of the Stem.Y-extent get_property () call,
the top frame is popped off and checked. If it's '(), the
property can be cached normally. If it's a 1-element list
containing *prebreak-estimate*, it receives caching with
a prebreak value and a postbreak value. Anything else
leads to no caching at all. (Except if benchmarks tell that
it brings a performance advantage to cache according to
(start, end) in a hash table.)

Re: C++ question on wrapper API for setting Guile fluids

[Dan Eble]

On Apr 21, 2022, at 06:54, Jean Abou Samra  wrote:
> 
> Le 21/04/2022 à 12:47, David Kastrup a écrit :
>> So what one would want is a C++ wrapper working with functionals? 
...
> So this needs an interface that is easier to use, whether it uses
> RAII or functionals or something else. Current talks in this thread
> favor the RAII approach.

That is probably mostly due to your early remark that your control-flow options 
are limited in the case of a with_fluid wrapper taking a lambda.  (It 
definitely steered my answer.)
— 
Dan

Re: C++ question on wrapper API for setting Guile fluids

[David Kastrup]

Jean Abou Samra  writes:

> Le 21/04/2022 à 10:38, Han-Wen Nienhuys a écrit :
>> On Thu, Apr 21, 2022 at 12:04 AM Jean Abou Samra  wrote:
>>> I am working on code that pervasively utilizes Guile fluids. They should
>>> be set in dynamic scopes.
>> That sounds scary. Care to tell more?
>
> What is scary about it exactly?
>
> I am trying to reimplement purity in terms of fluids, so the set of
> parameters is not hardcoded to 'start, end' and all the code doesn't
> have to be littered with functions working both as pure and impure and
> forwarding start/end to the property callbacks they
> cause.

Good.

> Essentially, this should look like {   Dynwind_context dwc;  
> dwc->make_assumption (Lily::prebreak_estimate, SCM_BOOL_T);  
> ... get_property (grob, "property") ... } or in Scheme:
> (under-assumptions ((*prebreak-estimate* #t))   ...) and by virtue of
> the dynamic context, the callback that computes the grob property
> understands that it should do pure estimates.

My personal take on this would move the magic out of the normal reach of
users.  You get (*start-column*) and (*end-column*) which you can use
for accessing the respective fluids but the function *start-column* does
not merely access %start-column but it also registers the associated
impurity for the sake of caching.

This also helps by not registering impurities that don't arise for a
particular grob (things like breakpoint-depending accidentals
comparatively rarely rise to the level of actual relevance but get
everything dealt with as impure, with the associated costs, all the
time).

> I haven't gotten as far as a working proof of concept, but I expect a
> speedup. This is essentially a remix of an idea from David:
> https://lists.gnu.org/archive/html/lilypond-devel/2015-05/msg00397.html

I probably repeat myself...

-- 
David Kastrup

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Le 21/04/2022 à 12:47, David Kastrup a écrit :
So what one would want is a C++ wrapper working with functionals? 



My criterion is rather convenience. scm_c_with_fluid could
be convenient if I could use it with a lambda. However, I cannot,
I'd need to define a separate function every time, taking void *
and casting that to some struct where needed local variables
are stored. For repeated use, that is a nuisance.

So this needs an interface that is easier to use, whether it uses
RAII or functionals or something else. Current talks in this thread
favor the RAII approach.

Re: C++ question on wrapper API for setting Guile fluids

[Luca Fascione]

I wonder if you can 'chain' them:

Dynwind_context dwc2(dwc);

(you can at a minimum 'pause' dwc so to emit a runtime message that the
"wrong" thing is happening, but I guess you could hand yourself to your
parent and do more sophisticated shenanigans too... I must say I loathe
this stateful stuff though)

L

On Thu, Apr 21, 2022 at 12:23 PM Dan Eble  wrote:

> On Apr 21, 2022, at 02:55, Luca Fascione  wrote:
> >
> > I'd think you can up this by one, and get a cleaner looking piece of code
> > if you implement scm_dynwind_fluid() as a forwarded method on your
> context:
> ...
> >  dwc.fluid (fluid2, value2);
>
> Here's something that does bother me.  That reads as if dwc holds state
> affecting the outcome, which is untrue.
>
> {
>   Dynwind_context dwc;
>   // . . .
>
>   {
> Dynwind_context dwc2;
> // . . .
> dwc.free (p);  // not what it seems
> // . . .
>   }
> }
>
> The scm_ functions operate implicitly on the current context.  Dressing
> them differently would be confusing.
> —
> Dan
>
>

-- 
Luca Fascione
Distinguished Engineer - Ray Tracing - NVIDIA

Re: C++ question on wrapper API for setting Guile fluids

[David Kastrup]

Jean Abou Samra  writes:

>> Le 21/04/2022 11:05, David Kastrup  a écrit :
>> Just for the record, your usage would not fit with the existing
>> lily/include/fluid.hh ,
>
>
> The existing code in fluid.hh is concerned with retrieving
> fluids. I'm asking about setting them. Whether the new code
> can go in fluid.hh is a question, but it is new code.
>
>
>> and scm_c_with_fluid does not suffice?
>
>
> It would suffice, but it does not work with C++ lambdas,
> which makes it inconvenient.

So what one would want is a C++ wrapper working with functionals?

-- 
David Kastrup

Re: C++ question on wrapper API for setting Guile fluids

[Dan Eble]

On Apr 21, 2022, at 02:55, Luca Fascione  wrote:
> 
> I'd think you can up this by one, and get a cleaner looking piece of code
> if you implement scm_dynwind_fluid() as a forwarded method on your context:
...
>  dwc.fluid (fluid2, value2);

Here's something that does bother me.  That reads as if dwc holds state 
affecting the outcome, which is untrue.

{
  Dynwind_context dwc;
  // . . .

  {
Dynwind_context dwc2;
// . . .
dwc.free (p);  // not what it seems
// . . .
  }
}

The scm_ functions operate implicitly on the current context.  Dressing them 
differently would be confusing.
— 
Dan

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Not sure why my message got mangled? Resending, properly formatted.


Le 21/04/2022 à 10:51, Jean Abou Samra a écrit :

Le 21/04/2022 à 10:38, Han-Wen Nienhuys a écrit :
On Thu, Apr 21, 2022 at 12:04 AM Jean Abou Samra  
wrote:
I am working on code that pervasively utilizes Guile fluids. They 
should

be set in dynamic scopes.

That sounds scary. Care to tell more?


What is scary about it exactly?




I am trying to reimplement purity in terms of fluids, so the set of
parameters is not hardcoded to 'start, end' and all the code doesn't
have to be littered with functions working both as pure and impure and
forwarding start/end to the property callbacks they
cause. Essentially, this should look like


{
  Dynwind_context dwc;
  dwc->make_assumption (Lily::prebreak_estimate, SCM_BOOL_T);
  ... get_property (grob, "property") ...
}

or in Scheme:

(under-assumptions ((*prebreak-estimate* #t))
  ...)

and by virtue of the dynamic context, the callback that computes the
grob property understands that it should do pure estimates. The
property then gets cached if it doesn't depend on assumptions, or if
it only depends on *prebreak-estimate* (in that case with two cached
value, prebreak and postbreak), but not if the callback uses
*prebreak-estimate-start* or *prebreak-estimate-end*. I'll have to
experiment with caching strategies, but this is the current idea.

The advantage is that 95% of code that needs to interact with purity
(via ly:make-unpure-pure-container, get_maybe_pure_property, etc.) no
longer needs to do so. Purity becomes a very localized thing and no
longer pervades the code base. It can also be worked on more easily;
my work on https://gitlab.com/lilypond/lilypond/-/merge_requests/744
involves introducing pure widths, and I don't want to add
maybe_pure_x_extent and change hundreds of callbacks so they do
maybe_pure_x_extent instead of extent ( X_AXIS). There is the same
need in order to properly implement functionality demonstrated in a
recent -user thread
https://lists.gnu.org/archive/html/lilypond-user/2022-04/msg00062.html
where LyricText.X-offset needs a pure/unpure distinction in order to
avoid after-line-breaking.

Also, since whether callbacks access start and end is now known, pure
properties that depend on whether the call is pure or not but not on
start and end (namely the vast majority of them) can start getting
cached. Right now, we use a trick to cache item pure heights, assuming
that Y-extent/Y-offset does not depend on start/end for items
(Item::pure_y_extent). This can then be done for spanners as well. I
haven't gotten as far as a working proof of concept, but I expect a
speedup.


This is essentially a remix of an idea from David:
https://lists.gnu.org/archive/html/lilypond-devel/2015-05/msg00397.html

Jean

Re: C++ question on wrapper API for setting Guile fluids

[Han-Wen Nienhuys]

On Thu, Apr 21, 2022 at 10:53 AM Jean Abou Samra  wrote:
> >> On Thu, Apr 21, 2022 at 12:04 AM Jean Abou Samra 
> >> wrote:
> >>> I am working on code that pervasively utilizes Guile fluids. They
> >>> should
> >>> be set in dynamic scopes.
> >> That sounds scary. Care to tell more?
> >
> > What is scary about it exactly?

I am worried about introducing problems associated with dynamic
scoping into the code base; the (un)pure stuff is already there
though, so it probably is not going to create new problems, though.

> I am trying to reimplement purity in terms of fluids, so the set of
> parameters is not hardcoded to 'start, end' and all the code doesn't
> have to be littered with functions working both as pure and impure and
> forwarding start/end to the property callbacks they
> cause. Essentially, this should look like
>
>
> {
>Dynwind_context dwc;
>dwc->make_assumption (Lily::prebreak_estimate, SCM_BOOL_T);
>... get_property (grob, "property") ...
> }
>
> or in Scheme:
>
> (under-assumptions ((*prebreak-estimate* #t))
>...)
>
> and by virtue of the dynamic context, the callback that computes the
> grob property understands that it should do pure estimates. The
> property then gets cached if it doesn't depend on assumptions, or if
> it only depends on *prebreak-estimate* (in that case with two cached
> value, prebreak and postbreak), but not if the callback uses
> *prebreak-estimate-start* or *prebreak-estimate-end*. I'll have to
> experiment with caching strategies, but this is the current idea.

OK. I'm curious to see how that turns out.  Does every call to
get_property() will have to check the fluid as well to see if it
should cache the computed value?
How does the caching framework know if the computation depends on the
assumption?

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

> Le 21/04/2022 11:05, David Kastrup  a écrit :
> Just for the record, your usage would not fit with the existing
> lily/include/fluid.hh ,


The existing code in fluid.hh is concerned with retrieving
fluids. I'm asking about setting them. Whether the new code
can go in fluid.hh is a question, but it is new code.


> and scm_c_with_fluid does not suffice?


It would suffice, but it does not work with C++ lambdas,
which makes it inconvenient.

Re: C++ question on wrapper API for setting Guile fluids

[David Kastrup]

Jean Abou Samra  writes:

> Hi,
>
> It's me again ...
>
> I am working on code that pervasively utilizes Guile fluids. They should
> be set in dynamic scopes. In C++, usage of the Guile API looks like this:
>
>   scm_dynwind_begin ();
>   scm_dynwind_fluid (fluid, value);
>   ...
>   scm_dynwind_end ();

Just for the record, your usage would not fit with the existing
lily/include/fluid.hh , and scm_c_with_fluid does not suffice?

-- 
David Kastrup

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Hi Luca,

Thanks for your insights! Much appreciated.


Le 21/04/2022 à 08:55, Luca Fascione a écrit :
I'd think you can up this by one, and get a cleaner looking piece of 
code if you implement scm_dynwind_fluid() as a forwarded method on 
your context:


{

  Dynwind_context dwc; // overloaded so you can call 
dwc(SCM_F_DYNWIND_REWINDABLE);

  dwc.fluid (fluid1, value1);
  dwc.fluid (fluid2, value2);

  dwc.unwind_handler(handler, data, flags); // overloaded for the SCM 
vs void* cases,
  dwc.rewind_handler(handler, data, flags); // overloaded for the SCM 
vs void* cases,
                                            // maybe it ought to check 
if the constructor was DYNWIND_REWINDABLE and complain
                                            // (only in debug builds?) 
if things are not set up right

  dwc.free(something);

}




Making a method of the context is an attractive proposal, as
it prevents from forgetting to introduce a context.

Actually, the specific use case is much narrower than this particular
set of methods: I don't need C++ wrappers for all dynwind-related Guile
APIs, I just have one need for fluids. The example in the first email was
a straw man for the sake of explanation.



However in all this, I must say I don't understand this passage in manual:

The context is ended either implicitly when a non-local exit
happens, or explicitly with scm_dynwind_end. You must make sure
that a dynwind context is indeed ended properly. If you fail to
call scm_dynwind_end for each scm_dynwind_begin, the behavior is
undefined.


It seems to me the first phrase and the rest slightly contradict each 
other: as I hear it, the first phrase says "either the C side calls 
scm_dynwind_end(), OR a non-local exit happens", whereas the rest 
seems to be saying "the C side _shall_ call scm_dynwind_end". This 
bothers me, because in the second case our C++ is nice and lovely, but 
in the first meaning the destructor of dwc has to somehow figure out 
whether a non-local exit has happened, and avoid calling 
scm_dynwind_end(). And I don't think scm library can cope on its own, 
because these things look to me like they would nest, so making 
scm_dynwind_end() idempotent without some sort of explicit marker on 
the scope seems... hard.


So yes, I'd think RAII is the idiomatic way to go, I would add the 
wrappers because they make the pattern cleaner, but do figure out 
what's up with this last question first, because it could bring it all 
crumbling down.



Let's do a little test:

diff --git a/lily/main.cc b/lily/main.cc
index c98db72e8c..b8e928e29c 100644
--- a/lily/main.cc
+++ b/lily/main.cc
@@ -496,6 +496,21 @@ do_chroot_jail ()
 }
 #endif

+class Dynwind_context
+{
+public:
+  Dynwind_context ()
+  {
+    message ("starting dynwind context");
+    scm_dynwind_begin (static_cast (0));
+  }
+  ~Dynwind_context ()
+  {
+    message ("ending dynwind context");
+    scm_dynwind_end ();
+  }
+};
+
 static void
 main_with_guile (void *, int, char **)
 /*
@@ -505,6 +520,17 @@ main_with_guile (void *, int, char **)
  * to main_with_guile.
  */
 {
+  message ("normal termination:");
+  {
+    Dynwind_context dwc;
+    message ("before normal termination");
+  }
+  message ("termination by exception:");
+  {
+    Dynwind_context dwc;
+    message ("before termination by exception");
+    scm_throw (ly_symbol2scm ("x"), SCM_EOL);
+  }
   /* Engravers use lily.scm contents, so we need to make Guile find it.
  Prepend onto GUILE %load-path.
   %load-path is the symbol Guile searches for .scm files


Running this, I get:

normal termination:
starting dynwind context
before normal termination
ending dynwind context
termination by exception:
starting dynwind context
before termination by exceptionBacktrace:
In ice-9/boot-9.scm:
  1752:10  1 (with-exception-handler _ _ #:unwind? _ # _)
In unknown file:
   0 (apply-smob/0 #)

ERROR: In procedure apply-smob/0:
Throw to key `x' with args `()'.

As you can see, scm_dynwind_end () is not called on the non-local exit.

There are actually two kinds of non-local exits that we are talking about
here: the C++ ones, and the Guile ones. C++ does them via return,
break, continue, C++ exceptions. Guile does them via Guile exceptions
and continuations. When Guile captures a continuation, for example,
it just copies the whole C(++) stack in an object, and reinstating
the continuation simply copies the saved stack data back into the
stack. See the end of
https://www.gnu.org/software/guile/manual/html_node/Continuations.html

Exceptions are kind of a special case of this (except that in Guile 2
and later it looks like they are actually implemented with
delimited/composable continuations, aka prompts, rather than traditional
continuations, but I haven't looked at that too closely).

Thus, C++ RAII stuff ensures destructors get called upon C++ non-local
exits, but not Guile ones.

So the meaning of the docs here is that you must insert a call to

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Le 21/04/2022 à 10:38, Han-Wen Nienhuys a écrit :

On Thu, Apr 21, 2022 at 12:04 AM Jean Abou Samra  wrote:

I am working on code that pervasively utilizes Guile fluids. They should
be set in dynamic scopes.

That sounds scary. Care to tell more?


What is scary about it exactly?

I am trying to reimplement purity in terms of fluids, so the set of 
parameters is not hardcoded to 'start, end' and all the code doesn't 
have to be littered with functions working both as pure and impure and 
forwarding start/end to the property callbacks they cause. Essentially, 
this should look like {   Dynwind_context dwc;   dwc->make_assumption 
(Lily::prebreak_estimate, SCM_BOOL_T);   ... get_property (grob, 
"property") ... } or in Scheme: (under-assumptions ((*prebreak-estimate* 
#t))   ...) and by virtue of the dynamic context, the callback that 
computes the grob property understands that it should do pure estimates. 
The property then gets cached if it doesn't depend on assumptions, or if 
it only depends on *prebreak-estimate* (in that case with two cached 
value, prebreak and postbreak), but not if the callback uses 
*prebreak-estimate-start* or *prebreak-estimate-end*. I'll have to 
experiment with caching strategies, but this is the current idea. The 
advantage is that 95% of code that needs to interact with purity (via 
ly:make-unpure-pure-container, get_maybe_pure_property, etc.) no longer 
needs to do so. Purity becomes a very localized thing and no longer 
pervades the code base. It can also be worked on more easily; my work on 
https://gitlab.com/lilypond/lilypond/-/merge_requests/744 involves 
introducing pure widths, and I don't want to add maybe_pure_x_extent and 
change hundreds of callbacks so they do maybe_pure_x_extent instead of 
extent ( X_AXIS). There is the same need in order to properly implement 
functionality demonstrated in a recent -user thread 
https://lists.gnu.org/archive/html/lilypond-user/2022-04/msg00062.html 
where LyricText.X-offset needs a pure/unpure distinction in order to 
avoid after-line-breaking. Also, since whether callbacks access start 
and end is now known, pure properties that depend on whether the call is 
pure or not but not on start and end (namely the vast majority of them) 
can start getting cached. Right now, we use a trick to cache item pure 
heights, assuming that Y-extent/Y-offset does not depend on start/end 
for items (Item::pure_y_extent). This can then be done for spanners as 
well. I haven't gotten as far as a working proof of concept, but I 
expect a speedup. This is essentially a remix of an idea from David: 
https://lists.gnu.org/archive/html/lilypond-devel/2015-05/msg00397.html




Jean

Re: C++ question on wrapper API for setting Guile fluids

[Han-Wen Nienhuys]

On Thu, Apr 21, 2022 at 12:04 AM Jean Abou Samra  wrote:
> I am working on code that pervasively utilizes Guile fluids. They should
> be set in dynamic scopes.

That sounds scary. Care to tell more?

-- 
Han-Wen Nienhuys - hanw...@gmail.com - http://www.xs4all.nl/~hanwen

Re: C++ question on wrapper API for setting Guile fluids

[Luca Fascione]

On Thu, Apr 21, 2022 at 8:12 AM Jean Abou Samra  wrote:

> Le 21/04/2022 à 04:57, Dan Eble a écrit :
> >  {
> >// dwc constructor calls scm_dynwind_begin ()
> >Dynwind_context dwc;
> >scm_dynwind_fluid (fluid1, value1);
> >scm_dynwind_fluid (fluid2, value2);
> >. . .
> >// dwc destructor calls scm_dynwind_end ()
> >  }
>
>
>
> Why not. There is likely just one caller that will need to introduce
> several
> fluids at once, but it is probably clearer this way.
>

I'd think you can up this by one, and get a cleaner looking piece of code
if you implement scm_dynwind_fluid() as a forwarded method on your context:

{

  Dynwind_context dwc; // overloaded so you can call
dwc(SCM_F_DYNWIND_REWINDABLE);
  dwc.fluid (fluid1, value1);
  dwc.fluid (fluid2, value2);

  dwc.unwind_handler(handler, data, flags); // overloaded for the SCM vs
void* cases,
  dwc.rewind_handler(handler, data, flags); // overloaded for the SCM vs
void* cases,
// maybe it ought to check if
the constructor was DYNWIND_REWINDABLE and complain
// (only in debug builds?) if
things are not set up right
  dwc.free(something);

}

However in all this, I must say I don't understand this passage in manual:

The context is ended either implicitly when a non-local exit happens, or
explicitly with scm_dynwind_end. You must make sure that a dynwind context
is indeed ended properly. If you fail to call scm_dynwind_end for each
scm_dynwind_begin, the behavior is undefined.

It seems to me the first phrase and the rest slightly contradict each
other: as I hear it, the first phrase says "either the C side calls
scm_dynwind_end(), OR a non-local exit happens", whereas the rest seems to
be saying "the C side _shall_ call scm_dynwind_end". This bothers me,
because in the second case our C++ is nice and lovely, but in the first
meaning the destructor of dwc has to somehow figure out whether a non-local
exit has happened, and avoid calling scm_dynwind_end(). And I don't think
scm library can cope on its own, because these things look to me like they
would nest, so making scm_dynwind_end() idempotent without some sort of
explicit marker on the scope seems... hard.

So yes, I'd think RAII is the idiomatic way to go, I would add the wrappers
because they make the pattern cleaner, but do figure out what's up with
this last question first, because it could bring it all crumbling down.

HTH
Luca

Re: C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Le 21/04/2022 à 04:57, Dan Eble a écrit :

On Apr 20, 2022, at 18:04, Jean Abou Samra  wrote:

Calls to scm_dynwind_begin () and scm_dynwind_end () must be
paired correctly. Obviously, if '...' contains statements
causing non-local control flow like return, break, continue,
and such, it is easy to miss a scm_dynwind_end (). I read
that you can avoid that using RAII:

Yes.


   {
 // Constructor does scm_dynwind_begin () and scm_dynwind_fluid ()
 Fluid_setter setter (fluid, value);

Might you want to call scm_dynwind_fluid () more than once?  How about this?

 {
   // dwc constructor calls scm_dynwind_begin ()
   Dynwind_context dwc;
   scm_dynwind_fluid (fluid1, value1);
   scm_dynwind_fluid (fluid2, value2);
   . . .
   // dwc destructor calls scm_dynwind_end ()
 }




Why not. There is likely just one caller that will need to introduce several
fluids at once, but it is probably clearer this way.




What disturbs me slightly with this approach is the variable 'setter',
which will never be used, but must nevertheless be defined in order
for the object not to be thrown away as temporary.

If I understand you, turning the main effect (scm_dynwind_begin) into a side 
effect of the constructor bothers you.  It wouldn't bother me as long as the 
class had a good name.  I can think of two alternatives, but they have their 
own problems.


Does that sound about right? Would this be considered an idiomatic
approach?

Yes.



Thanks!

Jean

Re: C++ question on wrapper API for setting Guile fluids

[Dan Eble]

On Apr 20, 2022, at 18:04, Jean Abou Samra  wrote:
> 
> Calls to scm_dynwind_begin () and scm_dynwind_end () must be
> paired correctly. Obviously, if '...' contains statements
> causing non-local control flow like return, break, continue,
> and such, it is easy to miss a scm_dynwind_end (). I read
> that you can avoid that using RAII:

Yes.

>   {
> // Constructor does scm_dynwind_begin () and scm_dynwind_fluid ()
> Fluid_setter setter (fluid, value);

Might you want to call scm_dynwind_fluid () more than once?  How about this?

{
  // dwc constructor calls scm_dynwind_begin ()
  Dynwind_context dwc;
  scm_dynwind_fluid (fluid1, value1);
  scm_dynwind_fluid (fluid2, value2);
  . . .
  // dwc destructor calls scm_dynwind_end ()
}

> What disturbs me slightly with this approach is the variable 'setter',
> which will never be used, but must nevertheless be defined in order
> for the object not to be thrown away as temporary.

If I understand you, turning the main effect (scm_dynwind_begin) into a side 
effect of the constructor bothers you.  It wouldn't bother me as long as the 
class had a good name.  I can think of two alternatives, but they have their 
own problems.

> Does that sound about right? Would this be considered an idiomatic
> approach?

Yes.
— 
Dan

C++ question on wrapper API for setting Guile fluids

[Jean Abou Samra]

Hi,

It's me again ...

I am working on code that pervasively utilizes Guile fluids. They should
be set in dynamic scopes. In C++, usage of the Guile API looks like this:

  scm_dynwind_begin ();
  scm_dynwind_fluid (fluid, value);
  ...
  scm_dynwind_end ();

Docs: https://www.gnu.org/software/guile/manual/html_node/Dynamic-Wind.html

Calls to scm_dynwind_begin () and scm_dynwind_end () must be
paired correctly. Obviously, if '...' contains statements
causing non-local control flow like return, break, continue,
and such, it is easy to miss a scm_dynwind_end (). I read
that you can avoid that using RAII:

  {
    // Constructor does scm_dynwind_begin () and scm_dynwind_fluid ()
    Fluid_setter setter (fluid, value);
    // Rest of this block is in a dynamic scope where the fluid
    // has the new value.
    ...
    // Destructor does scm_dynwind_end ().
  }


What disturbs me slightly with this approach is the variable 'setter',
which will never be used, but must nevertheless be defined in order
for the object not to be thrown away as temporary.

Does that sound about right? Would this be considered an idiomatic
approach?

An alternative would be

  call_with_fluid (fluid, value, [&](){
    ...
  });


However, this does not map in the same straightforward way to fluid-less
code: it does not allow return statements jumping out of the main function
(as opposed to the inner lambda).

Does that nevertheless look better to trained C++ eyes? Are there
other ways?

Thanks for your patience,
Jean

Re: C++ question:

[David Kastrup]

Knut Petersen  writes:

>>   // A markup property whiteout-markup-wzd is implemented.
>>   // The following definition is used for that property:
>>   // \markup { \with-dimensions #'(0 . 0) #'(0 . 0) {
>>   //   \filled-box #'(0.0 . 1.0) #'(-0.5 . 0.5) #0.0 } }
>>   SCM properties = Font_interface::text_font_alist_chain (me);
>>   SCM ws_zd_mod = Text_interface::interpret_markup (
>>  me->layout ()->self_scm (),
>>  properties,
>>  me->get_property ("whiteout-markup-wzd"));
>>   Stencil wsa = *unsmob (ws_zd_mod);
>>
>
>>   // The following three lines should give an equivalent definition:
>>   Box wb (Interval (0.0, 1.0), Interval (-0.5, 0.5));
>>   Stencil wsb (Lookup::round_filled_box (wb, 0));
>>   wsb.set_empty(false); 
>
> The difference between Stencil wsa and wsb is that \markup { \withdimensions 
> #'(0 . 0) #'(0 . 0) ...}
> not only sets the dimensions :
>
> (define-markup-command (with-dimensions layout props x y arg)
>   (number-pair? number-pair? markup?)
>   #:category other
>   "
> @cindex setting extent of text objects
>
> Set the dimensions of @var{arg} to @var{x} and@tie{}@var{y}."
>   (let* ((expr (ly:stencil-expr (interpret-markup layout props arg
> (ly:stencil-add
>  (make-transparent-box-stencil x y)
>  (ly:make-stencil
>   `(delay-stencil-evaluation ,(delay expr))
>   x y

I think that is half of a red herring: apparently the delayed stencil
evaluation is only used in order to sneak the stencil outline past the
outline tracing code in lily/stencil-integral.cc.  And that apparently
causes the dimensions to be totally ignored, so a transparent box is
pasted on top.

What a steaming heap of something.  So your code would likely have
worked in LilyPond 2.16.  I think it would make sense to create a new
type of stencil expression explicitly intended to bypass outlining.

Probably by just containing _two_ stencils: one for typesetting, one for
outlining.  That would make for a much more transparent manner of
programming things like that.

-- 
David Kastrup

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question:

[Knut Petersen]

  // A markup property whiteout-markup-wzd is implemented.
  // The following definition is used for that property:
  // \markup { \with-dimensions #'(0 . 0) #'(0 . 0) {
  //   \filled-box #'(0.0 . 1.0) #'(-0.5 . 0.5) #0.0 } }
  SCM properties = Font_interface::text_font_alist_chain (me);
  SCM ws_zd_mod = Text_interface::interpret_markup (
 me->layout ()->self_scm (),
 properties,
 me->get_property ("whiteout-markup-wzd"));
  Stencil wsa = *unsmob (ws_zd_mod);




  // The following three lines should give an equivalent definition:
  Box wb (Interval (0.0, 1.0), Interval (-0.5, 0.5));
  Stencil wsb (Lookup::round_filled_box (wb, 0));
  wsb.set_empty(false); 


The difference between Stencil wsa and wsb is that \markup { \withdimensions 
#'(0 . 0) #'(0 . 0) ...}
not only sets the dimensions :

(define-markup-command (with-dimensions layout props x y arg)
  (number-pair? number-pair? markup?)
  #:category other
  "
@cindex setting extent of text objects

Set the dimensions of @var{arg} to @var{x} and@tie{}@var{y}."
  (let* ((expr (ly:stencil-expr (interpret-markup layout props arg
(ly:stencil-add
 (make-transparent-box-stencil x y)
 (ly:make-stencil
  `(delay-stencil-evaluation ,(delay expr))
  x y

Knut

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question:

[Knut Petersen]

Hi David!

Sigh.  Why do you use set_empty _after_ translation?  That makes the
remaining point stick out from the actual glyph.  And of course, it
totally moots any effect of set_empty before translation.


It does not matter where set_empty is used, it does not have the desired
effect. I tried a lot of possible places and combinations.

See comments in the following snippet:

  Stencil total;
  for (int i = 0; i < hyphens; i++)
{
  Stencil ds = dash_stencil;
  ds.translate_axis (span_points[LEFT] + start_space +
 i * (dash_period + dash_length), X_AXIS);
  total.add_stencil (ds);
  if (whiteout > 0.0 )
{
  // A markup property whiteout-markup-wzd is implemented.
  // The following definition is used for that property:
  // \markup { \with-dimensions #'(0 . 0) #'(0 . 0) {
  //   \filled-box #'(0.0 . 1.0) #'(-0.5 . 0.5) #0.0 } }
  SCM properties = Font_interface::text_font_alist_chain (me);
  SCM ws_zd_mod = Text_interface::interpret_markup (
 me->layout ()->self_scm (),
 properties,
 me->get_property ("whiteout-markup-wzd"));
  Stencil wsa = *unsmob (ws_zd_mod);

  // The following three lines should give an equivalent definition:
  Box wb (Interval (0.0, 1.0), Interval (-0.5, 0.5));
  Stencil wsb (Lookup::round_filled_box (wb, 0));
  wsb.set_empty(false);

  Stencil wos;
  // If this definition of wos is used everything works as intended,
  // collsion detection ignores wos:
  wos = wsa.in_color (1.0, 0, 0);
  // If this definition is used, collision detection does _not_ ignore
  // wos but moves the last lyric line in the testfile down:
  //
  // wos = wsb.in_color (1.0, 0, 0);

  Real xscale = (dash_stencil.extent (X_AXIS).length ()
 + 2 * whiteout * lt);
  Real yscale = (dash_stencil.extent (Y_AXIS).length ()
 + 2 * whiteout * lt);
  wos.scale(xscale,yscale);

  wos.translate_axis (span_points[LEFT] + start_space
  + i * (dash_period + dash_length)
  - whiteout * lt, X_AXIS);

  Real cory = 0.5 * (dash_stencil.extent (Y_AXIS)[DOWN]
 + dash_stencil.extent (Y_AXIS) [UP]);
  wos.translate_axis (cory, Y_AXIS);

  total.add_stencil (wos);
}
}

  total.translate_axis (-me->relative_coordinate (common, X_AXIS), X_AXIS);
  return total.smobbed_copy ();
}


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question:

[David Kastrup]

Knut Petersen  writes:

>> Uh, you set the dimensions to 0,0 _after_ moving the thing to its place.
>> That would make the remaining point stick out seriously wide of the
>> actual glyph.  Maybe move the set_empty before the translate_axis?
>
> Not even the following code shows any improvement:
>
>   Stencil total;
>   for (int i = 0; i < hyphens; i++)
> {
>   Stencil ds = dash_stencil;
>   ds.translate_axis (span_points[LEFT] + start_space +
>  i * (dash_period + dash_length), X_AXIS);
>   total.add_stencil (ds);
>   if (whiteout > 0.0 )
> {
>   Box wb (Interval (0, dash_length + 2 * whiteout * lt),
>   Interval (h - whiteout * lt, h + th + whiteout * lt));
>   Stencil ws;
>   ws.set_empty(false);
>   ws = (Lookup::round_filled_box (wb, 0.8 * lt));
>   ws.set_empty(false);
>   ws = ws.in_color (1.0, 0.0, 0.0);
>   ws.translate_axis (span_points[LEFT] + start_space + i * 
> (dash_period
>  + dash_length) - whiteout * lt, X_AXIS);
>   ws.set_empty(false);
>   total.add_stencil (ws);
>   total.set_empty(false);
> }
> }
>
>   total.translate_axis (-me->relative_coordinate (common, X_AXIS), X_AXIS);
>   total.set_empty(false);
>   return total.smobbed_copy ();
> }

Sigh.  Why do you use set_empty _after_ translation?  That makes the
remaining point stick out from the actual glyph.  And of course, it
totally moots any effect of set_empty before translation.

-- 
David Kastrup

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question:

[Knut Petersen]

Am 20.01.2017 um 13:35 schrieb David Kastrup:

Knut Petersen  writes:


Hi David!

Uh what?

If you want to change the dimensions of a stencil copy, you can just do

Stencil wd0 = orig;
wd0.set_empty (false);

I use the following code to emit hyphens (stencil ds) and whiteout boxes 
(stencil ws)
in lyric-hyphen.cc:

   Stencil total;
   for (int i = 0; i < hyphens; i++)
 { // ok
   Stencil ds = dash_stencil;
   ds.translate_axis (span_points[LEFT] + start_space +
  i * (dash_period + dash_length), X_AXIS);
   total.add_stencil (ds);
   if (whiteout > 0.0 )
 {
   Box wb (Interval (0, dash_length + 2 * whiteout * lt),
   Interval (h - whiteout * lt, h + th + whiteout * lt));
   Stencil ws (Lookup::round_filled_box (wb, 0.8 * lt));
   ws = ws.in_color (1.0, 0.0, 0.0);
   ws.translate_axis (span_points[LEFT] + start_space + i * (dash_period
  + dash_length) - whiteout * lt, X_AXIS);
   ws.set_empty(false); // <== Does not help against collision detection
   total.add_stencil (ws);
 }
 }

As you can see in the attached jpgs, a collision of the whiteout box moves the 
last lyric line down.
set_empty(true) nor anything else I tried does change anything.

Uh, you set the dimensions to 0,0 _after_ moving the thing to its place.
That would make the remaining point stick out seriously wide of the
actual glyph.  Maybe move the set_empty before the translate_axis?


Not even the following code shows any improvement:

  Stencil total;
  for (int i = 0; i < hyphens; i++)
{
  Stencil ds = dash_stencil;
  ds.translate_axis (span_points[LEFT] + start_space +
 i * (dash_period + dash_length), X_AXIS);
  total.add_stencil (ds);
  if (whiteout > 0.0 )
{
  Box wb (Interval (0, dash_length + 2 * whiteout * lt),
  Interval (h - whiteout * lt, h + th + whiteout * lt));
  Stencil ws;
  ws.set_empty(false);
  ws = (Lookup::round_filled_box (wb, 0.8 * lt));
  ws.set_empty(false);
  ws = ws.in_color (1.0, 0.0, 0.0);
  ws.translate_axis (span_points[LEFT] + start_space + i * (dash_period
 + dash_length) - whiteout * lt, X_AXIS);
  ws.set_empty(false);
  total.add_stencil (ws);
  total.set_empty(false);
}
}

  total.translate_axis (-me->relative_coordinate (common, X_AXIS), X_AXIS);
  total.set_empty(false);
  return total.smobbed_copy ();
}

Knut

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question:

[David Kastrup]

Knut Petersen  writes:

> Hi David!
>> Uh what?
>>
>> If you want to change the dimensions of a stencil copy, you can just do
>>
>> Stencil wd0 = orig;
>> wd0.set_empty (false);
>
> I use the following code to emit hyphens (stencil ds) and whiteout boxes 
> (stencil ws)
> in lyric-hyphen.cc:
>
>   Stencil total;
>   for (int i = 0; i < hyphens; i++)
> { // ok
>   Stencil ds = dash_stencil;
>   ds.translate_axis (span_points[LEFT] + start_space +
>  i * (dash_period + dash_length), X_AXIS);
>   total.add_stencil (ds);
>   if (whiteout > 0.0 )
> {
>   Box wb (Interval (0, dash_length + 2 * whiteout * lt),
>   Interval (h - whiteout * lt, h + th + whiteout * lt));
>   Stencil ws (Lookup::round_filled_box (wb, 0.8 * lt));
>   ws = ws.in_color (1.0, 0.0, 0.0);
>   ws.translate_axis (span_points[LEFT] + start_space + i * 
> (dash_period
>  + dash_length) - whiteout * lt, X_AXIS);
>   ws.set_empty(false); // <== Does not help against collision 
> detection
>   total.add_stencil (ws);
> }
> }
>
> As you can see in the attached jpgs, a collision of the whiteout box moves 
> the last lyric line down.
> set_empty(true) nor anything else I tried does change anything.

Uh, you set the dimensions to 0,0 _after_ moving the thing to its place.
That would make the remaining point stick out seriously wide of the
actual glyph.  Maybe move the set_empty before the translate_axis?

> The ugly hack to define a markup property containing a filled box inside a 
> \with-dimensions zero
> for both axes, interpret, scale and move the stencil as required
> works. But there must be a better way ...

Your markup example does _not_ include any shift before zeroing the
bounding boxes of the whiteout rectangle.  So it would appear like you
don't actually do the same thing here.

> WithUglyHack.jpg shows the targeted result ...



-- 
David Kastrup

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question:

[Knut Petersen]

Hi David!

Uh what?

If you want to change the dimensions of a stencil copy, you can just do

Stencil wd0 = orig;
wd0.set_empty (false);


I use the following code to emit hyphens (stencil ds) and whiteout boxes 
(stencil ws)
in lyric-hyphen.cc:

  Stencil total;
  for (int i = 0; i < hyphens; i++)
{ // ok
  Stencil ds = dash_stencil;
  ds.translate_axis (span_points[LEFT] + start_space +
 i * (dash_period + dash_length), X_AXIS);
  total.add_stencil (ds);
  if (whiteout > 0.0 )
{
  Box wb (Interval (0, dash_length + 2 * whiteout * lt),
  Interval (h - whiteout * lt, h + th + whiteout * lt));
  Stencil ws (Lookup::round_filled_box (wb, 0.8 * lt));
  ws = ws.in_color (1.0, 0.0, 0.0);
  ws.translate_axis (span_points[LEFT] + start_space + i * (dash_period
 + dash_length) - whiteout * lt, X_AXIS);
  ws.set_empty(false); // <== Does not help against collision detection
  total.add_stencil (ws);
}
}

As you can see in the attached jpgs, a collision of the whiteout box moves the 
last lyric line down.
set_empty(true) nor anything else I tried does change anything.

The ugly hack to define a markup property containing a filled box inside a 
\with-dimensions zero
for both axes, interpret, scale and move the stencil as required works. But 
there must be a better way ...
WithUglyHack.jpg shows the targeted result ...

 Knut

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question:

[David Kastrup]

Knut Petersen  writes:

> Hi everybody!
>
>
> Is there an equivalent of  \with-dimensions #'(0 . 0) #'(0 . 0) that can be 
> used on a stencil within the C++ part of lilypond?
>
> If not: I thought I could work around that problem  by constructing and using 
> a markup:
>
>   SCM properties = Font_interface::text_font_alist_chain (me);
>   SCM ws_mol = Text_interface::interpret_markup (me->layout 
> ()->self_scm (), properties, ly_string2scm ("\\markup {\\bold foo}"));
>   Stencil ws = *unsmob (ws_mol);
>
> That emits "\markup {\bold foo}" instead of the result of the interpreted 
> markup. If I define a property test-markup,
> replace 'ly_string2scm ("\\markup {\\bold foo}")' with something like  
> 'me->get_property ("test-markup")' the code works
> as expected. So ly_string2scm is not enough ... I need a string2markup(). Any 
> idea?

Uh what?

If you want to change the dimensions of a stencil copy, you can just do

Stencil wd0 = orig;
wd0.set_empty (false);

(set_empty (false) sets the bounding dimensions to 0,0 like
point-stencil does, set_empty (true) to nothing like overriding the
stencil with ##f does).

-- 
David Kastrup

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

C++ question:

[Knut Petersen]

Hi everybody!


Is there an equivalent of  \with-dimensions #'(0 . 0) #'(0 . 0) that can be 
used on a stencil within the C++ part of lilypond?

If not: I thought I could work around that problem  by constructing and using a 
markup:

  SCM properties = Font_interface::text_font_alist_chain (me);
  SCM ws_mol = Text_interface::interpret_markup (me->layout ()->self_scm (), 
properties, ly_string2scm ("\\markup {\\bold foo}"));
  Stencil ws = *unsmob (ws_mol);

That emits "\markup {\bold foo}" instead of the result of the interpreted 
markup. If I define a property test-markup,
replace 'ly_string2scm ("\\markup {\\bold foo}")' with something like  'me->get_property 
("test-markup")' the code works
as expected. So ly_string2scm is not enough ... I need a string2markup(). Any 
idea?


 Knut

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

C++ question

[m...@mikesolomon.org]

Hey all,

If I have a header file with stuff like:

class MyClass {
typedef int (*happy)(int a, int b, int c);

happy foo
happy bar;
happy baz;
}

Is there a way to define foo, bar, and baz with the same typedef in the c++ 
file or do I have to enumerate all the arguments for the function definition à 
la

int baz (int a, int b, int c) { return a + b + c; }

?

Cheers,
MS



___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question

[David Kastrup]

m...@mikesolomon.org m...@mikesolomon.org writes:

 Hey all,

 If I have a header file with stuff like:

 class MyClass {
 typedef int (*happy)(int a, int b, int c);

 happy foo
 happy bar;
 happy baz;
 }

 Is there a way to define foo, bar, and baz with the same typedef in
 the c++ file

No.  The parameter names are not part of the type signature.  You could
use a macro, but I'd advise against it.

-- 
David Kastrup


___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel

Re: C++ question

[Benkő Pál]

hi Mike,

 class MyClass {
 typedef int (*happy)(int a, int b, int c);

 happy foo
 happy bar;
 happy baz;
 }

 Is there a way to define foo, bar, and baz with the same typedef in the c++ 
 file or do I have to enumerate all the arguments for the function definition 
 à la

 int baz (int a, int b, int c) { return a + b + c; }

no.  foo, bar and baz are _defined_ in the header as pointers to
function (and not declared as functions of given signature).
you can use them like

int lily (int i, int n, int t)
{
  return i + n * t;
}

struct pond
{
  static int lily (int i, int n, int t) { return i *n + t; }
};

foo = lily;
bar = pond::lily;
baz = foo;

p

___
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel