Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(83)

Issue 117050043: Simplify the NullVoice context

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by Keith
Modified:
4 years, 10 months ago
Reviewers:
janek, dak, dan
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Let NullVoice work completely correctly when inside a Staff: Users have begun to use NullVoice in ways that require it to live within a Staff (for example to print solfege above the staff) Note-head interface: add missing property

Patch Set 1 #

Patch Set 2 : Accidentals, engraved by staff, need hidden #

Patch Set 3 : and AccidentalSuggestion #

Total comments: 7

Patch Set 4 : NullVoice a child of Score #

Patch Set 5 : NullVoice a child of Score, but robust enough to move to Staff #

Patch Set 6 : back to Staff #

Total comments: 1

Patch Set 7 : paper over a load of problems #

Patch Set 8 : supress accidentals correctly, using a property of NullVoice #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -18 lines) Patch
M lily/accidental-engraver.cc View 1 chunk +12 lines, -14 lines 0 comments Download
M lily/note-head.cc View 1 chunk +1 line, -0 lines 0 comments Download
M ly/engraver-init.ly View 2 chunks +7 lines, -3 lines 0 comments Download
M ly/performer-init.ly View 1 chunk +1 line, -0 lines 0 comments Download
M scm/define-context-properties.scm View 1 chunk +4 lines, -1 line 0 comments Download

Messages

Total messages: 14
janek
5 years, 1 month ago (2014-07-26 21:14:24 UTC) #1
janek
Hi Keith, sorry for writing so late - i didn't notice this patch until yesterday ...
5 years, 1 month ago (2014-07-26 21:17:35 UTC) #2
janek
Hi Keith, your changes look good overall, but i think that NullVoice could be simplified ...
5 years, 1 month ago (2014-07-27 11:25:41 UTC) #3
dak
https://codereview.appspot.com/117050043/diff/40001/ly/engraver-init.ly File ly/engraver-init.ly (right): https://codereview.appspot.com/117050043/diff/40001/ly/engraver-init.ly#newcode787 ly/engraver-init.ly:787: \override NoteHead.meta.interfaces = #(delete 'rhythmic-head-interface On 2014/07/27 11:25:41, janek ...
5 years, 1 month ago (2014-07-27 11:50:17 UTC) #4
Keith
On 2014/07/27 11:25:41, janek wrote: > https://codereview.appspot.com/117050043/diff/40001/input/regression/lyric-combine-nullvoice.ly#newcode15 > input/regression/lyric-combine-nullvoice.ly:15: >> } > This regtest is ...
5 years, 1 month ago (2014-07-27 21:17:40 UTC) #5
Keith
On 2014/07/27 11:50:17, dak wrote: > https://codereview.appspot.com/117050043/diff/40001/ly/engraver-init.ly#newcode787 > ly/engraver-init.ly:787: \override NoteHead.meta.interfaces = #(delete > 'rhythmic-head-interface ...
5 years, 1 month ago (2014-07-27 21:31:52 UTC) #6
dak
On 2014/07/27 21:31:52, Keith wrote: > On 2014/07/27 11:50:17, dak wrote: > > > https://codereview.appspot.com/117050043/diff/40001/ly/engraver-init.ly#newcode787 ...
5 years, 1 month ago (2014-07-28 06:07:58 UTC) #7
Keith
On Sun, 27 Jul 2014 23:07:58 -0700, <dak@gnu.org> wrote: > The ugliness is with messing ...
5 years, 1 month ago (2014-07-29 04:22:46 UTC) #8
janek
Patchset 4 LGTM :) I agree that it would be nice to separate melisma stuff ...
5 years, 1 month ago (2014-08-02 07:28:02 UTC) #9
dak
https://codereview.appspot.com/117050043/diff/90001/ly/engraver-init.ly File ly/engraver-init.ly (right): https://codereview.appspot.com/117050043/diff/90001/ly/engraver-init.ly#newcode788 ly/engraver-init.ly:788: \override NoteHead.meta.interfaces = #(delete 'rhythmic-head-interface This again messes around ...
5 years ago (2014-09-07 08:21:59 UTC) #10
Keith
On Sun, 07 Sep 2014 01:21:58 -0700, <dak@gnu.org> wrote: > But messing up the basic ...
5 years ago (2014-09-08 05:34:12 UTC) #11
dak
"Keith OHara" <k-ohara5a5a@oco.net> writes: > On Sun, 07 Sep 2014 01:21:58 -0700, <dak@gnu.org> wrote: > ...
5 years ago (2014-09-08 07:54:29 UTC) #12
dan_faithful.be
On Sep 8, 2014, at 03:54 , dak@gnu.org wrote: > "Keith OHara" <k-ohara5a5a@oco.net> writes: > ...
5 years ago (2014-09-08 23:28:31 UTC) #13
dak
5 years ago (2014-09-09 03:17:10 UTC) #14
On 2014/09/08 23:28:31, dan_faithful.be wrote:
> On Sep 8, 2014, at 03:54 , mailto:dak@gnu.org wrote:
> 
> > Again: I would strongly suggest that you backpedal and consider the
> > question "what was my first approach which did not work, for
> > reasons?".
> > 
> > And then we see what it would take to address the reasons.
> 
> How about placing the voices inside a VoiceGroup context, moving the
> Accidental_engraver there, and leaving the NullVoice outside, like in my
recent
> partcombine experiments?
> 
> VoiceGroup would by default consist of nothing, accept any kind of Voice, and
be
> accepted by any kind of Staff.

A solution that reorganizes the default voice hierarchy will cause interference
with all user code fiddling with hierarchy.  At the current point of time, such
code has some place for cross-voice placements of items like stems, ties, or
slurs.  That kind of thing has its place in piano music.  So touching the
default hierarchy should be a last resort.  One can use music functions for
juggling with \accepts/\denies in local context modifications and thus change
the hierarchy ad-hoc just at the place where an effect is desired.

I am not sure I understand your proposal correctly, namely whether you are
thinking of changing stuff like the \defaultchild of Staff.  If you don't, all
VoiceGroup staves would have to be instantiated manually.  If you do, many
things will change.

The former could be used for providing music functions solving a particular
problem.  But it sounds like a solution that has quite a bit of potential of
interfering with other solutions focused around similar problems.  It would, of
course, at least spare us the hackery in the internals of the grob
implementation.  But I think we can do better, see
<URL:https://code.google.com/p/lilypond/issues/detail?id=4093#c6>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b