Stepping into the code for nested properties

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Stepping into the code for nested properties

Rodolfo Zitellini
Dear All,
I am looking to issue #1063, mostly as an excuse to investigate the
internals of lilypond a bit (and also because I need to revert nested
properties for a transcription I'm doing).
Is there someone who would mentor me on this? I have already setup a
dev system with lilypond and gdb, and I have stepped thougly the code,
which I mostly uderstand what is doing. But there is some logic which
is not completely clear to me, and I would need help on that.

Thanks,
Rodolfo

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Carl Sorensen
I would be happy to mentor you on this.

Neil Puttock seems to have done most of the work on nested properties, and
he follows lilypond-devel very closely.

Thanks for asking,

Carl



On 5/7/10 3:32 AM, "Rodolfo Zitellini" <[hidden email]> wrote:

> Dear All,
> I am looking to issue #1063, mostly as an excuse to investigate the
> internals of lilypond a bit (and also because I need to revert nested
> properties for a transcription I'm doing).
> Is there someone who would mentor me on this? I have already setup a
> dev system with lilypond and gdb, and I have stepped thougly the code,
> which I mostly uderstand what is doing. But there is some logic which
> is not completely clear to me, and I would need help on that.
>
> Thanks,
> Rodolfo
>
> ---
> ----
> Join the Frogs!
>


---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Neil Puttock
On 7 May 2010 16:36, Carl Sorensen <[hidden email]> wrote:
> I would be happy to mentor you on this.

I'd also be happy to help, though I can't promise to provide many
insights into the code.

> Neil Puttock seems to have done most of the work on nested properties, and
> he follows lilypond-devel very closely.

Heh, I've done a few inconsequential tweaks related to nested
properties which have made it into master, but the lion's share of the
work is Han-Wen's. :)

Regards,
Neil

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Rodolfo Zitellini
Thanks to you both, Carl and Neil. I will prepare my list of questions shortly!

Regards,
Rodolfo

On Fri, May 7, 2010 at 11:43 PM, Neil Puttock <[hidden email]> wrote:

> On 7 May 2010 16:36, Carl Sorensen <[hidden email]> wrote:
>> I would be happy to mentor you on this.
>
> I'd also be happy to help, though I can't promise to provide many
> insights into the code.
>
>> Neil Puttock seems to have done most of the work on nested properties, and
>> he follows lilypond-devel very closely.
>
> Heh, I've done a few inconsequential tweaks related to nested
> properties which have made it into master, but the lion's share of the
> work is Han-Wen's. :)
>
> Regards,
> Neil
>

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Rodolfo Zitellini
Ok here I am with my first round of questions.
If I understand correctly, an object's properties are represented by
an alist of pairs in the form (property-name . value), and "value" can
be another alist (for nested properties)
Now, when you call execute_override_property (in context-property.cc)
you get the alist with all the properties and change the relevant one,
but... how is the list of the properties created in the first place?
where do the default values come from?
I ask because when you do a \revert and you did not do any \override
before in the same grob execute_revert_property simply does nothing
and returns, but if the \override was done, in the case of a nested
property the one you are reverting simply gets removed from the alist
(the code in nested_property_revert_alist(91) is quite explicit too:
/* old value is dropped. */). I assume this is the problem, did I
understand right here?

Thanks,
Rodolfo

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Carl Sorensen



On 5/11/10 2:11 AM, "Rodolfo Zitellini" <[hidden email]> wrote:

> Ok here I am with my first round of questions.
> If I understand correctly, an object's properties are represented by
> an alist of pairs in the form (property-name . value), and "value" can
> be another alist (for nested properties)
> Now, when you call execute_override_property (in context-property.cc)
> you get the alist with all the properties and change the relevant one,
> but... how is the list of the properties created in the first place?
> where do the default values come from?

IIUC, the default values are part of the *context* properties, rather than
part of the *grob* properties.  If there is no grob property list, then the
value of the properties is determined from the defaults in the context.

Also, IIUC, when you call execute_override_property, you don't *replace* the
override, you *prepend* it.  Or at least, that's what happens for non-nested
properties.  It's possible that that *doesn't* happen for nested properties,
and may be the cause of some of the nested-property bugs.

> I ask because when you do a \revert and you did not do any \override
> before in the same grob execute_revert_property simply does nothing
> and returns, but if the \override was done, in the case of a nested
> property the one you are reverting simply gets removed from the alist
> (the code in nested_property_revert_alist(91) is quite explicit too:
> /* old value is dropped. */). I assume this is the problem, did I
> understand right here?

I think so.

Thanks,

Carl


---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Neil Puttock
On 11 May 2010 17:59, Carl Sorensen <[hidden email]> wrote:

> IIUC, the default values are part of the *context* properties, rather than
> part of the *grob* properties.  If there is no grob property list, then the
> value of the properties is determined from the defaults in the context.

Each grob is a context property (I learned this from you during our
discussions on autobeaming. :)

The defaults for each grob are contained in all-grob-descriptions (in
scm/define-grobs.scm), which the parser uses via \grobdescriptions to
set each grob in the Global context.  You can see how this works by
using \applyContext:

#(use-modules (ice-9 pretty-print))

\new Voice \relative c' {
  cis d e f
  \applyContext #(lambda (ctx)
                   (pretty-print
                    (ly:context-property ctx 'Accidental)))
}

Once an override comes along, the defaults are updated recursively
(updated_grob_properties ()) to carry over any overrides from parent
contexts.  The overrides are prepended to a copy of the default alist,
which itself is prepended to the default settings:

#(use-modules (ice-9 pretty-print))

\new Voice \relative c' {
  \override Accidental #'color = #red
  cis d e f
  \applyContext #(lambda (ctx)
                   (pretty-print
                    (ly:context-property ctx 'Accidental)))
}

> Also, IIUC, when you call execute_override_property, you don't *replace* the
> override, you *prepend* it.  Or at least, that's what happens for non-nested
> properties.  It's possible that that *doesn't* happen for nested properties,
> and may be the cause of some of the nested-property bugs.

It looks the same for nested properties, as far as I can tell.

Cheers,
Neil

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Rodolfo Zitellini
I did some digging with the debugger (and pretty-print :)
It seems that when overriding nested props are treated exactly like
non-nested ones, as Neil suggested. When you do an \override, in the
CAR of the current property list is placed a new list with the
elements of the same list:
(<many elements>) -> ((<many elements>) <many elements>)

The new value is then prepended, but the old one is maintained in the CAR:
((<the_element> <other elements>) <the_element> <other elements>) ->
(<the_element_new> <the_element> <other elements>) <the_element>
<other elements>)

So we end up with a "backup" copy of the list and the current list in
the CAR, with two copies of the modified prop: old and new values.
Nested properties are treated similary. If you do an override, say in
(bound-details left <an_item>) only the sublist of the nested prop is
duplicated and not the whole prop:

(bound-details (left <my_item> <other_items>)) ->
(bound-details
           (left <my_item_new> <my_item> <other_items>)
           (left <my_item> <other_items>))

I hope I got this correctly! :)

Now, what seems to trigger the problem is that in the debugger I see a
call to execute_revert_property() before every
execute_override_property(). So the code:

\override TrillSpanner #'(bound-details left Y) = #'99

seems to generate a revert on #'(bound-details left Y) before setting
the property.
If this is the first override in that context
execute_revert_property() just returns. BUT if an override was already
done the CAR of it's properties contains the current used properties.
nested_property_revert_alist() will then find the property you are
overriding and it pops it out of the list. After the override you end
up with something like this:

(bound-details
           (left <my_item_new> <other_items>)
           (left <my_item> <other_items>))

note that in the first instance of (left) the old value of <my_item>
is missing, since it was popped by the revert.

When you explicitally do a \reverts on this property,
nested_property_revert_alist() seems to care only of the first (left)
that it encounters, and pops (correctly) <my_item_new>, leaving the
nested property without a copy of <my_item>.
This extends easily to the \revert-one-some-prop breaking after an
\override-another: nested_property_revert_alist() always pops out the
list the \reverted item, even if it is the only copy present. It seems
then that other parts of the program just access the first instance of
the nested prop they encounter, thus not finding it's subproperty,
this even if subsequently in the list other copies of the nested
property (with all elements) are present.
Can't the default value just be copied from the "backup" part of the
context's property alist, which is always present?

Hope I was not too boring :)

cheers,
Rodolfo

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Carl Sorensen



On 5/12/10 11:13 AM, "Rodolfo Zitellini" <[hidden email]> wrote:

> I did some digging with the debugger (and pretty-print :)
> It seems that when overriding nested props are treated exactly like
> non-nested ones, as Neil suggested. When you do an \override, in the
> CAR of the current property list is placed a new list with the
> elements of the same list:
> (<many elements>) -> ((<many elements>) <many elements>)

Remember that these are alists, not lists.  Pretty-print sometimes chooses
to show them in a way that it's not ovbious they are alists.

So, in alist form, I think you get two prepends:  one for left, which gives
you:

(bound-details . ((left . ((Y . new-y-value)
                           (Y . old-y-value)
                           (other-left-prop . other-value)))
                  (other-detail . other-detail-alist)))

and then a second for bound-details, which gives you

(bound-details . ((left . ((Y . new-y-value)
                           (Y . old-y-value)
                           (other-left-prop . other-value)))
                  (other-detail . other-detail-alist))
                  (left . ((Y . old-y-value)
                           (other-left-prop . other-value)))
                  (other-detail . other-detail-alist)))

