From f264464ec68737b4eb888b9080c442bd131811fa Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 6 Oct 2018 18:50:56 +0200 Subject: [PATCH 1/6] rect: Use more efficient float calculations for ceil, floor This fixes warnings from LGTM: Multiplication result may overflow 'float' before it is converted to 'double'. While the floor function always calculates with double, here the overloaded std::floor can be used to handle the float arguments more efficiently. Replace also old C++ type casts by static_cast. Signed-off-by: Stefan Weil --- src/ccstruct/rect.h | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/ccstruct/rect.h b/src/ccstruct/rect.h index 50583f3a..344559a0 100644 --- a/src/ccstruct/rect.h +++ b/src/ccstruct/rect.h @@ -21,7 +21,7 @@ #define RECT_H #include // for std::max, std::min -#include // for ceil, floor +#include // for std::ceil, std::floor #include // for INT16_MAX #include // for FILE #include "platform.h" // for DLLSYM @@ -162,29 +162,33 @@ class DLLSYM TBOX { // bounding box void move( // move box const FCOORD vec) { // by float vector - bot_left.set_x ((int16_t) floor (bot_left.x () + vec.x ())); + bot_left.set_x(static_cast(std::floor(bot_left.x() + vec.x()))); // round left - bot_left.set_y ((int16_t) floor (bot_left.y () + vec.y ())); + bot_left.set_y(static_cast(std::floor(bot_left.y() + vec.y()))); // round down - top_right.set_x ((int16_t) ceil (top_right.x () + vec.x ())); + top_right.set_x(static_cast(std::ceil(top_right.x() + vec.x()))); // round right - top_right.set_y ((int16_t) ceil (top_right.y () + vec.y ())); + top_right.set_y(static_cast(std::ceil(top_right.y() + vec.y()))); // round up } void scale( // scale box const float f) { // by multiplier - bot_left.set_x ((int16_t) floor (bot_left.x () * f)); // round left - bot_left.set_y ((int16_t) floor (bot_left.y () * f)); // round down - top_right.set_x ((int16_t) ceil (top_right.x () * f)); // round right - top_right.set_y ((int16_t) ceil (top_right.y () * f)); // round up + // round left + bot_left.set_x(static_cast(std::floor(bot_left.x() * f))); + // round down + bot_left.set_y(static_cast(std::floor(bot_left.y() * f))); + // round right + top_right.set_x(static_cast(std::ceil(top_right.x() * f))); + // round up + top_right.set_y(static_cast(std::ceil(top_right.y() * f))); } void scale( // scale box const FCOORD vec) { // by float vector - bot_left.set_x ((int16_t) floor (bot_left.x () * vec.x ())); - bot_left.set_y ((int16_t) floor (bot_left.y () * vec.y ())); - top_right.set_x ((int16_t) ceil (top_right.x () * vec.x ())); - top_right.set_y ((int16_t) ceil (top_right.y () * vec.y ())); + bot_left.set_x(static_cast(std::floor(bot_left.x() * vec.x()))); + bot_left.set_y(static_cast(std::floor(bot_left.y() * vec.y()))); + top_right.set_x(static_cast(std::ceil(top_right.x() * vec.x()))); + top_right.set_y(static_cast(std::ceil(top_right.y() * vec.y()))); } // rotate doesn't enlarge the box - it just rotates the bottom-left @@ -314,8 +318,10 @@ class DLLSYM TBOX { // bounding box inline TBOX::TBOX( // constructor const FCOORD pt // floating centre ) { - bot_left = ICOORD ((int16_t) floor (pt.x ()), (int16_t) floor (pt.y ())); - top_right = ICOORD ((int16_t) ceil (pt.x ()), (int16_t) ceil (pt.y ())); + bot_left = ICOORD(static_cast(std::floor(pt.x())), + static_cast(std::floor(pt.y()))); + top_right = ICOORD(static_cast(std::ceil(pt.x())), + static_cast(std::ceil(pt.y()))); } From 819c43d3775951f0b6e337abc1cf22680aebc2a9 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 6 Oct 2018 18:59:23 +0200 Subject: [PATCH 2/6] chop: Use more efficient float calculations for sqrt This fixes warnings from LGTM: Multiplication result may overflow 'float' before it is converted to 'double'. While the sqrt function always calculates with double, here the overloaded std::sqrt can be used to handle the float arguments more efficiently. Replace also an old C++ type cast by a static_cast. Signed-off-by: Stefan Weil --- src/wordrec/chop.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/wordrec/chop.cpp b/src/wordrec/chop.cpp index 036041d6..c15b9a40 100644 --- a/src/wordrec/chop.cpp +++ b/src/wordrec/chop.cpp @@ -89,7 +89,6 @@ int Wordrec::angle_change(EDGEPT *point1, EDGEPT *point2, EDGEPT *point3) { VECTOR vector2; int angle; - float length; /* Compute angle */ vector1.x = point2->pos.x - point1->pos.x; @@ -97,7 +96,7 @@ int Wordrec::angle_change(EDGEPT *point1, EDGEPT *point2, EDGEPT *point3) { vector2.x = point3->pos.x - point2->pos.x; vector2.y = point3->pos.y - point2->pos.y; /* Use cross product */ - length = (float)sqrt((float)LENGTH(vector1) * LENGTH(vector2)); + float length = std::sqrt(static_cast(LENGTH(vector1)) * LENGTH(vector2)); if ((int) length == 0) return (0); angle = static_cast(floor(asin(CROSS (vector1, vector2) / From a7982185c9711bc2796a37c4424e5431b8369613 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 6 Oct 2018 19:39:58 +0200 Subject: [PATCH 3/6] genericvector: Pass parameters by reference This fixes warnings like the following one from LGTM: This parameter of type ParamsTrainingHypothesis is 112 bytes - consider passing a pointer/reference instead. Most parameters can also get the const attribute. Signed-off-by: Stefan Weil --- src/ccutil/genericvector.h | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/ccutil/genericvector.h b/src/ccutil/genericvector.h index e6878dd1..e0d31b47 100644 --- a/src/ccutil/genericvector.h +++ b/src/ccutil/genericvector.h @@ -39,7 +39,7 @@ class GenericVector { GenericVector() { init(kDefaultVectorSize); } - GenericVector(int size, T init_val) { + GenericVector(int size, const T& init_val) { init(size); init_to_size(size, init_val); } @@ -60,7 +60,7 @@ class GenericVector { void double_the_size(); // Resizes to size and sets all values to t. - void init_to_size(int size, T t); + void init_to_size(int size, const T& t); // Resizes to size without any initialization. void resize_no_init(int size) { reserve(size); @@ -101,31 +101,31 @@ class GenericVector { // Return the index of the T object. // This method NEEDS a compare_callback to be passed to // set_compare_callback. - int get_index(T object) const; + int get_index(const T& object) const; // Return true if T is in the array - bool contains(T object) const; + bool contains(const T& object) const; // Return true if the index is valid T contains_index(int index) const; // Push an element in the end of the array int push_back(T object); - void operator+=(T t); + void operator+=(const T& t); // Push an element in the end of the array if the same // element is not already contained in the array. - int push_back_new(T object); + int push_back_new(const T& object); // Push an element in the front of the array // Note: This function is O(n) - int push_front(T object); + int push_front(const T& object); // Set the value at the given index - void set(T t, int index); + void set(const T& t, int index); // Insert t at the given index, push other elements to the right. - void insert(T t, int index); + void insert(const T& t, int index); // Removes an element at the given index and // shifts the remaining elements to the left. @@ -705,7 +705,7 @@ void GenericVector::double_the_size() { // Resizes to size and sets all values to t. template -void GenericVector::init_to_size(int size, T t) { +void GenericVector::init_to_size(int size, const T& t) { reserve(size); size_used_ = size; for (int i = 0; i < size; ++i) @@ -740,7 +740,7 @@ T GenericVector::pop_back() { // Return the object from an index. template -void GenericVector::set(T t, int index) { +void GenericVector::set(const T& t, int index) { assert(index >= 0 && index < size_used_); data_[index] = t; } @@ -749,7 +749,7 @@ void GenericVector::set(T t, int index) { // space for the new elements and inserts the given element // at the specified index. template -void GenericVector::insert(T t, int index) { +void GenericVector::insert(const T& t, int index) { assert(index >= 0 && index <= size_used_); if (size_reserved_ == size_used_) double_the_size(); @@ -779,7 +779,7 @@ T GenericVector::contains_index(int index) const { // Return the index of the T object. template -int GenericVector::get_index(T object) const { +int GenericVector::get_index(const T& object) const { for (int i = 0; i < size_used_; ++i) { assert(compare_cb_ != nullptr); if (compare_cb_->Run(object, data_[i])) @@ -790,7 +790,7 @@ int GenericVector::get_index(T object) const { // Return true if T is in the array template -bool GenericVector::contains(T object) const { +bool GenericVector::contains(const T& object) const { return get_index(object) != -1; } @@ -806,7 +806,7 @@ int GenericVector::push_back(T object) { } template -int GenericVector::push_back_new(T object) { +int GenericVector::push_back_new(const T& object) { int index = get_index(object); if (index >= 0) return index; @@ -815,7 +815,7 @@ int GenericVector::push_back_new(T object) { // Add an element in the array (front) template -int GenericVector::push_front(T object) { +int GenericVector::push_front(const T& object) { if (size_used_ == size_reserved_) double_the_size(); for (int i = size_used_; i > 0; --i) @@ -826,7 +826,7 @@ int GenericVector::push_front(T object) { } template -void GenericVector::operator+=(T t) { +void GenericVector::operator+=(const T& t) { push_back(t); } From 238c872753625d12411a3db8fd5f497a2f1bd565 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 6 Oct 2018 19:48:13 +0200 Subject: [PATCH 4/6] GENERIC_2D_ARRAY: Pass parameters by reference This fixes warnings from LGTM: This parameter of type FontClassInfo is 192 bytes - consider passing a pointer/reference instead. Signed-off-by: Stefan Weil --- src/ccstruct/matrix.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ccstruct/matrix.h b/src/ccstruct/matrix.h index 6feec223..7dc1bb15 100644 --- a/src/ccstruct/matrix.h +++ b/src/ccstruct/matrix.h @@ -365,7 +365,7 @@ class GENERIC_2D_ARRAY { } // Accumulates the element-wise sums of squares of src into *this. - void SumSquares(const GENERIC_2D_ARRAY& src, T decay_factor) { + void SumSquares(const GENERIC_2D_ARRAY& src, const T& decay_factor) { T update_factor = 1.0 - decay_factor; int size = num_elements(); for (int i = 0; i < size; ++i) { @@ -377,7 +377,7 @@ class GENERIC_2D_ARRAY { // Scales each element using the adam algorithm, ie array_[i] by // sqrt(sqsum[i] + epsilon)). void AdamUpdate(const GENERIC_2D_ARRAY& sum, - const GENERIC_2D_ARRAY& sqsum, T epsilon) { + const GENERIC_2D_ARRAY& sqsum, const T& epsilon) { int size = num_elements(); for (int i = 0; i < size; ++i) { array_[i] += sum.array_[i] / (sqrt(sqsum.array_[i]) + epsilon); From 18f7ab751e29f7f6d053fda489eb5bb771531f6b Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 6 Oct 2018 20:06:38 +0200 Subject: [PATCH 5/6] WERD_RES: Remove comparisons which are constant This fixes warnings from LGTM: Comparison is always false because id >= 0. Comparison is always true because mirrored >= 1. Comparison is always false because id >= 0. INVALID_UNICHAR_ID is -1, so the warnings are correct. Signed-off-by: Stefan Weil --- src/ccstruct/pageres.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ccstruct/pageres.h b/src/ccstruct/pageres.h index b405bf61..cb559fb1 100644 --- a/src/ccstruct/pageres.h +++ b/src/ccstruct/pageres.h @@ -363,10 +363,10 @@ class WERD_RES : public ELIST_LINK { blob_index >= best_choice->length()) return nullptr; UNICHAR_ID id = best_choice->unichar_id(blob_index); - if (id < 0 || id >= uch_set->size() || id == INVALID_UNICHAR_ID) + if (id < 0 || id >= uch_set->size()) return nullptr; UNICHAR_ID mirrored = uch_set->get_mirror(id); - if (in_rtl_context && mirrored > 0 && mirrored != INVALID_UNICHAR_ID) + if (in_rtl_context && mirrored > 0) id = mirrored; return uch_set->id_to_unichar_ext(id); } @@ -375,7 +375,7 @@ class WERD_RES : public ELIST_LINK { if (blob_index < 0 || blob_index >= raw_choice->length()) return nullptr; UNICHAR_ID id = raw_choice->unichar_id(blob_index); - if (id < 0 || id >= uch_set->size() || id == INVALID_UNICHAR_ID) + if (id < 0 || id >= uch_set->size()) return nullptr; return uch_set->id_to_unichar(id); } From 685abc91f3f2110b9ed1cc61f7f15f3b6d308bb8 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Sat, 6 Oct 2018 20:14:20 +0200 Subject: [PATCH 6/6] pgedit: Change some variables from global to local ones This fixes compiler warnings and a warning from LGTM: Poor global variable name 'pe'. Prefer longer, descriptive names [...] Signed-off-by: Stefan Weil --- src/ccmain/pgedit.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ccmain/pgedit.cpp b/src/ccmain/pgedit.cpp index ec3ee8e2..83cf18b0 100644 --- a/src/ccmain/pgedit.cpp +++ b/src/ccmain/pgedit.cpp @@ -100,15 +100,15 @@ enum ColorationMode { * */ -ScrollView* image_win; -ParamsEditor* pe; -bool stillRunning = false; +static ScrollView* image_win; +static ParamsEditor* pe; +static bool stillRunning = false; -ScrollView* bln_word_window = nullptr; // baseline norm words +static ScrollView* bln_word_window = nullptr; // baseline norm words -CMD_EVENTS mode = CHANGE_DISP_CMD_EVENT; // selected words op +static CMD_EVENTS mode = CHANGE_DISP_CMD_EVENT; // selected words op -bool recog_done = false; // recog_all_words was called +static bool recog_done = false; // recog_all_words was called // These variables should remain global, since they are only used for the // debug mode (in which only a single Tesseract thread/instance will exist).