Hey all, This patch fixes issue 1951 but causes other problems (check out accidental-suggestions.ly). Does ...
12 years, 5 months ago
(2011-10-09 19:29:00 UTC)
#1
Hey all,
This patch fixes issue 1951 but causes other problems (check out
accidental-suggestions.ly). Does anyone have any intuition as to where the
cyclic dependencies are coming from (I don't see how Scripts would rely on
AccidentalSuggestions and vice versa in this patch but not in current master).
Cheers,
MS
On Oct 9, 2011, at 9:29 PM, mtsolo@gmail.com wrote: > Reviewers: , > > Message: ...
12 years, 5 months ago
(2011-10-09 19:53:39 UTC)
#2
On Oct 9, 2011, at 9:29 PM, mtsolo@gmail.com wrote:
> Reviewers: ,
>
> Message:
> Hey all,
>
> This patch fixes issue 1951 but causes other problems (check out
> accidental-suggestions.ly). Does anyone have any intuition as to where
> the cyclic dependencies are coming from (I don't see how Scripts would
> rely on AccidentalSuggestions and vice versa in this patch but not in
> current master).
Problem fixed in newest patch set.
Cheers,
MS
Hey all, The newest patchset adds a debug function to bug.cc. I'd push it as ...
12 years, 5 months ago
(2011-10-10 07:15:35 UTC)
#3
Hey all,
The newest patchset adds a debug function to bug.cc. I'd push it as a separate
commit - it has helped me a lot with this & other patches & I'd like to keep it
in the code base.
Cheers,
MS
On 2011/10/10 07:15:35, MikeSol wrote: > The newest patchset adds a debug function to bug.cc. ...
12 years, 5 months ago
(2011-10-10 07:26:23 UTC)
#4
On 2011/10/10 07:15:35, MikeSol wrote:
> The newest patchset adds a debug function to bug.cc.
Not according to the uploaded patch sets.
> I'd push it as a separate
> commit - it has helped me a lot with this & other patches & I'd like to keep
it
> in the code base.
Just make sure that it can reasonably be found and used starting from reading
the contributor's guide. Material in master should be useful for more than one
person.
On Oct 10, 2011, at 9:26 AM, dak@gnu.org wrote: > On 2011/10/10 07:15:35, MikeSol wrote: ...
12 years, 5 months ago
(2011-10-10 07:40:24 UTC)
#5
On Oct 10, 2011, at 9:26 AM, dak@gnu.org wrote:
> On 2011/10/10 07:15:35, MikeSol wrote:
>> The newest patchset adds a debug function to bug.cc.
>
> Not according to the uploaded patch sets.
>
>
Sorry, box.cc I meant.
>> I'd push it as a separate
>> commit - it has helped me a lot with this & other patches & I'd like
> to keep it
>> in the code base.
>
> Just make sure that it can reasonably be found and used starting from
> reading the contributor's guide. Material in master should be useful
> for more than one person.
>
Yes'r. I think that this will be useful in the same way that Skyline::print is
useful (I use them in tandem).
Cheers,
MS
On 2011/10/10 07:40:24, mike_apollinemike.com wrote: > Yes'r. I think that this will be useful in ...
12 years, 5 months ago
(2011-10-10 08:01:40 UTC)
#6
On 2011/10/10 07:40:24, mike_apollinemike.com wrote:
> Yes'r. I think that this will be useful in the same way that Skyline::print
is
> useful (I use them in tandem).
This is not meant as a criticism of you in this case, but as a general
observation: we have locked away too many parts of Lilypond in C++. The
necessity for writing C++ functions that can be called in the debugger is a
hindrance: one should be able to print everything from Guile at a time
granularity useful for debugging.
Without writing C++ code and without recompilation.
I don't see a master plan how to get there.
On Oct 18, 2011, at 3:56 PM, n.puttock@gmail.com wrote: > > http://codereview.appspot.com/5235052/diff/17001/lily/script-engraver.cc > File lily/script-engraver.cc ...
12 years, 5 months ago
(2011-10-19 08:05:34 UTC)
#8
On Oct 18, 2011, at 3:56 PM, n.puttock@gmail.com wrote:
>
> http://codereview.appspot.com/5235052/diff/17001/lily/script-engraver.cc
> File lily/script-engraver.cc (right):
>
>
http://codereview.appspot.com/5235052/diff/17001/lily/script-engraver.cc#newc...
> lily/script-engraver.cc:206:
> Script_engraver::acknowledge_inline_accidental (Grob_info info)
> Doesn't this add every accidental to a script in a chord?
Yes - I could try adding only the extremal accidentals, but I'm not sure if this
would speed things up.
>
> I'm concerned there's a risk the side-positioning calculation might be
> incorrect, since the accidentals are shuffled around via
> AccidentalPlacement later.
>
The shuffling happens in the Y-offset callback (Accidental's Y-offset is
ly:grob::x-parent-positioning, added in Accidental_placement::add_accidental)
which is calculated when the extent is calculated and reported back to the boxes
(and thus skylines) in Side_position_interface. I don't think
AccidentalPlacement does any subsequent shuffling (although I could be wrong).
>
http://codereview.appspot.com/5235052/diff/17001/lily/side-position-interface.cc
> File lily/side-position-interface.cc (right):
>
>
http://codereview.appspot.com/5235052/diff/17001/lily/side-position-interface...
> lily/side-position-interface.cc:218: b[ax] = e->maybe_pure_extent
> (common[ax], ax, pure, start, end);
> Accidentals have special treatment for more accurate boxes. Could you
> use them instead (or get the skyline from AccidentalPlacement instead)?
>
Good call! I don't think the skylines are reusable, as the skylines in
accidental-placement.cc are horizontal skylines calculated to aid in
accidentals' placement during the loop in position_apes, not vertical skylines
that represent where they actually fall. However,
Accidental_interface::accurate_boxes is a neat function that's in the new
patchset.
> http://codereview.appspot.com/5235052/
On Oct 21, 2011, at 3:41 AM, bordage.bertrand@gmail.com wrote: > LGTM, but the result is ...
12 years, 5 months ago
(2011-10-21 06:48:22 UTC)
#10
On Oct 21, 2011, at 3:41 AM, bordage.bertrand@gmail.com wrote:
> LGTM, but the result is really disturbing. We need to think about a
> better approach of character boxes in MetaFont. The ideal solution
> would be to create a mask for each character. For example, a mask for
> the "espressivo" glyph would be a "fill" between its 6 dots.
>
> I know it's impossible to interpret this mask in C++...
>
Not necessarily - you can stash all sorts of information in font tables, which
are then accessible via the otf_tables.
The same thing needs to be done for flags, which have been glued to stems via a
kludge in the X-offset function for some time.
Janek was toying around with font tables last time I checked in with him - it'd
take me some time to learn how the metafont part of the code works, so if this
is something you think you could tackle, I'd be able to implement the C++ side.
> Anyway, I think this patch can be pushed as the first part of a fix to
> issue 1951.
>
> Bertrand
>
>
> http://codereview.appspot.com/5235052/diff/23001/lily/script-engraver.cc
> File lily/script-engraver.cc (right):
>
>
http://codereview.appspot.com/5235052/diff/23001/lily/script-engraver.cc#newc...
> lily/script-engraver.cc:208: int script_count = scripts_.size ();
> vsize instead of int. Could you change the others above?
fixed.
>
>
http://codereview.appspot.com/5235052/diff/23001/lily/script-engraver.cc#newc...
> lily/script-engraver.cc:209: for (int i = 0; i < script_count; i++)
> vsize.
fixed.
>
> http://codereview.appspot.com/5235052/diff/23001/lily/slur.cc
> File lily/slur.cc (right):
>
> http://codereview.appspot.com/5235052/diff/23001/lily/slur.cc#newcode275
> lily/slur.cc:275: // cannot use is empty because some 0-extent scripts
> I don't understand. Did you meant "if empty"?
I meant is_empty (the function).
Cheers,
MS
>> [...] We need to think about a better approach of character boxes >> in ...
12 years, 5 months ago
(2011-10-21 10:37:40 UTC)
#11
>> [...] We need to think about a better approach of character boxes
>> in MetaFont. The ideal solution would be to create a mask for each
>> character. [...]
>
> Not necessarily - you can stash all sorts of information in font
> tables, which are then accessible via the otf_tables.
Exactly. The meta-ness really helps here in defining auxiliary points
which could serve for better metrics instead of plain boxes. If you
look up the mailing list, such ideas have already been presented years
ago; cf. this thread:
http://lists.gnu.org/archive/html/lilypond-devel/2006-11/msg00162.html
Werner
Issue 5235052: First stab at getting script offsets right.
(Closed)
Created 12 years, 5 months ago by MikeSol
Modified 12 years, 5 months ago
Reviewers: mike_apollinemike.com, dak, Neil Puttock, Bertrand Bordage, wl_gnu.org
Base URL:
Comments: 5