Reviewers: Dan Eble, benko.pal,
Message:
On 2018/07/10 17:10:53, benko.pal wrote:
LGTM; just by looking I can't see how it can make make fail.
using rp[-2] and rp[-1] instead of lastcol and c would be cleaner to
me, but
YMMV.
Since the whole point is to do the operation in-place, rp[-2] may
already have been overwritten by rp[-1] in the last iteration. It may
seem cleaner to you to use rp[-2] but it would be a rather ugly bug.
Doing things in-place is more efficient but you have to keep track of
what you are doing.
I consider it possible that mixing a const iterator with a non-const one
might be what caused the compilation error but I'd want to see the error
message to be sure.
Description:
Spacing_spanner::prune_loose_columns: prune in-place
This saves a tiny amount of time and memory.
Please review this at https://codereview.appspot.com/351720043/
Affected files (+14, -9 lines):
M lily/spacing-determine-loose-columns.cc
Index: lily/spacing-determine-loose-columns.cc
diff --git a/lily/spacing-determine-loose-columns.cc
b/lily/spacing-determine-loose-columns.cc
index
4be23042ad44bbddc007c8879714cd0c201bd35d..5b5e994108372844b6cabfcc0b3f5885857e3b58
100644
--- a/lily/spacing-determine-loose-columns.cc
+++ b/lily/spacing-determine-loose-columns.cc
@@ -189,13 +189,16 @@ Spacing_spanner::prune_loose_columns (Grob *me,
vector<Grob *> *cols,
Spacing_options *options)
{
- vector<Grob *> newcols;
- for (vsize i = 0; i < cols->size (); i++)
+ // In-place iteration will read at rp and conditionally write at wp.
+ vector<Grob *>::const_iterator rp;
+ vector<Grob *>::iterator wp;
+ Grob * lastcol = 0;
+ for (rp = wp = cols->begin (); rp != cols->end ();)
{
- Grob *c = cols->at (i);
+ Grob *c = *rp++;
- bool loose = (i > 0 && i + 1 < cols->size ())
- && is_loose_column (cols->at (i - 1), c, cols->at (i +
1), options);
+ bool loose = (lastcol && rp != cols->end ()
+ && is_loose_column (lastcol, c, *rp, options));
/* Breakable columns never get pruned; even if they are loose,
their broken pieces are not. However, we mark them so that
@@ -230,8 +233,8 @@ Spacing_spanner::prune_loose_columns (Grob *me,
if (!right_neighbor || !left_neighbor)
{
c->programming_error ("Cannot determine neighbors for
floating column.");
- c->set_object ("between-cols", scm_cons (cols->at (i -
1)->self_scm (),
- cols->at (i +
1)->self_scm ()));
+ c->set_object ("between-cols", scm_cons (lastcol->self_scm
(),
+ (*rp)->self_scm
()));
}
else
{
@@ -249,10 +252,12 @@ Spacing_spanner::prune_loose_columns (Grob *me,
}
else
- newcols.push_back (c);
+ *wp++ = c;
+
+ lastcol = c;
}
- *cols = newcols;
+ cols->erase (wp, rp);
}
/*
_______________________________________________
lilypond-devel mailing list
lilypond-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/lilypond-devel