So you've prepended the new value for Y onto the left alist; then you
prepend the new value of left onto the bound-details alist.

At least, that's my understanding of the way it works. (And in writing this
down, I think I've discovered what I needed to know about my new autobeaming
data structure -- thanks for the insight!)

>
> The new value is then prepended, but the old one is maintained in the CAR:
> ((<the_element> <other elements>) <the_element> <other elements>) ->
> (<the_element_new> <the_element> <other elements>) <the_element>
> <other elements>)
>
> So we end up with a "backup" copy of the list and the current list in
> the CAR, with two copies of the modified prop: old and new values.
> Nested properties are treated similary. If you do an override, say in
> (bound-details left <an_item>) only the sublist of the nested prop is
> duplicated and not the whole prop:
>
> (bound-details (left <my_item> <other_items>)) ->
> (bound-details
>            (left <my_item_new> <my_item> <other_items>)
>            (left <my_item> <other_items>))
>
> I hope I got this correctly! :)
>
> Now, what seems to trigger the problem is that in the debugger I see a
> call to execute_revert_property() before every
> execute_override_property(). So the code:
>
> \override TrillSpanner #'(bound-details left Y) = #'99
>
> seems to generate a revert on #'(bound-details left Y) before setting
> the property.
> If this is the first override in that context
> execute_revert_property() just returns. BUT if an override was already
> done the CAR of it's properties contains the current used properties.
> nested_property_revert_alist() will then find the property you are
> overriding and it pops it out of the list. After the override you end
> up with something like this:
>
> (bound-details
>            (left <my_item_new> <other_items>)
>            (left <my_item> <other_items>))
>
> note that in the first instance of (left) the old value of <my_item>
> is missing, since it was popped by the revert.
>
> When you explicitally do a \reverts on this property,
> nested_property_revert_alist() seems to care only of the first (left)
> that it encounters, and pops (correctly) <my_item_new>, leaving the
> nested property without a copy of <my_item>.
> This extends easily to the \revert-one-some-prop breaking after an
> \override-another: nested_property_revert_alist() always pops out the
> list the \reverted item, even if it is the only copy present. It seems
> then that other parts of the program just access the first instance of
> the nested prop they encounter, thus not finding it's subproperty,
> this even if subsequently in the list other copies of the nested
> property (with all elements) are present.
> Can't the default value just be copied from the "backup" part of the
> context's property alist, which is always present?

