|
|
Descriptioncleanup: infinity, get property
add a callback for calculating lyric syllables offset
Patch Set 1 #
Total comments: 16
Patch Set 2 : move things to lyric_text #Patch Set 3 : no code dup. works properly. #Patch Set 4 : here's how i tried to introduce lyricDefaultAlignment #Patch Set 5 : inherit from Item #Patch Set 6 : use grob property instead of context property #
Total comments: 1
MessagesTotal messages: 16
http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc File lily/lyric-engraver.cc (right): http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode42 lily/lyric-engraver.cc:42: TRANSLATOR_DECLARATIONS (Lyric_engraver); You'll need to create a lyric-text.cc and put this in there. Then, in ADD_INTERFACE (Lyric_text, "foo" "bar" "etc.") you'll need to add the property min-X-offset. http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode60 lily/lyric-engraver.cc:60: /* unless there is a melisma or the user explicitly specifies X-alignment, I don't see code in here dealing with melismas. http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode75 lily/lyric-engraver.cc:75: If self-alignment-X is specified, then call something from Side_position_interface to avoid the code dup. http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode76 lily/lyric-engraver.cc:76: Real restriction = scm_is_number (align_prop) align_prop will always be a number, as self-alignment-X for LyricText is currently set to CENTER in define-grobs.scm http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode92 lily/lyric-engraver.cc:92: return scm_from_double (max (offset, restriction)); I would go about this differently. If the user doesn't want a min-X-offset, she can change it to -inf.0 manually. That way, you don't need to build that logic in. So, all this can become something like: MAKE_SCHEME_CALLBACK (Lyric_engraver, calc_x_offset, 1); SCM Lyric_text::calc_x_offset (SCM smob) { Grob *me = unsmob_grob (smob); Real min_xo = robust_scm2double (me->get_property ("min-X-offset"), 0.0); Real current_xo = robust_scm2double (Self_alignment_interface::aligned_on_parent (grob, X_AXIS), 0.0); return scm_from_double (max (current_xo, min_xo)); } http://codereview.appspot.com/6310043/diff/1/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/6310043/diff/1/scm/define-grob-properties.scm#n... scm/define-grob-properties.scm:584: more than this to the left of the syllable") Period at the end of the sentence. http://codereview.appspot.com/6310043/diff/1/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/6310043/diff/1/scm/define-grobs.scm#newcode1283 scm/define-grobs.scm:1283: (X-offset . ,ly:lyric-engraver::calc-x-offset) You want this to be ly:lyric-text::calc-x-offset
Sign in to reply to this message.
there are some disputable points, but generally i will send new version (hopefully today). http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc File lily/lyric-engraver.cc (right): http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode42 lily/lyric-engraver.cc:42: TRANSLATOR_DECLARATIONS (Lyric_engraver); On 2012/06/14 08:19:01, MikeSol wrote: > You'll need to create a lyric-text.cc and put this in there. > > Then, in ADD_INTERFACE (Lyric_text, "foo" "bar" "etc.") you'll need to add the > property min-X-offset. will do. http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode60 lily/lyric-engraver.cc:60: /* unless there is a melisma or the user explicitly specifies X-alignment, On 2012/06/14 08:19:01, MikeSol wrote: > I don't see code in here dealing with melismas. When there's a melisma, aling_prop will be set. However, the wording is perhaps misleading. http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode75 lily/lyric-engraver.cc:75: On 2012/06/14 08:19:01, MikeSol wrote: > If self-alignment-X is specified, then call something from > Side_position_interface to avoid the code dup. You mean Self_alignment_interface? http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode76 lily/lyric-engraver.cc:76: Real restriction = scm_is_number (align_prop) On 2012/06/14 08:19:01, MikeSol wrote: > align_prop will always be a number, as self-alignment-X for LyricText is > currently set to CENTER in define-grobs.scm Nope, i deleted self-alignment-X in define-grobs.scm. So it won't be a number unless the user sets it /or/ the syllable is a melisma, in which case Lyric_engraver will set it's alignment to "lyricMelismaAlignment". http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode92 lily/lyric-engraver.cc:92: return scm_from_double (max (offset, restriction)); On 2012/06/14 08:19:01, MikeSol wrote: > I would go about this differently. If the user doesn't want a min-X-offset, she > can change it to -inf.0 manually. That way, you don't need to build that logic > in. Not quite. The user may want min-X-offset for "default syllables", but not for ones with self-X-alignment tweaked. Consider this case: \relative f' { \partial 8 g8 g'8. f16 e8. d16 d4 } \addlyrics { Be -- hold the Lamb of God! } % i don't like the placement of the lyrics, so i change it using handy shortcuts: lyrLeft = { \once \override LyricText #'self-alignment-X = #0.8 } lyrRight = { \once \override LyricText #'self-alignment-X = #-0.7 } \relative f' { \partial 8 g8 g'8. f16 e8. d16 d4 } \addlyrics { Be -- hold \lyrLeft the \lyrRight Lamb of \lyrRight God! } % now, i don't want Lily to mess with my custom syllable placement - % i don't want any min-X-offset applied to them, ever! % but i want min-X-offset applied to syllables that i don't tweak. % Does this explain why we cannot do this as you suggested? http://codereview.appspot.com/6310043/diff/1/scm/define-grob-properties.scm File scm/define-grob-properties.scm (right): http://codereview.appspot.com/6310043/diff/1/scm/define-grob-properties.scm#n... scm/define-grob-properties.scm:584: more than this to the left of the syllable") On 2012/06/14 08:19:01, MikeSol wrote: > Period at the end of the sentence. Done. http://codereview.appspot.com/6310043/diff/1/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/6310043/diff/1/scm/define-grobs.scm#newcode1283 scm/define-grobs.scm:1283: (X-offset . ,ly:lyric-engraver::calc-x-offset) On 2012/06/14 08:19:01, MikeSol wrote: > You want this to be ly:lyric-text::calc-x-offset ok
Sign in to reply to this message.
Hi, i've moved things around. I've used your suggestion to check if it works at all - it works, but the results are wrong ;) That's not a problem, though; I'll fix them tomorrow. Meanwhile please take a look at my previous comments in Lyric_engraver (before the function was moved): http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc File lily/lyric-engraver.cc (right): http://codereview.appspot.com/6310043/diff/1/lily/lyric-engraver.cc#newcode42 lily/lyric-engraver.cc:42: TRANSLATOR_DECLARATIONS (Lyric_engraver); On 2012/06/14 08:19:01, MikeSol wrote: > You'll need to create a lyric-text.cc and put this in there. > > Then, in ADD_INTERFACE (Lyric_text, "foo" "bar" "etc.") you'll need to add the > property min-X-offset. Done. http://codereview.appspot.com/6310043/diff/1/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/6310043/diff/1/scm/define-grobs.scm#newcode1283 scm/define-grobs.scm:1283: (X-offset . ,ly:lyric-engraver::calc-x-offset) On 2012/06/14 18:40:02, janek wrote: > On 2012/06/14 08:19:01, MikeSol wrote: > > You want this to be ly:lyric-text::calc-x-offset > > ok Done.
Sign in to reply to this message.
How do you like this? I've tried to define a new property lyricDefaultAlignment (similar to lyricMelismaAlignment) and use it in line 47, but i got an error: 'internal_get_property' was not declared in this scope I'll investigate when i get back home.
Sign in to reply to this message.
On 2012/06/15 09:22:28, janek wrote: > How do you like this? > > I've tried to define a new property lyricDefaultAlignment (similar to > lyricMelismaAlignment) and use it in line 47, but i got an error: > 'internal_get_property' was not declared in this scope > > I'll investigate when i get back home. What makes you think Lyric_text has properties? It is a class you defined yourself. I really can't figure out what you are trying to do here.
Sign in to reply to this message.
I forgot that my git-cl is configured to add -devel to cc! This is a draft which i was discussing with Mike, but since it got public, i welcome any comments! On 2012/06/15 09:42:03, dak wrote: > On 2012/06/15 09:22:28, janek wrote: > > How do you like this? > > > > I've tried to define a new property lyricDefaultAlignment (similar to > > lyricMelismaAlignment) and use it in line 47, but i got an error: > > 'internal_get_property' was not declared in this scope > > > > I'll investigate when i get back home. > > What makes you think Lyric_text has properties? It is a class you defined > yourself. > > I really can't figure out what you are trying to do here. I'm trying to define a new context property lyricDefaultAlignment similarly to how lyricMelismaAlignment is defined. (did i use the right name? lyricMelismaAlignment is a context property, i.e. a property that doesn't belong to a specific grob?) Look at Lyric_engraver, lines 76-77: last_text_->set_property ("self-alignment-X", get_property ("lyricMelismaAlignment")); unless i'm totally wrong, last_text_ is a pointer to a LyricText item. These two lines set that LyricText's self-alignment-X to what is inside lyricMelismaAlignment. I want to do something similar: in Lyric_text, i have a pointer to LyricText item and i want to set its self-alignment to what's inside another property: lyricDefaultAlignment. > What makes you think Lyric_text has properties? Does it need any? I'm trying to read a context property (or at least that's what i think i'm trying), i thought that they don't need to be "owned". > It is a class you defined yourself. I've looked at other class definitions (in particular, flag.cc) and tried to define Lyric_text accordingly to what i saw. Apparently i missed something, but i don't know what :( I thought that ADD_INTERFACE is responsible for connecting my class with the rest of the code, and MAKE_SCHEME_CALLBACK makes a method available in Scheme "layer". I don't have any idea what else is necessary... I'd be grateful for an explanation. thanks, Janek
Sign in to reply to this message.
Janek Warchoł <janek.lilypond@gmail.com> writes: > I forgot that my git-cl is configured to add -devel to cc! > This is a draft which i was discussing with Mike, but since it got > public, i welcome any comments! > > On 2012/06/15 09:42:03, dak wrote: >> On 2012/06/15 09:22:28, janek wrote: >> > How do you like this? >> > >> > I've tried to define a new property lyricDefaultAlignment (similar to >> > lyricMelismaAlignment) and use it in line 47, but i got an error: >> > 'internal_get_property' was not declared in this scope >> > >> > I'll investigate when i get back home. >> >> What makes you think Lyric_text has properties? It is a class you defined >> yourself. >> >> I really can't figure out what you are trying to do here. > > I'm trying to define a new context property lyricDefaultAlignment > similarly to how lyricMelismaAlignment is defined. > (did i use the right name? lyricMelismaAlignment is a context > property, i.e. a property that doesn't belong to a specific grob?) > Look at Lyric_engraver, lines 76-77: > > last_text_->set_property ("self-alignment-X", > get_property ("lyricMelismaAlignment")); > > unless i'm totally wrong, last_text_ is a pointer to a LyricText item. > These two lines set that LyricText's self-alignment-X to what is > inside lyricMelismaAlignment. But this is inside of a member function of an _Engraver_. An engraver has an associated context, and get_property inside of an engraver will access the respective context property. > I want to do something similar: in Lyric_text, i have a pointer to > LyricText item and i want to set its self-alignment to what's inside > another property: lyricDefaultAlignment. > >> What makes you think Lyric_text has properties? > > Does it need any? I'm trying to read a context property (or at least > that's what i think i'm trying), i thought that they don't need to be > "owned". What do you think the word "context" in "context property" means? How should the C++ compiler guess what context you are talking about when you just write get_property ("lyricMelismaAlignment") ? >> It is a class you defined yourself. > > I've looked at other class definitions (in particular, flag.cc) and > tried to define Lyric_text accordingly to what i saw. Apparently i > missed something, but i don't know what :( You call a member function get_property (or rather via a macro definition internal_get_property) but don't define it. > I thought that ADD_INTERFACE is responsible for connecting my class > with the rest of the code, and MAKE_SCHEME_CALLBACK makes a method > available in Scheme "layer". I don't have any idea what else is > necessary... > I'd be grateful for an explanation. Again: how should LilyPond guess the context from which you want the property read? The code will, of course, depend on the answer to that question. -- David Kastrup
Sign in to reply to this message.
On Fri, Jun 15, 2012 at 5:04 PM, David Kastrup <dak@gnu.org> wrote: > Janek Warchoł <janek.lilypond@gmail.com> writes: > >> I'm trying to define a new context property lyricDefaultAlignment >> similarly to how lyricMelismaAlignment is defined. >> (did i use the right name? lyricMelismaAlignment is a context >> property, i.e. a property that doesn't belong to a specific grob?) >> Look at Lyric_engraver, lines 76-77: >> >> last_text_->set_property ("self-alignment-X", >> get_property ("lyricMelismaAlignment")); >> >> unless i'm totally wrong, last_text_ is a pointer to a LyricText item. >> These two lines set that LyricText's self-alignment-X to what is >> inside lyricMelismaAlignment. > > But this is inside of a member function of an _Engraver_. An engraver > has an associated context, and get_property inside of an engraver will > access the respective context property. Ah, indeed - silly me! >>> What makes you think Lyric_text has properties? >> >> Does it need any? I'm trying to read a context property (or at least >> that's what i think i'm trying), i thought that they don't need to be >> "owned". > > What do you think the word "context" in "context property" means? How > should the C++ compiler guess what context you are talking about when > you just write get_property ("lyricMelismaAlignment") ? I thought that since i have a pointer to a LyricText Item, which is placed in some context, that context would be used. From what i see in Context::internal_get_property, i can start from a child context of the context i need (i.e. if MyProperty isn't set for Staff, but it is set for Score, i can start from Staff and it will go up to Score). >>> It is a class you defined yourself. >> >> I've looked at other class definitions (in particular, flag.cc) and >> tried to define Lyric_text accordingly to what i saw. Apparently i >> missed something, but i don't know what :( > > You call a member function get_property (or rather via a macro > definition internal_get_property) but don't define it. Should i #include context.hh then? >> I thought that ADD_INTERFACE is responsible for connecting my class >> with the rest of the code, and MAKE_SCHEME_CALLBACK makes a method >> available in Scheme "layer". I don't have any idea what else is >> necessary... >> I'd be grateful for an explanation. > > Again: how should LilyPond guess the context from which you want the > property read? The code will, of course, depend on the answer to that > question. I'd like to use the parent context of the Item that i have. Can it be done this way? Could you point me to an example? thanks, Janek
Sign in to reply to this message.
Janek Warchoł <janek.lilypond@gmail.com> writes: > On Fri, Jun 15, 2012 at 5:04 PM, David Kastrup <dak@gnu.org> wrote: >> Janek Warchoł <janek.lilypond@gmail.com> writes: >> >>> I'm trying to define a new context property lyricDefaultAlignment >>> similarly to how lyricMelismaAlignment is defined. >>> (did i use the right name? lyricMelismaAlignment is a context >>> property, i.e. a property that doesn't belong to a specific grob?) >>> Look at Lyric_engraver, lines 76-77: >>> >>> last_text_->set_property ("self-alignment-X", >>> get_property >>> ("lyricMelismaAlignment")); >>> >>> unless i'm totally wrong, last_text_ is a pointer to a LyricText item. >>> These two lines set that LyricText's self-alignment-X to what is >>> inside lyricMelismaAlignment. >> >> But this is inside of a member function of an _Engraver_. An >> engraver has an associated context, and get_property inside of an >> engraver will access the respective context property. > > Ah, indeed - silly me! > >>>> What makes you think Lyric_text has properties? >>> >>> Does it need any? I'm trying to read a context property (or at least >>> that's what i think i'm trying), i thought that they don't need to be >>> "owned". >> >> What do you think the word "context" in "context property" means? >> How should the C++ compiler guess what context you are talking about >> when you just write get_property ("lyricMelismaAlignment") ? > > I thought that since i have a pointer to a LyricText Item, which is > placed in some context, that context would be used. What makes you think LyricText is an Item? Its definition is class Lyric_text { public: DECLARE_SCHEME_CALLBACK (calc_x_offset, (SCM)); DECLARE_GROB_INTERFACE (); }; It is a class with one member function and an interface. That does not make it an Item. If you want it to be an Item, you need to have it inherit from Item in which case it will know its context (and consequently how to get properties) from the way it has been created. >>> I thought that ADD_INTERFACE is responsible for connecting my class >>> with the rest of the code, and MAKE_SCHEME_CALLBACK makes a method >>> available in Scheme "layer". I don't have any idea what else is >>> necessary... >>> I'd be grateful for an explanation. >> >> Again: how should LilyPond guess the context from which you want the >> property read? The code will, of course, depend on the answer to that >> question. > > I'd like to use the parent context of the Item that i have. So far, you don't have an Item that I can see. -- David Kastrup
Sign in to reply to this message.
On Fri, Jun 15, 2012 at 6:00 PM, David Kastrup <dak@gnu.org> wrote: > What makes you think LyricText is an Item? Its definition is > > class Lyric_text > { > public: > DECLARE_SCHEME_CALLBACK (calc_x_offset, (SCM)); > DECLARE_GROB_INTERFACE (); > }; > > It is a class with one member function and an interface. That does not > make it an Item. If you want it to be an Item, you need to have it > inherit from Item in which case it will know its context (and > consequently how to get properties) from the way it has been created. indeed, i'm more silly than i thought. Is the inheritance correct now? I still get an error, though... Btw, why doesn't Flag inherit from Item? thanks, Janek
Sign in to reply to this message.
Janek Warchoł <janek.lilypond@gmail.com> writes: > On Fri, Jun 15, 2012 at 6:00 PM, David Kastrup <dak@gnu.org> wrote: >> What makes you think LyricText is an Item? Its definition is >> >> class Lyric_text >> { >> public: >> DECLARE_SCHEME_CALLBACK (calc_x_offset, (SCM)); >> DECLARE_GROB_INTERFACE (); >> }; >> >> It is a class with one member function and an interface. That does not >> make it an Item. If you want it to be an Item, you need to have it >> inherit from Item in which case it will know its context (and >> consequently how to get properties) from the way it has been created. > > indeed, i'm more silly than i thought. > Is the inheritance correct now? > I still get an error, though... > Btw, why doesn't Flag inherit from Item? Good question. Looking through this and stem, it would seem that this does not work via inheritance but rather callbacks. So disregard what I said previously. You likely want to use a grob property, in which case you would write me->get_property (...). Using a context property does not make sense here since the lifetime of a grob is not synchronized to the lifetime of context properties. -- David Kastrup
Sign in to reply to this message.
On Fri, Jun 15, 2012 at 6:48 PM, David Kastrup <dak@gnu.org> wrote: > Janek Warchoł <janek.lilypond@gmail.com> writes: >> indeed, i'm more silly than i thought. >> Is the inheritance correct now? >> I still get an error, though... >> Btw, why doesn't Flag inherit from Item? > > Good question. Looking through this and stem, it would seem that this > does not work via inheritance but rather callbacks. So disregard what I > said previously. Should Lyric_text inherit from Item, then? Or is it totally irrelevant? > You likely want to use a grob property, in which case you would write > me->get_property (...). Done - works without any problem :) Should i also change lyricMelismaAlignment to be a grob property? > Using a context property does not make sense > here since the lifetime of a grob is not synchronized to the lifetime of > context properties. You mean that a context property may be changed during the lifetime of one grob? thanks for help! Janek http://codereview.appspot.com/6310043/diff/2008/scm/define-grobs.scm File scm/define-grobs.scm (right): http://codereview.appspot.com/6310043/diff/2008/scm/define-grobs.scm#newcode1282 scm/define-grobs.scm:1282: (default-alignment . 0) should i name it 'default-alignment-X'?
Sign in to reply to this message.
janek.lilypond@gmail.com writes: > On Fri, Jun 15, 2012 at 6:48 PM, David Kastrup <dak@gnu.org> wrote: >> Janek Warchoł <janek.lilypond@gmail.com> writes: >>> indeed, i'm more silly than i thought. >>> Is the inheritance correct now? >>> I still get an error, though... >>> Btw, why doesn't Flag inherit from Item? > >> Good question. Looking through this and stem, it would seem that this >> does not work via inheritance but rather callbacks. So disregard >> what I said previously. > > Should Lyric_text inherit from Item, then? Apparently not. > Or is it totally irrelevant? Possibly. I consider it likely that this class is never instantiated, but I don't have enough of a clue right now. >> You likely want to use a grob property, in which case you would write >> me->get_property (...). > > Done - works without any problem :) > Should i also change lyricMelismaAlignment to be a grob property? No idea. >> Using a context property does not make sense here since the lifetime >> of a grob is not synchronized to the lifetime of context properties. > > You mean that a context property may be changed during the lifetime of > one grob? Sure. The lifetime of a grob is until the output is produced. The life time of the interface is likely more interesting. > http://codereview.appspot.com/6310043/diff/2008/scm/define-grobs.scm > File scm/define-grobs.scm (right): > > http://codereview.appspot.com/6310043/diff/2008/scm/define-grobs.scm#newcode1282 > scm/define-grobs.scm:1282: (default-alignment . 0) > should i name it 'default-alignment-X'? No idea. Someone else needs to answer this. -- David Kastrup
Sign in to reply to this message.
Ok, I am currently somewhat fuzzy. A grob has individual properties, and it has interfaces. Interfaces also carry properties. When a grob is created, its properties are initialized from the context and then live on independently. I think that the interface-induced properties initialize from context properties unless overriden per grob. But I really don't have enough of a clue of the backend to really tell right now.
Sign in to reply to this message.
On 15 juin 2012, at 22:10, dak@gnu.org wrote: > Ok, I am currently somewhat fuzzy. A grob has individual properties, > and it has interfaces. Interfaces also carry properties. When a grob > is created, its properties are initialized from the context and then > live on independently. > > I think that the interface-induced properties initialize from context > properties unless overriden per grob. But I really don't have enough of > a clue of the backend to really tell right now. > > http://codereview.appspot.com/6310043/ For a grob to have a property, it must have an interface that implements that property. What do you mean by interface induced property? If a grob implements an interface with property X, said grob does not have a value set for property X unless it is explicitly defined in define-grobs.scm. Cheers, MS
Sign in to reply to this message.
On Fri, Jun 15, 2012 at 9:58 PM, <dak@gnu.org> wrote: >>> Good question. Looking through this and stem, it would seem that >>> this does not work via inheritance but rather callbacks. So disregard >>> what I said previously. >> >> Should Lyric_text inherit from Item, then? > > Apparently not. ok, it doesn't. >>> You likely want to use a grob property, in which case you would write >>> me->get_property (...). >> >> Done - works without any problem :) >> Should i also change lyricMelismaAlignment to be a grob property? > > No idea. i think i'll change it. On Fri, Jun 15, 2012 at 10:10 PM, <dak@gnu.org> wrote: > Ok, I am currently somewhat fuzzy. A grob has individual properties, > and it has interfaces. Interfaces also carry properties. When a grob > is created, its properties are initialized from the context and then > live on independently. > > I think that the interface-induced properties initialize from context > properties unless overriden per grob. But I really don't have enough of > a clue of the backend to really tell right now. Nevertheless many thanks for explanations! I understand things better now. cheers, Janek
Sign in to reply to this message.
|