Menu
▾
▴
#3629 Patch: Improvements to \offset
Originally created by: *anonymous
Originally created by: david.na...@gmail.com
Originally owned by: david.na...@gmail.com
Improvements to \offset
Property can be specified with or without #' prefix.
Directed tweaks are possible.
Regtest:
--remove #' prefix before property names
--remove unnecessary # before numbers
--add example showing directed tweaks
Revise docstring of find-value-to-offset in `scm/music-functions.scm.'
Originally posted by: pkx1...@gmail.com
Passes Make, make check and a full make doc.
One reg test shows up, attached the last line as that is the only thing that seems to change.
Labels: -Patch-new Patch-review
Owner: david.na...@gmail.com
Originally posted by: david.na...@gmail.com
Yes, that's expected--I added an example to demonstrate directed tweaks.
Originally posted by: david.na...@gmail.com
Improvements to \offset
Property can be specified with or without #' prefix.
Directed tweaks are possible.
Regtest:
--remove #' prefix before property names
--remove unnecessary # before numbers
--add example showing directed tweaks
Revise docstring of find-value-to-offset in `scm/music-functions.scm.'
http://codereview.appspot.com/15850044
Labels: -Patch-review Patch-new
Originally posted by: david.na...@gmail.com
Improvements to \offset
Property can be specified with or without #' prefix.
Directed tweaks are possible.
Regtest:
--remove #' prefix before property names
--remove unnecessary # before numbers
--add example showing directed tweaks
Revise docstring of find-value-to-offset in `scm/music-functions.scm.'
http://codereview.appspot.com/15850044
Originally posted by: pkx1...@gmail.com
Passes make, make check and full make doc.
Reg test gives me same picture as in comment #1, but I noticed today that I get this:
--snip--
@@ -3,23 +3,23 @@
Interpreting music...
programming error: Grob `DynamicText' has no interface for property `padding'
continuing, cross fingers
-[8][16][16]
+[8][16]
Preprocessing graphical objects...
Calculating line breaks...
Drawing systems...
-Writing header field `texidoc' to `/tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391.texidoc'...
-Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-1.signature
-Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-2.signature
-Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-3.signature
-Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-4.signature
-Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-5.signature
-Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391.eps'...
-Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-1.eps'...
-Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-2.eps'...
-Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-3.eps'...
-Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-4.eps'...
-Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-5.eps'...
-Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-systems.texi...
-Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-systems.tex...
-Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/4b/lily-f52c5391-systems.count...
-Writing timing to 4b/lily-f52c5391.profile...
+Writing header field `texidoc' to `/tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f.texidoc'...
+Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-1.signature
+Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-2.signature
+Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-3.signature
+Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-4.signature
+Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-5.signature
+Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f.eps'...
+Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-1.eps'...
+Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-2.eps'...
+Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-3.eps'...
+Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-4.eps'...
+Layout output to `/tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-5.eps'...
+Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-systems.texi...
+Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-systems.tex...
+Writing /tmp/build-lilypond-autobuild/out/lybook-testdb/50/lily-c00b6d5f-systems.count...
+Writing timing to 50/lily-c00b6d5f.profile...
--snip--
Now it may have happened also in the previous test - and I didn't notice. I can't be sure.
James
Labels: -Patch-new Patch-review
Originally posted by: dak@gnu.org
Looks like we overlooked it when the issue was originally committed:
% DynamicLineSpanner
c1-\offset #'padding #1 \f
This will try offsetting a DynamicText item first, and DynamicText does not have a #'padding property.
Originally posted by: david.na...@gmail.com
Improvements to \offset
Property can be specified with or without #' prefix.
Directed tweaks are possible.
Regtest:
--remove #' prefix before property names
--remove unnecessary # before numbers
--add example showing directed tweaks
Revise docstring of find-value-to-offset in `scm/music-functions.scm.'
http://codereview.appspot.com/15850044
Labels: -Patch-review Patch-new
Originally posted by: david.na...@gmail.com
OK, new patch set uploaded correcting the problem.
Sorry I didn't catch that in the first place!
Originally posted by: pkx1...@gmail.com
Still getting the same reg test information:
input/regression/offsets.log
--snip--
Parsing...
Renaming input to: `/tmp/lilypond-autobuild/input/regression/offsets.ly'
-Interpreting music...
-programming error: Grob `DynamicText' has no interface for property `padding'
-continuing, cross fingers
-[8][16][16]
+Interpreting music...[8][16]
Preprocessing graphical objects...
... etc
--snip--
Labels: -Patch-new Patch-needs_work
Originally posted by: dak@gnu.org
Uhm, no? In contrast to the previous (committed and wrong) state, the lines
Interpreting music...
programming error: Grob `DynamicText' has no interface for property `padding'
continuing, cross fingers
[8][16][16]
have disappeared. Instead there is now only
Interpreting music...[8][16]
Which is quite as it should be. Marking as review for that reason. If you have other reasons to mark this as Needs_work, feel free to do so.
Labels: -Patch-needs_work Patch-review
Status: Started
Originally posted by: dak@gnu.org
While we are at it, actually: you create a function creator called "offsetter".
Maybe one should have a general function creater called "grob:post-process" which is used as
\override NoteHead.X-offset = #(grob:post-process 'X-offset
(lambda (grob before-value) ...)
This would be a good starting position for doing further improvements, since it means that the algorithm for doing the offsetting itself can be improved in the lambda function for offsetter specifically, while grob:post-process can focus on supporting unpure-pure containers and such stuff if that's feasible (possibly a hen-and-egg problem).
Originally posted by: david.na...@gmail.com
Certainly worth looking into. I won't be able to look into this right away
(as my break from teaching is rapidly coming to an end). In the meantime,
I don't feel good about leaving the patch as-is in master, since it
requires the #'. I don't particularly want to document the function and
put up a changes entry with that older notation in place. Shall I open an
issue for this?
Originally posted by: david.na...@gmail.com
I think that was a little unclear. I simply mean that I'd like to go ahead
with this patch, and then tackle your enhancement suggestion later.
Originally posted by: david.na...@gmail.com
Ah, OK--it's no longer "needs work". Misled myself with the labels under comment #10.
Originally posted by: pkx1...@gmail.com
Patch on countdown for October 31st - 06:00 GMT
Labels: -Patch-review Patch-countdown
Originally posted by: pkx1...@gmail.com
Patch counted down - please push
Labels: -Patch-countdown Patch-push
Originally posted by: david.na...@gmail.com
Pushed as [rbf168d9084446839ce050db03d1b828291e88e0c].
Labels: -Patch-push Fixed_2_17_30
Status: Fixed
Originally posted by: dak@gnu.org
Stupid question: how does the following function play into this:
-- Function: ly:grob-chain-callback grob proc sym
Find the callback that is stored as property SYM of grob GROB and
chain PROC to the head of this, meaning that it is called using
GROB and the previous callback’s result.
That very much sounds relevant, doesn't it? Like it is supposed to do most of the job?
Originally posted by: dak@gnu.org
To answer my own question: ly:grob-chain-callback works on a live grob rather than on the grob property templates stored as context properties.
So to use ly:grob-chain-callback, one would need to have a grob acknowledger listen and apply operations to live grobs like the Tweak_engraver does. And/or like \overrideProperty maybe.
Originally posted by: david.na...@gmail.com
OK--so there would still be a need for the grob:post-process function you
suggest in #11.
Originally posted by: Elu...@gmail.com
(No comment was entered for this change.)
Status: Verified