<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
So I'm puzzled how often we are adding bookmark events and how that<br>
affects performance... or is this really just a first of a series and the<br>
others are more performance relevant?<br></blockquote><div><br></div><div>First of a series.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> diff --git a/dive.c b/dive.c<br>
> index b318c4b..4fb6717 100644<br>
> --- a/dive.c<br>
> +++ b/dive.c<br>
> @@ -150,6 +150,18 @@ void update_event_name(struct dive *d, struct event *event, char *name)<br>
> free(remove);<br>
> }<br>
><br>
> +struct event *get_event_by_time(struct dive *d, uint32_t time){<br>
> + if (!d)<br>
> + return NULL;<br>
> + struct divecomputer *dc = get_dive_dc(d, dc_number);<br>
> + if (!dc)<br>
> + return NULL;<br>
> + struct event **events = &dc->events;<br>
> + while((*events)->next && (*events)->time.seconds != time)<br>
> + events = &(*events)->next;<br>
> + return (*events);<br>
> +}<br>
<br>
Dear C expert... I am curious. Why are you doing this with a struct event **<br>
instead of just a struct event *... you need the ** if you want to be able<br>
to modify the list in an elegant way, but all you do here is walk the<br>
list, so this could drop one level of indirection... or am I missing<br>
something?<br></blockquote><div><br></div><div>Copied from the code just above that one.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> diff --git a/qt-ui/profile/diveeventitem.cpp b/qt-ui/profile/diveeventitem.cpp<br>
> index a9c3c3f..14cb0a3 100644<br>
> --- a/qt-ui/profile/diveeventitem.cpp<br>
> +++ b/qt-ui/profile/diveeventitem.cpp<br>
> @@ -9,6 +9,7 @@<br>
> #include <QDebug><br>
> #include "gettextfromc.h"<br>
> #include "metrics.h"<br>
> +#include <profile.h><br>
><br>
> extern struct ev_select *ev_namelist;<br>
> extern int evn_used;<br>
> @@ -152,18 +153,27 @@ bool DiveEventItem::shouldBeHidden()<br>
><br>
> void DiveEventItem::recalculatePos(bool instant)<br>
> {<br>
> - if (!vAxis || !hAxis || !internalEvent || !dataModel)<br>
> + if (!vAxis || !hAxis || !internalEvent || !dataModel){<br>
<br>
WHITESPACE<br></blockquote><div><br></div><div>MEEH<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> return;<br>
> + }<br>
<br>
And why add the curly braces, anyway?<br></blockquote><div><br></div><div>I was debugging and forgot to remove them.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> - QModelIndexList result = dataModel->match(dataModel->index(0, DivePlotDataModel::TIME), Qt::DisplayRole, internalEvent->time.seconds);<br>
> - if (result.isEmpty()) {<br>
> - Q_ASSERT("can't find a spot in the dataModel");<br>
> - hide();<br>
> - return;<br>
> + // find the correct time or interpolate between two points.<br>
> + int depth = -1;<br>
> + plot_data *entry = dataModel->data().entry;<br>
> + for(int i = 0; i < dataModel->data().nr; i++){<br>
> + if (entry[i].sec == internalEvent->time.seconds){<br>
> + depth = entry[i].depth;<br>
> + break;<br>
> + } else if (entry[i].sec > internalEvent->time.seconds) {<br>
> + int min = entry[i-1].depth;<br>
<br>
This could do something really bad if for some reason<br>
internalEvent->time.seconds is negative, right? then entry[0].sec (which<br>
should always be 0) could already be bigger and we access element entry[-1]<br></blockquote><div><br></div><div>Is that possible?<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> + int max = entry[i].depth;<br>
> + depth = min + ((max - min)/2);<br>
> + break;<br>
> + }<br>
> }<br>
> +<br>
> if (!isVisible() && !shouldBeHidden())<br>
> show();<br>
> - int depth = dataModel->data(dataModel->index(result.first().row(), DivePlotDataModel::DEPTH)).toInt();<br>
> qreal x = hAxis->posAtValue(internalEvent->time.seconds);<br>
> qreal y = vAxis->posAtValue(depth);<br>
> if (!instant)<br>
> @@ -172,4 +182,5 @@ void DiveEventItem::recalculatePos(bool instant)<br>
> setPos(x, y);<br>
> if (isVisible() && shouldBeHidden())<br>
> hide();<br>
> + qDebug() << pos();<br>
<br>
Grrrrrrrmbl<br></blockquote><div><br></div><div>MEEEH<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> diff --git a/qt-ui/profile/profilewidget2.cpp b/qt-ui/profile/profilewidget2.cpp<br>
> index 1970561..1181fc5 100644<br>
> --- a/qt-ui/profile/profilewidget2.cpp<br>
> +++ b/qt-ui/profile/profilewidget2.cpp<br>
> @@ -1277,8 +1277,11 @@ void ProfileWidget2::removeEvent()<br>
> tr("%1 @ %2:%3").arg(event->name).arg(event->time.seconds / 60).arg(event->time.seconds % 60, 2, 10, QChar('0'))),<br>
> QMessageBox::Ok | QMessageBox::Cancel) == QMessageBox::Ok) {<br>
> remove_event(event);<br>
> + copy_events(¤t_dive->dc, &displayed_dive.dc);<br>
<br>
this leaks memory, the events in displayed_dive are overwritten without<br>
being freed. Call free_events() on the existing events in displayed_dive<br></blockquote><div><br></div><div>Oh, sorry.<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> mark_divelist_changed(true);<br>
> - replot();<br>
> + scene()->removeItem(item);<br>
> + eventItems.removeOne(item);<br>
> + item->deleteLater();<br>
<br>
You are the expert on that code :-)<br>
<br>
> @@ -1287,8 +1290,20 @@ void ProfileWidget2::addBookmark()<br>
> QAction *action = qobject_cast<QAction *>(sender());<br>
> QPointF scenePos = mapToScene(mapFromGlobal(action->data().toPoint()));<br>
> add_event(current_dc, timeAxis->valueAt(scenePos), SAMPLE_EVENT_BOOKMARK, 0, 0, "bookmark");<br>
> + copy_events(¤t_dive->dc, &displayed_dive.dc);<br>
<br>
See above - leaks memory<br></blockquote><div><br></div><div>Oh sorry2<br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> + struct event *ev = get_event_by_time(&displayed_dive, timeAxis->valueAt(scenePos));<br>
> + qDebug() << "time = " << timeAxis->valueAt(scenePos) << "Event time: " << ev->time.seconds;<br>
<br>
Grrrrrmbl<br>
<br>
> mark_divelist_changed(true);<br>
> - replot();<br>
> + DiveEventItem *item = new DiveEventItem();<br>
> + item->setHorizontalAxis(timeAxis);<br>
> + item->setVerticalAxis(profileYAxis);<br>
> + item->setModel(dataModel);<br>
> + item->setEvent(ev);<br>
> + item->setZValue(2);<br>
> + item->recalculatePos(true);<br>
> + item->setVisible(true);<br>
> + eventItems.push_back(item);<br>
> + scene()->addItem(item);<br>
> }<br>
><br>
> void ProfileWidget2::addSetpointChange()<br>
> @@ -1377,10 +1392,12 @@ void ProfileWidget2::editName()<br>
> // order is important! first update the current dive (by matching the unchanged event),<br>
> // then update the displayed dive (as event is part of the events on displayed dive<br>
> // and will be freed as part of changing the name!<br>
> + int time = event->time.seconds;<br>
> update_event_name(current_dive, event, newName.toUtf8().data());<br>
> update_event_name(&displayed_dive, event, newName.toUtf8().data());<br>
> + struct event *ev = get_event_by_time(&displayed_dive, time);<br>
> mark_divelist_changed(true);<br>
> - replot();<br>
> + item->setEvent(ev);<br>
> }<br>
> }<br>
<br>
<br>
So this is pretty invasive and the patch has a few issues.<br>
<br>
Why should this go in before 4.3? Is there a specific bug this addresses?<br></blockquote><div><br></div><div>I was trying to find out why when moving a node for a few seconds while adding a new dive used 100% of the CPU in release mode. turned out it was because of too many replots that used the *very* expensive calls to PainterPath.<br></div><div>while I was trying to fix that I got carried away. I think I'll just generate the pixmap of the Glyphs and reuse them in the future for the PainterPath issue.<br><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Just trying to understand the what and the why... if this should go in,<br>
can I get a cleaned up patch (I have taken the other one).<br>
<span class="HOEnZb"><font color="#888888"><br>
/D<br>
</font></span></blockquote></div></div></div>