Re: [PATCH 4/7] Fix divecomputer part of UDDF export
On Sat, Dec 13, 2014 at 2:34 AM, Martin Long wrote: > XPath was incorrect for parsing divecomputers into the equipment > section, meaning they dont get inserted. Can you elaborate on this one as on my divelog all the dive computers are listed on the settings section and the original code worked with that properly. The sample dives are incorrect in this regard, but when you open them in Subsurface and save to new file, they get the stored in the settings section (and this is the state that is used when exporting from Subsurface). Dirk, shouldn't all the DCs be always listed in the settings section so that we don't need to parse the whole file to get a list of them? Of course, your way of grabbing the divecomputers from all the dives works as well, but it is faster to just grab them from the settings section (unless I am really missing something and not all DCs are available from there). miika ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: UDDF export - nearly there.
> On Dec 12, 2014, at 4:43 PM, Long, Martin wrote: > > So another 7 patches. This makes it almost 100% UDDF 3.2 compliant. > > I say "almost". There are a couple of places where the schema doesn't > agree with the documentation, where I believe the schema. I believe > there are also some bugs in the schema too. In both cases I'll raise > these with the authors. > > Also, XSD schema are pretty horrible in that certain circumstances > force you to use "sequence" validation, where you actually don't > really care about the ordering. This means we've had to comply with > ordering in certain places. I hate XML. Welcome to the club :-) > The only outstanding issue is the alarm types. It looks like we need > to map these to equivalent types in UDDF. It's 12:30am, and I'm tired, > so that may come tomorrow. Thank you so much for the hard work. This is truly appreciated. I won’t cut 4.3 until next week, so this will work out. > Finally, there are a couple of commits which group several minor > changes. Sorry for this. The alternative would have been a patch set > of about 20 one-line patches. Which would have been perfectly fine maybe even preferred, but what you sent looks good. /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
UDDF export - nearly there.
So another 7 patches. This makes it almost 100% UDDF 3.2 compliant. I say "almost". There are a couple of places where the schema doesn't agree with the documentation, where I believe the schema. I believe there are also some bugs in the schema too. In both cases I'll raise these with the authors. Also, XSD schema are pretty horrible in that certain circumstances force you to use "sequence" validation, where you actually don't really care about the ordering. This means we've had to comply with ordering in certain places. I hate XML. The only outstanding issue is the alarm types. It looks like we need to map these to equivalent types in UDDF. It's 12:30am, and I'm tired, so that may come tomorrow. Finally, there are a couple of commits which group several minor changes. Sorry for this. The alternative would have been a patch set of about 20 one-line patches. ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
[PATCH 3/7] Use generated Ids for site lookup in UDDF export.
As with buddies, sites could contain characters which are not valid in Ids. Also, it is very possible to have duplicate site names. This uses an XSL generated id to prevent any issues. Signed-off-by: Martin Long --- xslt/uddf-export.xslt | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/xslt/uddf-export.xslt b/xslt/uddf-export.xslt index 7f67d81..a5e2266 100644 --- a/xslt/uddf-export.xslt +++ b/xslt/uddf-export.xslt @@ -194,12 +194,12 @@ - + - + - + @@ -253,13 +253,13 @@ - + - + - + -- 1.9.1 ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
[PATCH 5/7] Various minor fixes to UDDF export
Removed underscore from buddy name elements Added 'mix' prefix to gas ids. Ids must not start with a number. Added mandatory profile data with empty tags. Signed-off-by: Martin Long --- xslt/uddf-export.xslt | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/xslt/uddf-export.xslt b/xslt/uddf-export.xslt index 0114ca3..16e6ed3 100644 --- a/xslt/uddf-export.xslt +++ b/xslt/uddf-export.xslt @@ -49,9 +49,12 @@ - + + + + @@ -81,12 +84,12 @@ - + - - + + - + @@ -141,7 +144,7 @@ just use the same references used internally on Subsurface. --> - + @@ -205,7 +208,6 @@ - @@ -473,10 +475,10 @@ - + - + -- 1.9.1 ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
[PATCH 4/7] Fix divecomputer part of UDDF export
XPath was incorrect for parsing divecomputers into the equipment section, meaning they dont get inserted. Signed-off-by: Martin Long --- xslt/uddf-export.xslt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xslt/uddf-export.xslt b/xslt/uddf-export.xslt index a5e2266..0114ca3 100644 --- a/xslt/uddf-export.xslt +++ b/xslt/uddf-export.xslt @@ -6,6 +6,7 @@ + @@ -52,7 +53,7 @@ - + -- 1.9.1 ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
[PATCH 7/7] Fix bug in tankpressurebegin in UDDF export
There was a bug when the first sample doesn't contain pressure info. This fixes that by selecting the first with pressure info. Signed-off-by: Martin Long --- xslt/uddf-export.xslt | 29 + 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/xslt/uddf-export.xslt b/xslt/uddf-export.xslt index d5d475f..c70aadc 100644 --- a/xslt/uddf-export.xslt +++ b/xslt/uddf-export.xslt @@ -475,37 +475,34 @@ + - - - - - - - + - + + + + + - - - - - - - + - + + + + + -- 1.9.1 ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
[PATCH 1/7] Update UDDF export header to include madatory tags.
Add name at level, and add additional in contact tag. Signed-off-by: Martin Long --- xslt/uddf-export.xslt | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/xslt/uddf-export.xslt b/xslt/uddf-export.xslt index 2f7ba70..7cdb2b2 100644 --- a/xslt/uddf-export.xslt +++ b/xslt/uddf-export.xslt @@ -12,9 +12,12 @@ +Subsurface Divelog Subsurface Team - http://subsurface-divelog.org/ + +http://subsurface-divelog.org/ + @@ -36,7 +39,7 @@ - + -- 1.9.1 ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
[PATCH 2/7] Use generated ids for buddies in UDDF export.
This is instead of using their names, which may contain illegal characters. Signed-off-by: Martin Long --- xslt/uddf-export.xslt | 45 - 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/xslt/uddf-export.xslt b/xslt/uddf-export.xslt index 7cdb2b2..7f67d81 100644 --- a/xslt/uddf-export.xslt +++ b/xslt/uddf-export.xslt @@ -1,4 +1,4 @@ -http://www.w3.org/1999/XSL/Transform"; xmlns:xt="http://www.jclark.com/xt"; +http://www.w3.org/1999/XSL/Transform"; xmlns:xt="http://www.jclark.com/xt"; extension-element-prefixes="xt" version="1.0"> @@ -7,6 +7,17 @@ + + + + + + + + + + + @@ -60,19 +71,11 @@ - - - - - - - - - + - + @@ -93,7 +96,7 @@ - + @@ -235,20 +238,20 @@ - - - - - - + + + + - - + + + - + + -- 1.9.1 ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: commit c76cb59, [PATCH prevent tank bar overlap]
On Fri, Dec 12, 2014 at 08:47:26PM +0100, Joakim Bygdell wrote: > > > Dirk test this patch and see it if looks ok on your dives. It does. Thanks. Good work /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: two patches
On Fri, Dec 12, 2014 at 5:38 PM, Dirk Hohndel wrote: > > On Fri, Dec 12, 2014 at 05:03:54PM -0200, Tomaz Canabrava wrote: > > > So I'm puzzled how often we are adding bookmark events and how that > > > affects performance... or is this really just a first of a series and > the > > > others are more performance relevant? > > > > First of a series. > > OK > > > > Dear C expert... I am curious. Why are you doing this with a struct > event > > > ** > > > instead of just a struct event *... you need the ** if you want to be > able > > > to modify the list in an elegant way, but all you do here is walk the > > > list, so this could drop one level of indirection... or am I missing > > > something? > > > > Copied from the code just above that one. > > But that code modified the list, right? > > > > And why add the curly braces, anyway? > > > > I was debugging and forgot to remove them. > > Tsktsktsk. So you added something because you forgot to remove it and > introduced whitespace damage that way... > > > > This could do something really bad if for some reason > > > internalEvent->time.seconds is negative, right? then entry[0].sec > (which > > > should always be 0) could already be bigger and we access element > entry[-1] > > > > Is that possible? > > Off the top of my head I'd say "unlikely", but I worry about dives > imported from other sources or something else that could break this > assumption. So I'd rather have you test for i > 0 before you access > entry[i - 1] > > > > So this is pretty invasive and the patch has a few issues. > > > > > > Why should this go in before 4.3? Is there a specific bug this > addresses? > > > > > > > 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. > > while I was trying to fix that I got carried away. > > I hear that happens. So does this mean "I'll rework this for until after > 4.3" or "I'll rework this to address the issues mentioned and then submit > the series that fixes the bigger problem later today"? > After 4.3 I'll touch this again, you can forget this one. :) > > > I think I'll just > > generate the pixmap of the Glyphs and reuse them in the future for the > > PainterPath issue. > > Or that and we forget the patch? That seems wrong because the idea in the > patch seems correct. > > /D > ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: commit c76cb59, [PATCH prevent tank bar overlap]
> On 12 Dec 2014, at 20:08, Joakim Bygdell wrote: > > >> On 12 Dec 2014, at 20:00, Dirk Hohndel wrote: >> >> On Fri, Dec 12, 2014 at 07:46:29PM +0100, Joakim Bygdell wrote: >>> Dirk, I suggest that you reverse commit c76cb59. >>> After your patch the line of the temperature graph sometimes gets drawn >>> through the depth notation of the deepest point of the profile thereby >>> obscuring the text. >> >> Can you provide sample dives? >> >> Without it if you turn on the tank bar the temperature graph is often >> drawn below the tank bar which is far worse. >> >> I looked at about 100 of my dives with my patch and in all cases get a >> reasonable profile graph with and without partial pressures, with and >> without the tank graph. So I'd need a sample drive where this fails to be >> able to tweak the algorith to work in more cases. >> >> Thanks >> >> /D > OK, this seems to be a case only when the tank bar is visible and not the gas > profiles. > If you activate the gas profiles or deactivate the tank bar the issue > dissapears. > I’ll send in a patch to fix it. > Dirk test this patch and see it if looks ok on your dives. 0001-Prevent-the-tank-bar-from-overlaping-the-temperature.patch Description: Binary data /Jocke ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: two patches
On Fri, Dec 12, 2014 at 05:03:54PM -0200, Tomaz Canabrava wrote: > > So I'm puzzled how often we are adding bookmark events and how that > > affects performance... or is this really just a first of a series and the > > others are more performance relevant? > > First of a series. OK > > Dear C expert... I am curious. Why are you doing this with a struct event > > ** > > instead of just a struct event *... you need the ** if you want to be able > > to modify the list in an elegant way, but all you do here is walk the > > list, so this could drop one level of indirection... or am I missing > > something? > > Copied from the code just above that one. But that code modified the list, right? > > And why add the curly braces, anyway? > > I was debugging and forgot to remove them. Tsktsktsk. So you added something because you forgot to remove it and introduced whitespace damage that way... > > This could do something really bad if for some reason > > internalEvent->time.seconds is negative, right? then entry[0].sec (which > > should always be 0) could already be bigger and we access element entry[-1] > > Is that possible? Off the top of my head I'd say "unlikely", but I worry about dives imported from other sources or something else that could break this assumption. So I'd rather have you test for i > 0 before you access entry[i - 1] > > So this is pretty invasive and the patch has a few issues. > > > > Why should this go in before 4.3? Is there a specific bug this addresses? > > > > 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. > while I was trying to fix that I got carried away. I hear that happens. So does this mean "I'll rework this for until after 4.3" or "I'll rework this to address the issues mentioned and then submit the series that fixes the bigger problem later today"? > I think I'll just > generate the pixmap of the Glyphs and reuse them in the future for the > PainterPath issue. Or that and we forget the patch? That seems wrong because the idea in the patch seems correct. /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: commit c76cb59
> On 12 Dec 2014, at 20:00, Dirk Hohndel wrote: > > On Fri, Dec 12, 2014 at 07:46:29PM +0100, Joakim Bygdell wrote: >> Dirk, I suggest that you reverse commit c76cb59. >> After your patch the line of the temperature graph sometimes gets drawn >> through the depth notation of the deepest point of the profile thereby >> obscuring the text. > > Can you provide sample dives? > > Without it if you turn on the tank bar the temperature graph is often > drawn below the tank bar which is far worse. > > I looked at about 100 of my dives with my patch and in all cases get a > reasonable profile graph with and without partial pressures, with and > without the tank graph. So I'd need a sample drive where this fails to be > able to tweak the algorith to work in more cases. > > Thanks > > /D OK, this seems to be a case only when the tank bar is visible and not the gas profiles. If you activate the gas profiles or deactivate the tank bar the issue dissapears. I’ll send in a patch to fix it. /Jocke ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: commit c76cb59
People, setZValue( -10 ) or something, in the offending item that's overlapping the one that shouldn't be overlapped. On Fri, Dec 12, 2014 at 5:00 PM, Dirk Hohndel wrote: > > On Fri, Dec 12, 2014 at 07:46:29PM +0100, Joakim Bygdell wrote: > > Dirk, I suggest that you reverse commit c76cb59. > > After your patch the line of the temperature graph sometimes gets drawn > through the depth notation of the deepest point of the profile thereby > obscuring the text. > > Can you provide sample dives? > > Without it if you turn on the tank bar the temperature graph is often > drawn below the tank bar which is far worse. > > I looked at about 100 of my dives with my patch and in all cases get a > reasonable profile graph with and without partial pressures, with and > without the tank graph. So I'd need a sample drive where this fails to be > able to tweak the algorith to work in more cases. > > Thanks > > /D > ___ > subsurface mailing list > subsurface@subsurface-divelog.org > http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface > ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: two patches
> So I'm puzzled how often we are adding bookmark events and how that
> affects performance... or is this really just a first of a series and the
> others are more performance relevant?
>
First of a series.
>
> > diff --git a/dive.c b/dive.c
> > index b318c4b..4fb6717 100644
> > --- a/dive.c
> > +++ b/dive.c
> > @@ -150,6 +150,18 @@ void update_event_name(struct dive *d, struct event
> *event, char *name)
> > free(remove);
> > }
> >
> > +struct event *get_event_by_time(struct dive *d, uint32_t time){
> > + if (!d)
> > + return NULL;
> > + struct divecomputer *dc = get_dive_dc(d, dc_number);
> > + if (!dc)
> > + return NULL;
> > + struct event **events = &dc->events;
> > + while((*events)->next && (*events)->time.seconds != time)
> > + events = &(*events)->next;
> > + return (*events);
> > +}
>
> Dear C expert... I am curious. Why are you doing this with a struct event
> **
> instead of just a struct event *... you need the ** if you want to be able
> to modify the list in an elegant way, but all you do here is walk the
> list, so this could drop one level of indirection... or am I missing
> something?
>
Copied from the code just above that one.
>
> > diff --git a/qt-ui/profile/diveeventitem.cpp
> b/qt-ui/profile/diveeventitem.cpp
> > index a9c3c3f..14cb0a3 100644
> > --- a/qt-ui/profile/diveeventitem.cpp
> > +++ b/qt-ui/profile/diveeventitem.cpp
> > @@ -9,6 +9,7 @@
> > #include
> > #include "gettextfromc.h"
> > #include "metrics.h"
> > +#include
> >
> > extern struct ev_select *ev_namelist;
> > extern int evn_used;
> > @@ -152,18 +153,27 @@ bool DiveEventItem::shouldBeHidden()
> >
> > void DiveEventItem::recalculatePos(bool instant)
> > {
> > - if (!vAxis || !hAxis || !internalEvent || !dataModel)
> > + if (!vAxis || !hAxis || !internalEvent || !dataModel){
>
> WHITESPACE
>
MEEH
> > return;
> > + }
>
> And why add the curly braces, anyway?
>
I was debugging and forgot to remove them.
>
> > - QModelIndexList result = dataModel->match(dataModel->index(0,
> DivePlotDataModel::TIME), Qt::DisplayRole, internalEvent->time.seconds);
> > - if (result.isEmpty()) {
> > - Q_ASSERT("can't find a spot in the dataModel");
> > - hide();
> > - return;
> > + // find the correct time or interpolate between two points.
> > + int depth = -1;
> > + plot_data *entry = dataModel->data().entry;
> > + for(int i = 0; i < dataModel->data().nr; i++){
> > + if (entry[i].sec == internalEvent->time.seconds){
> > + depth = entry[i].depth;
> > + break;
> > + } else if (entry[i].sec > internalEvent->time.seconds) {
> > + int min = entry[i-1].depth;
>
> This could do something really bad if for some reason
> internalEvent->time.seconds is negative, right? then entry[0].sec (which
> should always be 0) could already be bigger and we access element entry[-1]
>
Is that possible?
>
> > + int max = entry[i].depth;
> > + depth = min + ((max - min)/2);
> > + break;
> > + }
> > }
> > +
> > if (!isVisible() && !shouldBeHidden())
> > show();
> > - int depth = dataModel->data(dataModel->index(result.first().row(),
> DivePlotDataModel::DEPTH)).toInt();
> > qreal x = hAxis->posAtValue(internalEvent->time.seconds);
> > qreal y = vAxis->posAtValue(depth);
> > if (!instant)
> > @@ -172,4 +182,5 @@ void DiveEventItem::recalculatePos(bool instant)
> > setPos(x, y);
> > if (isVisible() && shouldBeHidden())
> > hide();
> > + qDebug() << pos();
>
> Grrrmbl
>
MEEEH
>
> > diff --git a/qt-ui/profile/profilewidget2.cpp
> b/qt-ui/profile/profilewidget2.cpp
> > index 1970561..1181fc5 100644
> > --- a/qt-ui/profile/profilewidget2.cpp
> > +++ b/qt-ui/profile/profilewidget2.cpp
> > @@ -1277,8 +1277,11 @@ void ProfileWidget2::removeEvent()
> > tr("%1 @
> %2:%3").arg(event->name).arg(event->time.seconds /
> 60).arg(event->time.seconds % 60, 2, 10, QChar('0'))),
> > QMessageBox::Ok | QMessageBox::Cancel)
> == QMessageBox::Ok) {
> > remove_event(event);
> > + copy_events(¤t_dive->dc, &displayed_dive.dc);
>
> this leaks memory, the events in displayed_dive are overwritten without
> being freed. Call free_events() on the existing events in displayed_dive
>
Oh, sorry.
>
> > mark_divelist_changed(true);
> > - replot();
> > + scene()->removeItem(item);
> > + eventItems.removeOne(item);
> > + item->deleteLater();
>
> You are the expert on that code :-)
>
> > @@ -1287,8 +1290,20 @@ void ProfileWidget2::addBookmark()
> > QAction *action = qobject_cast(sender());
> > QP
Re: commit c76cb59
On Fri, Dec 12, 2014 at 07:46:29PM +0100, Joakim Bygdell wrote: > Dirk, I suggest that you reverse commit c76cb59. > After your patch the line of the temperature graph sometimes gets drawn > through the depth notation of the deepest point of the profile thereby > obscuring the text. Can you provide sample dives? Without it if you turn on the tank bar the temperature graph is often drawn below the tank bar which is far worse. I looked at about 100 of my dives with my patch and in all cases get a reasonable profile graph with and without partial pressures, with and without the tank graph. So I'd need a sample drive where this fails to be able to tweak the algorith to work in more cases. Thanks /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: bug hunt / trac day
On Fri, Dec 12, 2014 at 05:53:42PM +, Pedro Neves wrote: > Of course... > > Debian/Sid. Qt 4.8.6, compiled from source on my system... Ah. Yes. Qt 4.x is known to have printing issues. That's why I went through all this work to have the majority of the binaries we provide (Win64, Mac, Ubuntu, LinuxMint) all use Qt5. We have an old Mac 32bit binary that I wasn't planning to update for 4.3 until someone complaints (it has been downloaded 4 times by something that doesn't look like a spider) which uses Qt4 and we have the Windows 32bit binary (used by about 10% of our users) that is also Qt4... > Hope this helps: It does. Thanks /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: two patches
On Fri, Dec 12, 2014 at 03:53:28PM -0200, Tomaz Canabrava wrote:
> From cc64959903353c1d1cb4b182d50246d7fd1cd701 Mon Sep 17 00:00:00 2001
> From: Tomaz Canabrava
> Date: Fri, 12 Dec 2014 14:06:04 -0200
> Subject: [PATCH 2/2] Fixes the Bookmark Add/Rename/Remove witout replotting
> everything.
>
> This seems to work. I'v reworked the Event positioning code so
> we don't need to recreate the full DisplayedDive when a new
> bookmark is added, just figure out where it should be added
> and position it at the right spot.
>
> This is a series of patches to start using less replots
> on the Profile.
So I'm puzzled how often we are adding bookmark events and how that
affects performance... or is this really just a first of a series and the
others are more performance relevant?
> diff --git a/dive.c b/dive.c
> index b318c4b..4fb6717 100644
> --- a/dive.c
> +++ b/dive.c
> @@ -150,6 +150,18 @@ void update_event_name(struct dive *d, struct event
> *event, char *name)
> free(remove);
> }
>
> +struct event *get_event_by_time(struct dive *d, uint32_t time){
> + if (!d)
> + return NULL;
> + struct divecomputer *dc = get_dive_dc(d, dc_number);
> + if (!dc)
> + return NULL;
> + struct event **events = &dc->events;
> + while((*events)->next && (*events)->time.seconds != time)
> + events = &(*events)->next;
> + return (*events);
> +}
Dear C expert... I am curious. Why are you doing this with a struct event **
instead of just a struct event *... you need the ** if you want to be able
to modify the list in an elegant way, but all you do here is walk the
list, so this could drop one level of indirection... or am I missing
something?
> diff --git a/qt-ui/profile/diveeventitem.cpp b/qt-ui/profile/diveeventitem.cpp
> index a9c3c3f..14cb0a3 100644
> --- a/qt-ui/profile/diveeventitem.cpp
> +++ b/qt-ui/profile/diveeventitem.cpp
> @@ -9,6 +9,7 @@
> #include
> #include "gettextfromc.h"
> #include "metrics.h"
> +#include
>
> extern struct ev_select *ev_namelist;
> extern int evn_used;
> @@ -152,18 +153,27 @@ bool DiveEventItem::shouldBeHidden()
>
> void DiveEventItem::recalculatePos(bool instant)
> {
> - if (!vAxis || !hAxis || !internalEvent || !dataModel)
> + if (!vAxis || !hAxis || !internalEvent || !dataModel){
WHITESPACE
> return;
> + }
And why add the curly braces, anyway?
> - QModelIndexList result = dataModel->match(dataModel->index(0,
> DivePlotDataModel::TIME), Qt::DisplayRole, internalEvent->time.seconds);
> - if (result.isEmpty()) {
> - Q_ASSERT("can't find a spot in the dataModel");
> - hide();
> - return;
> + // find the correct time or interpolate between two points.
> + int depth = -1;
> + plot_data *entry = dataModel->data().entry;
> + for(int i = 0; i < dataModel->data().nr; i++){
> + if (entry[i].sec == internalEvent->time.seconds){
> + depth = entry[i].depth;
> + break;
> + } else if (entry[i].sec > internalEvent->time.seconds) {
> + int min = entry[i-1].depth;
This could do something really bad if for some reason
internalEvent->time.seconds is negative, right? then entry[0].sec (which
should always be 0) could already be bigger and we access element entry[-1]
> + int max = entry[i].depth;
> + depth = min + ((max - min)/2);
> + break;
> + }
> }
> +
> if (!isVisible() && !shouldBeHidden())
> show();
> - int depth = dataModel->data(dataModel->index(result.first().row(),
> DivePlotDataModel::DEPTH)).toInt();
> qreal x = hAxis->posAtValue(internalEvent->time.seconds);
> qreal y = vAxis->posAtValue(depth);
> if (!instant)
> @@ -172,4 +182,5 @@ void DiveEventItem::recalculatePos(bool instant)
> setPos(x, y);
> if (isVisible() && shouldBeHidden())
> hide();
> + qDebug() << pos();
Grrrmbl
> diff --git a/qt-ui/profile/profilewidget2.cpp
> b/qt-ui/profile/profilewidget2.cpp
> index 1970561..1181fc5 100644
> --- a/qt-ui/profile/profilewidget2.cpp
> +++ b/qt-ui/profile/profilewidget2.cpp
> @@ -1277,8 +1277,11 @@ void ProfileWidget2::removeEvent()
> tr("%1 @
> %2:%3").arg(event->name).arg(event->time.seconds /
> 60).arg(event->time.seconds % 60, 2, 10, QChar('0'))),
> QMessageBox::Ok | QMessageBox::Cancel) ==
> QMessageBox::Ok) {
> remove_event(event);
> + copy_events(¤t_dive->dc, &displayed_dive.dc);
this leaks memory, the events in displayed_dive are overwritten without
being freed. Call free_events() on the existing events in displayed_dive
> mark_divelist_changed(true);
> - replot();
> + scene()->removeItem(item);
> + eventItems
commit c76cb59
Dirk, I suggest that you reverse commit c76cb59. After your patch the line of the temperature graph sometimes gets drawn through the depth notation of the deepest point of the profile thereby obscuring the text. /Jocke ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
two patches
Dirk,
Take a look at 002 carefully, since I changed and added a few things to
event manipulation.
the idea of 0002 is to add / remove / rename an Bookmark without triggering
an (expesive) redraw of the profile, so we create just the Event item and
put it on screen.
The issue is that the depth of the bookark is calculated based on the
events of the current_dive when we create_plot_data. so I called
copy_events(current_dive, displayed_dive) and did it by hand.
Tomaz
From cc64959903353c1d1cb4b182d50246d7fd1cd701 Mon Sep 17 00:00:00 2001
From: Tomaz Canabrava
Date: Fri, 12 Dec 2014 14:06:04 -0200
Subject: [PATCH 2/2] Fixes the Bookmark Add/Rename/Remove witout replotting
everything.
This seems to work. I'v reworked the Event positioning code so
we don't need to recreate the full DisplayedDive when a new
bookmark is added, just figure out where it should be added
and position it at the right spot.
This is a series of patches to start using less replots
on the Profile.
Signed-off-by: Tomaz Canabrava
---
dive.c | 12
dive.h | 2 +-
qt-ui/profile/diveeventitem.cpp | 25 ++---
qt-ui/profile/profilewidget2.cpp | 23 ---
4 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/dive.c b/dive.c
index b318c4b..4fb6717 100644
--- a/dive.c
+++ b/dive.c
@@ -150,6 +150,18 @@ void update_event_name(struct dive *d, struct event *event, char *name)
free(remove);
}
+struct event *get_event_by_time(struct dive *d, uint32_t time){
+ if (!d)
+ return NULL;
+ struct divecomputer *dc = get_dive_dc(d, dc_number);
+ if (!dc)
+ return NULL;
+ struct event **events = &dc->events;
+ while((*events)->next && (*events)->time.seconds != time)
+ events = &(*events)->next;
+ return (*events);
+}
+
void add_extra_data(struct divecomputer *dc, const char *key, const char *value)
{
struct extra_data **ed = &dc->extra_data;
diff --git a/dive.h b/dive.h
index f67c736..979c8bf 100644
--- a/dive.h
+++ b/dive.h
@@ -709,7 +709,7 @@ extern void per_cylinder_mean_depth(struct dive *dive, struct divecomputer *dc,
extern int get_cylinder_index(struct dive *dive, struct event *ev);
extern int nr_cylinders(struct dive *dive);
extern int nr_weightsystems(struct dive *dive);
-
+extern struct event *get_event_by_time(struct dive *d, uint32_t time);
/* UI related protopypes */
// extern void report_error(GError* error);
diff --git a/qt-ui/profile/diveeventitem.cpp b/qt-ui/profile/diveeventitem.cpp
index a9c3c3f..14cb0a3 100644
--- a/qt-ui/profile/diveeventitem.cpp
+++ b/qt-ui/profile/diveeventitem.cpp
@@ -9,6 +9,7 @@
#include
#include "gettextfromc.h"
#include "metrics.h"
+#include
extern struct ev_select *ev_namelist;
extern int evn_used;
@@ -152,18 +153,27 @@ bool DiveEventItem::shouldBeHidden()
void DiveEventItem::recalculatePos(bool instant)
{
- if (!vAxis || !hAxis || !internalEvent || !dataModel)
+ if (!vAxis || !hAxis || !internalEvent || !dataModel){
return;
+ }
- QModelIndexList result = dataModel->match(dataModel->index(0, DivePlotDataModel::TIME), Qt::DisplayRole, internalEvent->time.seconds);
- if (result.isEmpty()) {
- Q_ASSERT("can't find a spot in the dataModel");
- hide();
- return;
+ // find the correct time or interpolate between two points.
+ int depth = -1;
+ plot_data *entry = dataModel->data().entry;
+ for(int i = 0; i < dataModel->data().nr; i++){
+ if (entry[i].sec == internalEvent->time.seconds){
+ depth = entry[i].depth;
+ break;
+ } else if (entry[i].sec > internalEvent->time.seconds) {
+ int min = entry[i-1].depth;
+ int max = entry[i].depth;
+ depth = min + ((max - min)/2);
+ break;
+ }
}
+
if (!isVisible() && !shouldBeHidden())
show();
- int depth = dataModel->data(dataModel->index(result.first().row(), DivePlotDataModel::DEPTH)).toInt();
qreal x = hAxis->posAtValue(internalEvent->time.seconds);
qreal y = vAxis->posAtValue(depth);
if (!instant)
@@ -172,4 +182,5 @@ void DiveEventItem::recalculatePos(bool instant)
setPos(x, y);
if (isVisible() && shouldBeHidden())
hide();
+ qDebug() << pos();
}
diff --git a/qt-ui/profile/profilewidget2.cpp b/qt-ui/profile/profilewidget2.cpp
index 1970561..1181fc5 100644
--- a/qt-ui/profile/profilewidget2.cpp
+++ b/qt-ui/profile/profilewidget2.cpp
@@ -1277,8 +1277,11 @@ void ProfileWidget2::removeEvent()
tr("%1 @ %2:%3").arg(event->name).arg(event->time.seconds / 60).arg(event->time.seconds % 60, 2, 10, QChar('0'))),
QMessageBox::Ok | QMessageBox::Cancel) == QMessageBox::Ok) {
remove_event(event);
+ copy_events(¤t_dive->dc, &displayed_dive.dc);
mark_divelist_changed(true);
- replot();
+ scene()->removeItem(item);
+ eventItems.removeOne(item);
+ item->deleteLater();
}
}
@@ -1287,8 +1290,20 @@ void ProfileWidget2::addBookmark()
QAction *action = qobject_cast(sender());
QPointF scenePos = mapToScene(mapFromGlobal(action->data().toPoint()));
add_event(current_dc, time
Re: bug hunt / trac day
On Fri, Dec 12, 2014 at 04:53:07PM +, Pedro Neves wrote: > Hi all: > > On 12/12/2014 03:25 PM, Dirk Hohndel wrote: > >Play with printing (we seem to only check on that prior > >to a release). > > I found printing a bit strange. The profile is still rendered somewhat > "blurred " and the spacing between some of the characters seems uneven. > > I've pasted an image on the following link: > http://s5.postimg.org/wj9u6c7jb/Profile_print.jpg > > The areas marked in red are the areas that seem strange. The printing > settings were: print to PDF; 2 diver per page; A4 size. > > I remember a while back the issue with the profile rendering was raised. > Maybe it wasn't fixed, maybe it's my system... Can you tell us more about the binary you are using? Which OS? Did you build it yourself? Qt4 or Qt5? /D ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: bug hunt / trac day - EDIT|
Hi: I forgot to mention on my previous post that on the image I pasted (http://s5.postimg.org/wj9u6c7jb/Profile_print.jpg): - The gas used item show the name of the tank and the gas used. Maybe we should change the label to "tanks and gas used"? - The buddy list is truncated... Cheers: Pedro ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: bug hunt / trac day
Hi all: On 12/12/2014 03:25 PM, Dirk Hohndel wrote: Play with printing (we seem to only check on that prior to a release). I found printing a bit strange. The profile is still rendered somewhat "blurred " and the spacing between some of the characters seems uneven. I've pasted an image on the following link: http://s5.postimg.org/wj9u6c7jb/Profile_print.jpg The areas marked in red are the areas that seem strange. The printing settings were: print to PDF; 2 diver per page; A4 size. I remember a while back the issue with the profile rendering was raised. Maybe it wasn't fixed, maybe it's my system... Cheers: Pedro ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: bug hunt / trac day
I'm trying hard to speed up the profile here. when we are creating a new dive, if we just move the handler around for a while, CPU goes to 100%. managed to reduce the amount of calls to QPainterPath::addText by 1/4, wich helped a lot, but it seems that Qt calls that internally so I'm now trying to not replot the whole scene for a single move on the handler. On Fri, Dec 12, 2014 at 1:25 PM, Dirk Hohndel wrote: > > Hi there, > > with 4.3 almost ready to go, can I ask for a favor? > > There are a TON of open bugs in trac. > > Many of them are for divecomputer support - those are hard to track down > and usually only Jef can really answer them. But many other ones are > things that need to be followed up on. Ask the reporter if the bug is > still there in the latest daily, for example. Try to reproduce it yourself > (in case it doesn't require hardware you don't have access to). > > Even if you are not a developer and don't necessarily know how to fix it, > following up, asking questions really helps. > > And similarly, can I ask people to really try and hunt for bugs? > Play with filtering. There are many many combinations of actions one could > take. Filter, select, edit, accept or discard... can you produce incorrect > results / crashes? Keep track of what you are doing (so we can reproduce > it) and report it. Play with printing (we seem to only check on that prior > to a release). Statistics. Importing and exporting. Etc. > > Just give it a good workout, please. > > Thanks > > /D > > PS: special KUDOS to Robert and Tomaz who are already working on closing > trac bugs. Much appreciated. > ___ > subsurface mailing list > subsurface@subsurface-divelog.org > http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface > ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
bug hunt / trac day
Hi there, with 4.3 almost ready to go, can I ask for a favor? There are a TON of open bugs in trac. Many of them are for divecomputer support - those are hard to track down and usually only Jef can really answer them. But many other ones are things that need to be followed up on. Ask the reporter if the bug is still there in the latest daily, for example. Try to reproduce it yourself (in case it doesn't require hardware you don't have access to). Even if you are not a developer and don't necessarily know how to fix it, following up, asking questions really helps. And similarly, can I ask people to really try and hunt for bugs? Play with filtering. There are many many combinations of actions one could take. Filter, select, edit, accept or discard... can you produce incorrect results / crashes? Keep track of what you are doing (so we can reproduce it) and report it. Play with printing (we seem to only check on that prior to a release). Statistics. Importing and exporting. Etc. Just give it a good workout, please. Thanks /D PS: special KUDOS to Robert and Tomaz who are already working on closing trac bugs. Much appreciated. ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
Re: [PATCH 2/2] Don't rely on malloc to return NULL for zero size
Ack. With "our selfs" corrected to "ourselves" in the comment :-) Linus On Dec 11, 2014 11:59 PM, "Anton Lundin" wrote: > We rely on samples being NULL if a dc have no samples. Its completely > legal for malloc to return a valid pointer to nowhere for zero sized > malloc, which you can't follow and read what its pointing at. Its only > viable to call free() on. > > In other code, if samples is a valid pointer, we dereference it and look > at the first sample. > > Signed-off-by: Anton Lundin > --- > dive.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/dive.c b/dive.c > index 8e8330f..4cf532f 100644 > --- a/dive.c > +++ b/dive.c > @@ -602,6 +602,14 @@ void copy_samples(struct divecomputer *s, struct > divecomputer *d) > int nr = s->samples; > d->samples = nr; > d->alloc_samples = nr; > + // We expect to be able to read the memory in the other end of the > pointer > + // if its a valid pointer, so don't expect malloc() to return NULL > for > + // zero-sized malloc, do it our selfs. > + d->sample = NULL; > + > + if(!nr) > + return; > + > d->sample = malloc(nr * sizeof(struct sample)); > if (d->sample) > memcpy(d->sample, s->sample, nr * sizeof(struct sample)); > -- > 2.1.0 > > ___ > subsurface mailing list > subsurface@subsurface-divelog.org > http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface > ___ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
