On Mittwoch, 13. Dezember 2017 23:34:32 CET Dirk Hohndel wrote:
> > On Dec 13, 2017, at 11:31 AM, Berthold Stoeger
> > <[email protected]> wrote:>
> > PS: compilation of libdivecomputer produces tons of warnings. :-o
>
> Yes, Jef cranked up the warning level quite a bit.
> I actually removed two of them (-Wextra and -Wpedantic) to keep things at
> least somewhat manageable, but it is noisy.
Speaking of warnings: how about compiling subsurface with -Wall as a default?
At least on gcc it's pretty clean and only seems to address real issues.
Concerning the float warning, I have a patch that turns all float calculations
into double calculations sitting around for a few weeks. I think float is like
short: only to be used in very specific circumstances. But I'll let the pros
decide.
Berthold
>From 8f2ee2062322730610d79cce612e65f317990d2f Mon Sep 17 00:00:00 2001
From: Berthold Stoeger <[email protected]>
Date: Mon, 20 Nov 2017 21:39:50 +0100
Subject: [PATCH] Unify float calulations: use double
Internal floating point (FP) calculations should be performed using double
unless there is a very good reason. This avoids headaches with conversions.
Indeed, the vast majority of FP calculations were already done using double.
This patch adapts the remaining calculations.
An analysis of all instances follows:
core/plannernotes.c, l.404:
This was a comparison between two floats. On the left side, first an integer
was cast to float then multiplied with and integer and divided by a constant
double. The right hand side was an integer cast to a float. Simply divide by
1000.0 first to convert to double and continue with calculations. On the right
hand side, remove the cast, because the integer will be implicitely cast to
double for comparison.
core/planner.c, l.613:
Same analysis as previous case.
core/liquivision.c, l.267:
Here a local variable is declared as float. Just make it a double. Note that
the remaining float-casts in this file cannot be removed, because they concern
binary representations.
core/uemis.c, l.333:
Again a local variable. This one is directly cast back to int in the next line.
Let's make it a double for consistency anyway.
subsurface-desktop-main.cpp, l.155:
Again a local variable, here representing the version OpenGL version. Turn this
into integer logic. Not only does this avoid dreaded FP rounding issues, it also
works correctly for minor version > 10 (not that such a thing is to be expected
anytime soon).
abstractpreferenceswidget.[h/cpp]:
A widget where the position is described as a float. Turn into double.
desktop-widgets/divelogexportdialog.cpp, l.313:
total_weight is described as float. Use double arithmetics instead. This
instance fixes a truncation warning emitted by g++.
Signed-off-by: Berthold Stoeger <[email protected]>
---
core/liquivision.c | 2 +-
core/planner.c | 2 +-
core/plannernotes.c | 4 ++--
core/uemis.c | 4 ++--
desktop-widgets/divelogexportdialog.cpp | 2 +-
desktop-widgets/preferences/abstractpreferenceswidget.cpp | 4 ++--
desktop-widgets/preferences/abstractpreferenceswidget.h | 6 +++---
subsurface-desktop-main.cpp | 8 ++++----
8 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/core/liquivision.c b/core/liquivision.c
index f0762409c..1377b6c0b 100644
--- a/core/liquivision.c
+++ b/core/liquivision.c
@@ -264,7 +264,7 @@ static void parse_dives (int log_version, const unsigned char *buf, unsigned int
sample_interval = intervals[*(buf + ptr)];
ptr += 1;
- float start_cns = 0;
+ double start_cns = 0;
unsigned char dive_mode = 0, algorithm = 0;
if (array_uint32_le(buf + ptr) != sample_count) {
// Xeo, with CNS and OTU
diff --git a/core/planner.c b/core/planner.c
index 8fb03015b..8ace46e35 100644
--- a/core/planner.c
+++ b/core/planner.c
@@ -610,7 +610,7 @@ bool enough_gas(int current_cylinder)
if (!cyl->start.mbar)
return true;
if (cyl->type.size.mliter)
- return (float)(cyl->end.mbar - prefs.reserve_gas) * cyl->type.size.mliter / 1000.0 > (float) cyl->deco_gas_used.mliter;
+ return (cyl->end.mbar - prefs.reserve_gas) / 1000.0 * cyl->type.size.mliter > cyl->deco_gas_used.mliter;
else
return true;
}
diff --git a/core/plannernotes.c b/core/plannernotes.c
index 2e38aaff9..1773705b4 100644
--- a/core/plannernotes.c
+++ b/core/plannernotes.c
@@ -401,8 +401,8 @@ void add_plan_to_notes(struct diveplan *diveplan, struct dive *dive, bool show_d
translate("gettextFromC", "Warning:"),
translate("gettextFromC", "this is more gas than available in the specified cylinder!"));
else
- if ((float) cyl->end.mbar * cyl->type.size.mliter / 1000.0 / gas_compressibility_factor(&cyl->gasmix, cyl->end.mbar / 1000.0)
- < (float) cyl->deco_gas_used.mliter)
+ if (cyl->end.mbar / 1000.0 * cyl->type.size.mliter / gas_compressibility_factor(&cyl->gasmix, cyl->end.mbar / 1000.0)
+ < cyl->deco_gas_used.mliter)
snprintf(warning, sizeof(warning), "<br> — <span style='color: red;'>%s </span> %s",
translate("gettextFromC", "Warning:"),
translate("gettextFromC", "not enough reserve for gas sharing on ascent!"));
diff --git a/core/uemis.c b/core/uemis.c
index eeb8a79c4..09395a3da 100644
--- a/core/uemis.c
+++ b/core/uemis.c
@@ -330,7 +330,7 @@ void uemis_parse_divelog_binary(char *base64, void *datap)
if (template == 0)
template = 1;
for (i = 0; i < template; i++) {
- float volume = *(float *)(data + 116 + 25 * (gasoffset + i)) * 1000.0f;
+ double volume = *(float *)(data + 116 + 25 * (gasoffset + i)) * 1000.0;
/* uemis always assumes a working pressure of 202.6bar (!?!?) - I first thought
* it was 3000psi, but testing against all my dives gets me that strange number.
* Still, that's of course completely bogus and shows they don't get how
@@ -338,7 +338,7 @@ void uemis_parse_divelog_binary(char *base64, void *datap)
* we store the incorrect working pressure to get the SAC calculations "close"
* but the user will have to correct this manually
*/
- dive->cylinder[i].type.size.mliter = lrintf(volume);
+ dive->cylinder[i].type.size.mliter = lrint(volume);
dive->cylinder[i].type.workingpressure.mbar = 202600;
dive->cylinder[i].gasmix.o2.permille = *(uint8_t *)(data + 120 + 25 * (gasoffset + i)) * 10;
dive->cylinder[i].gasmix.he.permille = 0;
diff --git a/desktop-widgets/divelogexportdialog.cpp b/desktop-widgets/divelogexportdialog.cpp
index cb1745483..2af8d1b80 100644
--- a/desktop-widgets/divelogexportdialog.cpp
+++ b/desktop-widgets/divelogexportdialog.cpp
@@ -310,7 +310,7 @@ void DiveLogExportDialog::export_TeX(const char *filename, const bool selected_o
int i;
int qty_cyl;
int qty_weight;
- float total_weight;
+ double total_weight;
if (need_pagebreak)
put_format(&buf, "\\vfill\\eject\n");
diff --git a/desktop-widgets/preferences/abstractpreferenceswidget.cpp b/desktop-widgets/preferences/abstractpreferenceswidget.cpp
index 1b2aeaae5..88228ee98 100644
--- a/desktop-widgets/preferences/abstractpreferenceswidget.cpp
+++ b/desktop-widgets/preferences/abstractpreferenceswidget.cpp
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include "abstractpreferenceswidget.h"
-AbstractPreferencesWidget::AbstractPreferencesWidget(const QString& name, const QIcon& icon, float positionHeight)
+AbstractPreferencesWidget::AbstractPreferencesWidget(const QString& name, const QIcon& icon, double positionHeight)
: QWidget(), _icon(icon), _name(name), _positionHeight(positionHeight)
{
}
@@ -16,7 +16,7 @@ QString AbstractPreferencesWidget::name() const
return _name;
}
-float AbstractPreferencesWidget::positionHeight() const
+double AbstractPreferencesWidget::positionHeight() const
{
return _positionHeight;
}
diff --git a/desktop-widgets/preferences/abstractpreferenceswidget.h b/desktop-widgets/preferences/abstractpreferenceswidget.h
index 7e303d182..7466fda16 100644
--- a/desktop-widgets/preferences/abstractpreferenceswidget.h
+++ b/desktop-widgets/preferences/abstractpreferenceswidget.h
@@ -8,10 +8,10 @@
class AbstractPreferencesWidget : public QWidget {
Q_OBJECT
public:
- AbstractPreferencesWidget(const QString& name, const QIcon& icon, float positionHeight);
+ AbstractPreferencesWidget(const QString& name, const QIcon& icon, double positionHeight);
QIcon icon() const;
QString name() const;
- float positionHeight() const;
+ double positionHeight() const;
/* gets the values from the preferences and should set the correct values in
* the interface */
@@ -26,6 +26,6 @@ signals:
private:
QIcon _icon;
QString _name;
- float _positionHeight;
+ double _positionHeight;
};
#endif
diff --git a/subsurface-desktop-main.cpp b/subsurface-desktop-main.cpp
index 5b8424487..c439179c8 100644
--- a/subsurface-desktop-main.cpp
+++ b/subsurface-desktop-main.cpp
@@ -152,7 +152,6 @@ void validateGL()
QOffscreenSurface surface;
QOpenGLFunctions *func;
const char *verChar;
- float verFloat;
surface.setFormat(ctx.format());
surface.create();
@@ -182,9 +181,10 @@ void validateGL()
"before running Subsurface!\n").toUtf8().data();
return;
}
- if (sscanf(verChar, "%f", &verFloat) == 1) {
- verMajor = (GLint)verFloat;
- verMinor = (GLint)roundf((verFloat - verMajor) * 10.f);
+ int min, maj;
+ if (sscanf(verChar, "%d.%d", &maj, &min) == 2) {
+ verMajor = (GLint)maj;
+ verMinor = (GLint)min;
}
}
// attempt to detect version using the newer API
--
2.14.1
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
