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

Issue 6573066: Caches grob properties before evaluation.

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 7 months ago by MikeSol
Modified:
8 years, 6 months ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Base URL:
http://git.savannah.gnu.org/gitweb/?p=lilypond.git/trunk/
Visibility:
Public.

Description

Caches grob properties before evaluation. We cannot simply use the original value stored in the immutable_properties_alist_ because simple closures may be created at the engraver stage via chain_offset_callback and related functions. Instead, we grab the property data at the moment of evaluation and cache it.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -0 lines) Patch
A input/regression/restore-property-to-original-value.ly View 1 chunk +24 lines, -0 lines 0 comments Download
M lily/grob.cc View 4 chunks +4 lines, -0 lines 0 comments Download
M lily/grob-property.cc View 2 chunks +24 lines, -0 lines 0 comments Download
M lily/grob-scheme.cc View 1 chunk +14 lines, -0 lines 0 comments Download
M lily/grob-smob.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/include/grob.hh View 2 chunks +2 lines, -0 lines 0 comments Download
M lily/include/lily-guile-macros.hh View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 2
MikeSol
This slows down LilyPond - I haven't done comprehensive tests of how much. I'm pretty ...
11 years, 7 months ago (2012-09-28 07:38:05 UTC) #1
dak
8 years, 6 months ago (2015-10-02 14:06:18 UTC) #2
On 2012/09/28 07:38:05, MikeSol wrote:
> This slows down LilyPond - I haven't done comprehensive tests of how much. 
I'm
> pretty sure it works (the regtest works as expected).  Irrespective of how
> multiple passes are done, this seems like a necessary first step.
> 
> Note that this patch would not have a time impact if it used properties
stashed
> in immutable_properties_alist_. If all side effects were eliminated from
> LilyPond (which would be awesome), then this could be done.  However, this
would
> be very difficult - for example, tweaks are given to grobs after their
> immutable_properties_alist_ is fixed (see the tweak engraver).

I'm not really able to figure out where this patch was supposed to help.  But
for whatever it's worth, ly:make-simple-closure and its C++ equivalents are
gone.  chain_offset_callback and its ilk are still around but work via
unpure-pure-containers or callbacks now.  They still swap out values at the same
time as previously.

There also is now an \offset command using a grob-transformer function at its
heart: those have a similar effect but work exclusively in the immutable
property chain.
Sign in to reply to this message.

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