[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Implement Grob::event_cause, Grob::ultimate_event_cause (issue 55950
From: |
dak |
Subject: |
Re: Implement Grob::event_cause, Grob::ultimate_event_cause (issue 559500043 by address@hidden) |
Date: |
Thu, 20 Feb 2020 16:32:06 -0800 |
Reviewers: hahnjo, hanwenn, Dan Eble,
https://codereview.appspot.com/559500043/diff/559510043/lily/grob.cc
File lily/grob.cc (right):
https://codereview.appspot.com/559500043/diff/559510043/lily/grob.cc#newcode733
lily/grob.cc:733: while (unsmob<Grob> (cause))
On 2020/02/21 00:13:25, Dan Eble wrote:
> I appreciate that issuing warnings is not performance-sensitive, and
that you
> simply transplanted this code, but what do you think about avoiding
repeating
> the unsmob, to set a good example? Something like ...
>
> while (const Grob *g = unsmob<Grob> (cause))
> {
> cause = g->get_property ("cause");
> }
Not going to use const here since it is pointless while we have nothing
like const_unsmob, and would not use that order anyway since I consider
it misleading (it suggests a declaration of a constant, rather than a
pointer to a constant). Other than that, fine.
Description:
Implement Grob::event_cause, Grob::ultimate_event_cause
Those were implemented in Grob_info only previously which does not make
a lot of sense,
given that they don't access anything particular to Grob_info.
Also contains commit:
Use Grob::event_cause and Grob::ultimate_event_cause where useful
Frankly, this one has been bugging me on and off for years. I just
had one compilation error too much today.
Please review this at https://codereview.appspot.com/559500043/
Affected files (+42, -37 lines):
M lily/accidental-engraver.cc
M lily/accidental-placement.cc
M lily/grob.cc
M lily/grob-info.cc
M lily/include/grob.hh
M lily/lyric-engraver.cc
M lily/percent-repeat-item.cc
M lily/slur-engraver.cc
M lily/tie-engraver.cc
Index: lily/accidental-engraver.cc
diff --git a/lily/accidental-engraver.cc b/lily/accidental-engraver.cc
index
f15981ebb4f3d79f8f16ca073ef1c51935303413..877ec1cc5ba0c21d96fb808ab20d11d19741e06c
100644
--- a/lily/accidental-engraver.cc
+++ b/lily/accidental-engraver.cc
@@ -374,8 +374,8 @@ Accidental_engraver::stop_translation_timestep ()
{
// Don't mark accidentals as "tied" when the pitch is not
// actually the same. This is relevant for enharmonic ties.
- Stream_event *le = unsmob<Stream_event> (l->get_property ("cause"));
- Stream_event *re = unsmob<Stream_event> (r->get_property ("cause"));
+ Stream_event *le = l->event_cause ();
+ Stream_event *re = r->event_cause ();
if (le && re
&& !ly_is_equal (le->get_property ("pitch"), re->get_property
("pitch")))
continue;
Index: lily/accidental-placement.cc
diff --git a/lily/accidental-placement.cc b/lily/accidental-placement.cc
index
7965fcb4b9613399eca830f44a9087aa5709f1ee..e34bfb85293b1c55712d8fd3ac175d2169c2603c
100644
--- a/lily/accidental-placement.cc
+++ b/lily/accidental-placement.cc
@@ -39,9 +39,8 @@ using std::vector;
static Pitch *
accidental_pitch (Grob *acc)
{
- SCM cause = acc->get_parent (Y_AXIS)->get_property ("cause");
+ Stream_event *mcause = acc->get_parent (Y_AXIS)->event_cause ();
- Stream_event *mcause = unsmob<Stream_event> (cause);
if (!mcause)
{
programming_error ("note head has no event cause");
Index: lily/grob-info.cc
diff --git a/lily/grob-info.cc b/lily/grob-info.cc
index
70e2ef55b4b966019f8b35e9f437d6637eea6ef0..9b1c265db31ceba86eb00caef323dda4b75319ff
100644
--- a/lily/grob-info.cc
+++ b/lily/grob-info.cc
@@ -46,8 +46,7 @@ Grob_info::Grob_info ()
Stream_event *
Grob_info::event_cause () const
{
- SCM cause = grob_->get_property ("cause");
- return unsmob<Stream_event> (cause);
+ return grob_->event_cause ();
}
Context *
@@ -59,12 +58,7 @@ Grob_info::context () const
Stream_event *
Grob_info::ultimate_event_cause () const
{
- SCM cause = grob_->self_scm ();
- while (unsmob<Grob> (cause))
- {
- cause = unsmob<Grob> (cause)->get_property ("cause");
- }
- return unsmob<Stream_event> (cause);
+ return grob_->ultimate_event_cause ();
}
bool
Index: lily/grob.cc
diff --git a/lily/grob.cc b/lily/grob.cc
index
a17869b00adf97f330d16ee23c887132899c780c..c4646b96b59d7bd6bb277e20438d19eef14bb126
100644
--- a/lily/grob.cc
+++ b/lily/grob.cc
@@ -716,20 +716,36 @@ Grob::internal_vertical_less (Grob *g1, Grob *g2, bool
pure)
return false;
}
+/****************************************************************
+ CAUSES
+****************************************************************/
+Stream_event *
+Grob::event_cause () const
+{
+ SCM cause = get_property ("cause");
+ return unsmob<Stream_event> (cause);
+}
+
+Stream_event *
+Grob::ultimate_event_cause () const
+{
+ SCM cause = self_scm ();
+ while (unsmob<Grob> (cause))
+ {
+ cause = unsmob<Grob> (cause)->get_property ("cause");
+ }
+ return unsmob<Stream_event> (cause);
+}
+
+
+
/****************************************************************
MESSAGES
****************************************************************/
void
Grob::programming_error (const string &s) const
{
- SCM cause = self_scm ();
- while (Grob *g = unsmob<Grob> (cause))
- cause = g->get_property ("cause");
-
- /* ES TODO: cause can't be Music*/
- if (Music *m = unsmob<Music> (cause))
- m->origin ()->programming_error (s);
- else if (Stream_event *ev = unsmob<Stream_event> (cause))
+ if (Stream_event *ev = ultimate_event_cause ())
ev->origin ()->programming_error (s);
else
::programming_error (s);
@@ -738,14 +754,7 @@ Grob::programming_error (const string &s) const
void
Grob::warning (const string &s) const
{
- SCM cause = self_scm ();
- while (Grob *g = unsmob<Grob> (cause))
- cause = g->get_property ("cause");
-
- /* ES TODO: cause can't be Music*/
- if (Music *m = unsmob<Music> (cause))
- m->origin ()->warning (s);
- else if (Stream_event *ev = unsmob<Stream_event> (cause))
+ if (Stream_event *ev = ultimate_event_cause ())
ev->origin ()->warning (s);
else
::warning (s);
Index: lily/include/grob.hh
diff --git a/lily/include/grob.hh b/lily/include/grob.hh
index
d0195610af9f97634057333b595de0e9445783f4..d01fdf8169912cdf8eda02b21eabfbad4b6f348e
100644
--- a/lily/include/grob.hh
+++ b/lily/include/grob.hh
@@ -24,6 +24,7 @@
#include "virtual-methods.hh"
#include "dimension-cache.hh"
#include "grob-interface.hh"
+#include "lily-proto.hh"
#include <set>
@@ -117,6 +118,10 @@ public:
void instrumented_set_property (SCM, SCM, char const *, int, char const *);
void internal_set_property (SCM sym, SCM val);
+ /* causes */
+ Stream_event *event_cause () const;
+ Stream_event *ultimate_event_cause () const;
+
/* messages */
void warning (const std::string&) const;
void programming_error (const std::string&) const;
Index: lily/lyric-engraver.cc
diff --git a/lily/lyric-engraver.cc b/lily/lyric-engraver.cc
index
6cc5cff89cc2292a3d0088f109f352691d285bfc..1de70bf1ba6cbebc7b5359ea67b5e51231153d3c
100644
--- a/lily/lyric-engraver.cc
+++ b/lily/lyric-engraver.cc
@@ -146,7 +146,7 @@ get_current_note_head (Context *voice)
// It's a bit irritating that we just have the length and
// duration of the Grob.
Moment end_from_now =
- get_event_length (unsmob<Stream_event> (g->get_property ("cause")),
now)
+ get_event_length (g->event_cause (), now)
+ now;
// We cannot actually include more than a single grace note
// using busyGrobs on ungraced lyrics since a grob ending on
Index: lily/percent-repeat-item.cc
diff --git a/lily/percent-repeat-item.cc b/lily/percent-repeat-item.cc
index
177266693f6a8ecc2c9b9c4c6c4c1527b0478810..776a8b62a000d2dee3c3f71f35ef2867db1f2620
100644
--- a/lily/percent-repeat-item.cc
+++ b/lily/percent-repeat-item.cc
@@ -80,7 +80,7 @@ SCM
Percent_repeat_item_interface::beat_slash (SCM grob)
{
Grob *me = unsmob<Grob> (grob);
- Stream_event *cause = unsmob<Stream_event> (me->get_property ("cause"));
+ Stream_event *cause = me->event_cause ();
int count = robust_scm2int (cause->get_property ("slash-count"), 1);
Stencil m;
Index: lily/slur-engraver.cc
diff --git a/lily/slur-engraver.cc b/lily/slur-engraver.cc
index
8498ebf2abc2002613f4e9b09c745b6795b7b70d..731abc69f8dfd88c398ce516eb2ce6f21e0a9d3a
100644
--- a/lily/slur-engraver.cc
+++ b/lily/slur-engraver.cc
@@ -151,8 +151,7 @@ Slur_engraver::acknowledge_note_column (Grob_info info)
extract_grob_set (e, "note-heads", heads);
for (vsize i = heads.size (); i--;)
{
- if (Stream_event *ev =
- unsmob<Stream_event> (heads[i]->get_property ("cause")))
+ if (Stream_event *ev = heads[i]->event_cause ())
for (LEFT_and_RIGHT (d))
{
std::pair<Note_slurs::const_iterator, Note_slurs::const_iterator>
its
@@ -249,7 +248,7 @@ Slur_engraver::can_create_slur (SCM id, vsize old_slurs,
vsize *event_idx, Strea
if (!updown)
return false;
- Stream_event *c = unsmob<Stream_event> (slur->get_property
("cause"));
+ Stream_event *c = slur->event_cause ();
if (!c)
{
Index: lily/tie-engraver.cc
diff --git a/lily/tie-engraver.cc b/lily/tie-engraver.cc
index
f31eb177f3e98115fd7811ee91a4d606049ac71e..f05b866e4c6ba3960892683afaff8254db156197
100644
--- a/lily/tie-engraver.cc
+++ b/lily/tie-engraver.cc
@@ -157,8 +157,8 @@ Tie_engraver::tie_notehead (Grob *h, bool enharmonic)
for (vsize i = 0; i < heads_to_tie_.size (); i++)
{
Grob *th = heads_to_tie_[i].head_;
- Stream_event *right_ev = unsmob<Stream_event> (h->get_property
("cause"));
- Stream_event *left_ev = unsmob<Stream_event> (th->get_property
("cause"));
+ Stream_event *right_ev = h->event_cause ();
+ Stream_event *left_ev = th->event_cause ();
/*
maybe should check positions too.
@@ -282,8 +282,7 @@ Tie_engraver::process_acknowledged ()
for (vsize i = 0; i < now_heads_.size (); i++)
{
Grob *head = now_heads_[i];
- Stream_event *left_ev
- = unsmob<Stream_event> (head->get_property ("cause"));
+ Stream_event *left_ev = head->event_cause ();
if (!left_ev)
{