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

Issue 348120043: Allow scripts to be defined either as glyphs or stencils. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by Valentin Villenave
Modified:
5 years ago
Reviewers:
dak, rap.ardiputra127
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Allow scripts to be defined either as glyphs or stencils. This feature has been discussed several times over the years; this patch aims to lift the limitation that Script grobs are only to be defined as Feta glyphs rather than glyphs from any font (assuming their naming closely follows that of Feta), or any Stencil definition. (See regtest for a demonstration; proper doc to follow if this feature gets accepted.)

Patch Set 1 #

Total comments: 22

Patch Set 2 : Simplify. (but still use pair-or-stencil? for now) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+85 lines, -13 lines) Patch
A input/regression/script-stencil.ly View 1 1 chunk +56 lines, -0 lines 0 comments Download
M lily/script-interface.cc View 1 1 chunk +22 lines, -11 lines 0 comments Download
M scm/c++.scm View 1 chunk +3 lines, -0 lines 0 comments Download
M scm/define-grob-properties.scm View 1 chunk +3 lines, -2 lines 0 comments Download
M scm/lily.scm View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 11
dak
https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly File input/regression/script-stencil.ly (right): https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly#newcode31 input/regression/script-stencil.ly:31: #(append! default-script-alist No. Don't change any system data structures ...
5 years, 1 month ago (2019-02-22 16:19:53 UTC) #1
Valentin Villenave
Thanks David! You make valid points, I just have a couple of doubts below. https://codereview.appspot.com/348120043/diff/1/input/regression/script-stencil.ly ...
5 years, 1 month ago (2019-02-22 20:41:08 UTC) #2
dak
https://codereview.appspot.com/348120043/diff/1/lily/script-interface.cc File lily/script-interface.cc (right): https://codereview.appspot.com/348120043/diff/1/lily/script-interface.cc#newcode36 lily/script-interface.cc:36: Script_interface::get_glyph_or_stencil (Grob *me, Direction d) On 2019/02/22 20:41:08, Valentin ...
5 years, 1 month ago (2019-02-22 21:00:08 UTC) #3
Valentin Villenave
Simplify. (but still use pair-or-stencil? for now)
5 years, 1 month ago (2019-02-22 21:38:51 UTC) #4
Valentin Villenave
On 2019/02/22 21:00:08, dak wrote: > You are confusing stencils with stencil expressions. Stencils satisfy ...
5 years, 1 month ago (2019-02-22 21:51:39 UTC) #5
dak
On 2019/02/22 21:51:39, Valentin Villenave wrote: > On 2019/02/22 21:00:08, dak wrote: > > You ...
5 years, 1 month ago (2019-02-22 22:11:58 UTC) #6
Rap.ardiputra127
5 years, 1 month ago (2019-02-22 22:26:17 UTC) #7
Valentin Villenave
On 2019/02/22 22:11:58, dak wrote: > There is some fundamental confusion here. A stencil expression ...
5 years, 1 month ago (2019-02-23 21:51:33 UTC) #8
dak
v.villenave@gmail.com writes: > On 2019/02/22 22:11:58, dak wrote: >> There is some fundamental confusion here. ...
5 years, 1 month ago (2019-02-23 22:40:44 UTC) #9
Valentin Villenave
On 2019/02/23 22:40:44, dak wrote: > Sure, but you are trying to override stencils, aren't ...
5 years, 1 month ago (2019-02-24 08:48:22 UTC) #10
dak
5 years, 1 month ago (2019-02-24 14:01:13 UTC) #11
On 2019/02/24 08:48:22, Valentin Villenave wrote:

> I’m still bothered by the way script-interface.cc is written, specifically the
> hardcoded "feta" reference which I tried to address; do you have any thoughts
on
> that? (Other than that, it appears there’s very little to salvage from my
patch
> :-)

'feta here is nothing other than a particular character lookup scheme to be used
on the other details of the entry.  If you have suggestions for other, possibly
more general useful lookup schemes, they can be added into the get_stencil
procedure.  It would even be possible to use a 'stencil lookup scheme that would
buy nothing over just using the stencil callback other than selection of
different stencils for up/down.  Since you could do that kind of decision
equally well in a stencil callback, that would be quite redundant, however.

The ability to just override the stencil callback renders every such addition
redundant but since the character lookup scheme flag is there, one might as well
use it to allow more than a single choice.
Sign in to reply to this message.

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