We shouldn't need to copy from the "backup" part.  We should just eliminate
the whole prepend, imo.  That is, we should revert Y  in the left sublist;
then we should revert the left sublist in bound-details.  That should get us
back to the original condition.

But that logic only works when we revert exactly what we overrode
previously.  If we had done an override on (bound-details left Y) and then
did a revert on (bound-details left foo), we'd lose the override on
bound-details left Y as well with the logic I described above.

That's the logic that has been giving me grief in my autobeaming code.

I hope this was helpful, and that what I said was reasonably accurate.

Thanks,

Carl


---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Rodolfo Zitellini
Sorry, I preovously replyed only to Carl :)

I did some more investigation on the issue, and I think it all bolis
down simply to the fact that reverting a nested property will revert
it even if it was not overridden before.
Nested reverting works like this:
1) Get the first instance of the nested proeprty list
2) get the first instance of the eventual sublist of properties
3) find the first instance of the property
4) drop it

This works only assuming that you always have two copies (overridden
and original) of the property you are reverting in the very first
instance of it's parent list, with the overridden value in the head of
the list.
I think we can just modify nested_property_revert_alist() to prevent
it from dropping non-overridden values. We can simply check if we find
at least another copy of the property, so we can assume safely that
the first one we drop is the overridden one. If just only one copy is
found we can assume that it is the original, and not drop it.
What do you think about it? I made a slight mod
nested_property_revert_alist() and the idea seems to work, if you want
I can try to post a little patch :)

