Originally created by: *anonymous
Originally created by: lemzw...@googlemail.com
Originally owned by: k-ohara5...@oco.net
\version "2.17.11"
\header { texidoc = "
Gould says (p.~56) that in cases where a dot would need to be two or more
staff spaces away from the chord, you only need to use enough dots to cover
the number of spaces taken up by the chord.
Here is a counterexample.
" }
\relative c' {
<d e f g a b c d e f g>2.
}
Originally posted by: lemzw...@googlemail.com
(No comment was entered for this change.)
Owner: lemzw...@googlemail.com
Status: Started
Originally posted by: lemzw...@googlemail.com
Avoid excessive number of dots in chords (#3179).
http://codereview.appspot.com/7319049
Labels: Patch-new
Originally posted by: pkx1...@gmail.com
Patchy the autobot says: See https://www.yousendit.com/download/UW13Y05oZ1B0TWs5WThUQw for diffs, we're losing dots in other reg tests now
Labels: -Patch-new Patch-needs_work
Originally posted by: lemzw...@googlemail.com
Add missing conditional test.
http://codereview.appspot.com/7319049
Labels: -Patch-needs_work Patch-new
Originally posted by: lemzw...@googlemail.com
Whoever is going to review this patch: I would like to contribute not only a functionally correct patch but a structurally correct one, this is, the patch should fix the issue at the *expected* spot. My knowledge of Lilypond's various layout stages is too limited yet...
Thanks in advance.
Originally posted by: pkx1...@gmail.com
passes make and make test and make doc and the reg tests that show up seem ok. however for the sake of brevity for now I have attached one reg rest diff i think is now broken.
so this patch still needs work I think. if not put it back to 'new' and I;ll attach the remaining diffs that I think are expected. Just trying to save timne and bandwidth
Labels: -Patch-new Patch-needs_work
Originally posted by: pkx1...@gmail.com
(No comment was entered for this change.)
Originally posted by: lemzw...@googlemail.com
Make it work for unusual Y-offset values.
http://codereview.appspot.com/7319049
Labels: -Patch-needs_work Patch-new
Originally posted by: lemzw...@googlemail.com
I have a fundamental problem: My fix should work for the default situation, but the user should still be able to override it. In particular, setting the `staff-position' property of the Dots grob to non-standard values makes a dot disappear, which is not optimal.
How is this handled in lilypond in general? This is, do this if the property is set, and do that it the property is not set. Normally, lilypond returns value 0.0 if a numeric property isn't set, but you can't decide whether this is a default value or the user has set it explicitly to 0.0...
I've looked up the source code but wasn't able to identify a spot with a similar issue.
A possible solution is to add a property, say, `chord-dots' which switches the new behaviour on and off (where `on' is the default).
Originally posted by: dak@gnu.org
> Normally, lilypond returns value 0.0 if a numeric property isn't set
No, it doesn't. LilyPond returns SCM_EOL (the Scheme empty list constant, '()) for unset properties, regardless of the property's type. Only if you use robustscm2number on that or whatever it is called do you get whatever you specify as second argument.
The usual way, for that reason, is to use scm_is_number on the result of the property getting call and consider the property unset if it is not a number.
Originally posted by: lemzw...@googlemail.com
Ah yes, thanks. Unfortunately, it doesn't work for my case. Reason is this line in
define-grobs.scm, I think:
(staff-position . ,dots::calc-staff-position)
It makes `staff-position' always evaluate to a number, as far as I can see, if I try to extract it with `get_property'.
I could remove the above line from define-grobs.scm, converting `calc-staff-position' to a C++ function which provides the default position so that `staff-position' becomes a real user-definable property which is not set by default. However, I have big doubts whether this is the right way to go...
Originally posted by: dak@gnu.org
get_property_data should tell you whether you are dealing with straight override or callback function. But since there is nothing wrong with overriding with a callback function, this is a bit of a slippery slope.
Originally posted by: lemzw...@googlemail.com
Exactly. On the other hand, `dots::calc-staff-position' returns non-zero values for rests only. Hmm, tricky. BTW, how could I check in C++ that the property `staff-position' equals `dots::calc-staff-position' (defined in output-lib.scm)?
Originally posted by: pkx1...@gmail.com
Werner,
I have attached the full reg test output here
https://www.yousendit.com/download/UW13Y05vYXlGOFROUjhUQw
this is from the most recent patch. I suddenly noticed a missing dot or two on the 'ledger lines various' test (one line above and also when the note is sat on the first line of the staff).
So I expect I missed these and others the first time round as well - sorry.
Hence my attaching the whole lot for the 'many' to look at and check - one or two dots in a sea of notes are easy to miss.
Summary: too many dots in chords
Labels: -Patch-new Patch-needs_work
Originally posted by: lemzw...@googlemail.com
James, thanks for the test. However, this can't be the right zipfile, I believe. For example, using the current patch set 3, the file `augmentum.ly' properly shows the three dots on my GNU/Linux box, while `out-test/augmentum.png' in your zipfile doesn't. Please check.
Originally posted by: lemzw...@googlemail.com
Continuing comment #13: After some thinking I believe there are really only two options:
1. Move `dots::calc-staff-position' to C++ so that the `staff-position' property can be freely overridden by the user, not needing a callback but rather not set by default.
2. As already mentioned in comment #9, add a new property `chord-dots', ##t by default. If people are going to modify `staff-position' in a weird way, `chord-dots' should be set to ##f.
Do you have better ideas? I prefer option 2.
Originally posted by: pkx1...@gmail.com
(No comment was entered for this change.)
Labels: -Patch-needs_work Patch-new
Originally posted by: pkx1...@gmail.com
https://www.yousendit.com/download/UW14MFhucHZENlRvS3NUQw
reg test diffs.
Labels: -Patch-new Patch-needs_work
Originally posted by: k-ohara5...@oco.net
On the questions about Dots.staff-position, the third and best way is to honor these requested positions when deciding which dots to remove.
The Dots on a double-dotted thirty-second rest, [r32].., has staff-position 3, relative to the staff-position of the rest, indicating the ideal position for these dots is three positions above the reference line of the rest.
The code Werner is working on receives these requested staff-positions, shifted by the positions of the parent NoteHeads or Rests, and tries to place all the dots as closely as possible to their requests.
Werner's filter should look for dots that have been pushed outside the range of requested Dots-positions for that chord, not necessarily the same as the range of NoteHeads in that chord.
Originally posted by: lemzw...@googlemail.com
Regarding comment #18: Thanks a lot! This helps, and I will fix the problems soon.
Regarding comment #19: My code doesn't affect dotted rests at all (there are still small bugs in my code which actually make the dots of some rests disappear, but this will go away in the next patch set). I'm talking about the case where a user arbitrarily changes `staff-position' for dots of note heads (using e.g. \tweak) to get some weird optical effects.
I fear that what you say in the last paragraph doesn't work. Following your advice, I had to compare the default value of `staff-position' with the actual value to `look for dots outside of the requested dot positions'. And exactly this is the problem: There is *no* way to get the default! I *could* assume that the default is the value zero (this is what dots::calc-staff-position returns for note heads), but this would essentially mean hard-coding something in the binary which is defined in the Scheme code. Not a good idea, I believe.
Am I missing something?
Originally posted by: k-ohara5...@oco.net
My thinking was: you know where the collision-resolution algorithm put the dots, you know the positions requested by the current staff-position settings, so remove any dots that were pushed outside the range of requested positions.
Why do you care about the default value of staff-position ?
Removing dots by default would need to be done carefully. Here is Chopin Op27:1 meas41.
\relative c'' {\time 3/4 << {<c c'>4.. q16 q8 q} \\ <g' b>2. >>
If the chords were a bit more crowded the double-dot could be pushed above the space taken by the chord, but we should keep the double-dot.
Originally posted by: lemzw...@googlemail.com
Hmm. Have a look at the dot-column-engraver.ly regtest to see what Lilypond currently does, in particular the first note in the fourth beat (in the first staff). The solution with two dots looks strange, and indeed, checking my old Beethoven edition of sonatas:
http://conquest.imslp.info/files/imglnks/usimg/b/b0/IMSLP00018-Beethoven__L.v._-_Piano_Sonata_18.pdf
Look at page 352, bar 212 how to do it correctly. So dropping the upper dot (as my solution will do) only removes an uglyness without correcting it to the better :-)
Originally posted by: lemzw...@googlemail.com
Should pass all regression tests now.
http://codereview.appspot.com/7319049
Labels: -Patch-needs_work Patch-new
Originally posted by: lemzw...@googlemail.com
For reference, here's a scan of the Beethoven sonata (taken from http://conquest.imslp.info/files/imglnks/usimg/a/a8/IMSLP05544-Btsn31_3.pdf).
Originally posted by: lemzw...@googlemail.com
(No comment was entered for this change.)