---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Carl Sorensen



On 5/14/10 4:14 AM, "Rodolfo Zitellini" <[hidden email]> wrote:

> Sorry, I preovously replyed only to Carl :)
>
> I did some more investigation on the issue, and I think it all bolis
> down simply to the fact that reverting a nested property will revert
> it even if it was not overridden before.
> Nested reverting works like this:
> 1) Get the first instance of the nested proeprty list
> 2) get the first instance of the eventual sublist of properties
> 3) find the first instance of the property
> 4) drop it
>
> This works only assuming that you always have two copies (overridden
> and original) of the property you are reverting in the very first
> instance of it's parent list, with the overridden value in the head of
> the list.

So this seems like it should work, and it seems pretty simple (I was working
on some more complicated ways to handle it, IIRC).

Does your algorithm still work if you do something like:

\override MyGrob #'(bound-details left Y)  = #10
\override MyGrob #'(bound-details right X) =  #20
\revert MyGrob #'(bound-details left Y)

And will it work with

\override MyGrob #'(bound-details left Y)  = #10
\override MyGrob #'(bound-details right X) =  #20
\revert MyGrob #'bound-details

> I think we can just modify nested_property_revert_alist() to prevent
> it from dropping non-overridden values. We can simply check if we find
> at least another copy of the property, so we can assume safely that
> the first one we drop is the overridden one. If just only one copy is
> found we can assume that it is the original, and not drop it.
> What do you think about it? I made a slight mod
> nested_property_revert_alist() and the idea seems to work, if you want
> I can try to post a little patch :)

I think that a patch, including a test file (that can be added to our
regression tests) would be a great idea!

Thanks!

Carl



---
----
Join the Frogs!

Reply | Threaded
Open this post in threaded view
|

Re: Stepping into the code for nested properties

Rodolfo Zitellini
>
> Does your algorithm still work if you do something like:
>
> \override MyGrob #'(bound-details left Y)  = #10
> \override MyGrob #'(bound-details right X) =  #20
> \revert MyGrob #'(bound-details left Y)
yes!

> And will it work with
>
> \override MyGrob #'(bound-details left Y)  = #10
> \override MyGrob #'(bound-details right X) =  #20
> \revert MyGrob #'bound-details
no! argh! but this seems broken even without my mod :)
This is the problem as I see it:
When you first override bound-details, you get a copy of bound-details
with two "left" inside: the first has two "Y" properties and the
second is identical to the pre-override one. This bound-details list
is prepended to the current alist. So now you have two bound-details:
this and the original.

The second override works the same way: it makes a copy of the fist
bound-details it encounters (our new one), and prepends it to the
alist, modifying the "right" list in it as above. Now we have *three*
bound-details in the alist.

Then the revert arrives. It drops the first bound-details it
encounters, so now there are two: the one created with the first
override and the original one.
The one used subsequently is the first one in the list, the one with
two "left" sublists.

Please note that with the current version you will end up with two
"left" sublists and an empty "right" sublist since it will trigger the
nested property bug too.

This snippet shows it all:
\relative c' {
  c1\startTrillSpan
  c1\stopTrillSpan
  \override TrillSpanner #'(bound-details left Y) = #90
  \override TrillSpanner #'(bound-details right Y) = #90
  \revert TrillSpanner #'(bound-details)
  c1\startTrillSpan
  c1\stopTrillSpan
}

But this issue for the moment leaves me unprepared :)

Right now I think maybe the easiest solution is, in
execute_override_property(), to check if a copy of the nested list you
are modifying exists, and modify this one instead of making a whole
new copy.

>
> I think that a patch, including a test file (that can be added to our
> regression tests) would be a great idea!

Patch on it's way! please give me a couple days since I should be
working on my thesis right now :)

Ciao,
Rodolfo

---
----
Join the Frogs!