From 7766e70828782e2b4878243c0b391897c5bf52c3 Mon Sep 17 00:00:00 2001 From: JJones780 <2329541+JJones780@users.noreply.github.com> Date: Sat, 24 Aug 2019 04:07:00 -0600 Subject: [PATCH 1/3] compare algo's --- src/gui/Layout.cpp | 92 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-) diff --git a/src/gui/Layout.cpp b/src/gui/Layout.cpp index f4e1888a..21d5b468 100644 --- a/src/gui/Layout.cpp +++ b/src/gui/Layout.cpp @@ -9,6 +9,12 @@ #include #include #include + +#include +typedef std::chrono::high_resolution_clock Clock; +#include + + /** * Padding outside the pages, including shadow */ @@ -25,6 +31,12 @@ constexpr size_t const XOURNAL_ROOM_FOR_SHADOW = 3; constexpr size_t const XOURNAL_PADDING_BETWEEN = 15; +double searchtime1 = 0; +double searchtime2 = 0; +double searchtime3 = 0; +double plotindex = 0; + + Layout::Layout(XournalView* view, ScrollHandling* scrollHandling) : view(view), scrollHandling(scrollHandling) @@ -349,6 +361,7 @@ XojPageView* Layout::getViewAt(int x, int y) // auto cit = std::lower_bound( this->sizeCol.begin(), this->sizeCol.end(), x); // int cb = cit - this->sizeCol.begin(); + auto t0 = Clock::now(); auto fast_linear_search = [](std::vector const& container, size_t start, size_t value) { auto row_i = std::next(begin(container), start); @@ -367,6 +380,83 @@ XojPageView* Layout::getViewAt(int x, int y) auto const& col_i = fast_linear_search(this->widthCols, this->lastGetViewAtCol, x); + auto t1 = Clock::now(); + + /* Linear Up or Down Search from last position: */ + /* This is over 7x faster than the above code */ + + // Rows: + int r = std::max(0, static_cast(this->lastGetViewAtRow) - 1); + if (r > 0 && y <= this->heightRows[r]) //search lower + { + for (r--; r >= 0; r--) + { + if (y > this->heightRows[r]) + { + break; // past region - it's back up one + } + } + r++; + } + else //search higher + { + for (; r < this->heightRows.size(); r++) + { + if (y <= this->heightRows[r]) break; // found region + } + } + + + //Now for columns: + int c = std::max(0, static_cast(this->lastGetViewAtCol) - 1); + if (c > 0 && x <= this->widthCols[c]) //search lower + { + for (c--; c >= 0; c--) + { + if (x > this->widthCols[c]) + { + break; // past region + } + } + c++; + } + else + { + for (; c < this->widthCols.size(); c++) + { + if (x <= this->widthCols[c]) break; // found region + } + } + + + auto t2 = Clock::now(); + + //Binary Search ... too much overhead makes this a slower option in our use case. + /* This is over 2x faster than the first code block but 3x slower than the second code block*/ + auto rit = std::lower_bound(this->heightRows.begin(), this->heightRows.end(), y); + int rb = rit - this->heightRows.begin(); //get index + auto cit = std::lower_bound(this->widthCols.begin(), this->widthCols.end(), x); + int cb = cit - this->widthCols.begin(); + + auto t3 = Clock::now(); + + searchtime1 += std::chrono::duration_cast(t1 - t0).count(); + searchtime2 += std::chrono::duration_cast(t2 - t1).count(); + searchtime3 += std::chrono::duration_cast(t3 - t2).count(); + + + if (plotindex == 0) + { + std::clog << " index \tagreeR \t agreeC \t new \told \t linear" << std::endl; + } + //remove these along with timing etc when done testing: + int r0 = std::distance(this->heightRows.cbegin(), row_i); + int c0 = std::distance(this->widthCols.cbegin(), col_i); + + std::clog << plotindex++ << '\t' << r0 << '=' << r << '=' << rb << '\t' << c0 << '=' << c << '=' << cb << '\t' + << searchtime1 << '\t' << searchtime2 << '\t' << searchtime3 << '\t' << std::endl; + + if (col_i != end(this->widthCols) && row_i != end(this->heightRows)) { //Todo c++14+ cbegin(...); @@ -374,11 +464,11 @@ XojPageView* Layout::getViewAt(int x, int y) this->lastGetViewAtCol = std::distance(this->widthCols.cbegin(), col_i); auto optionalPage = this->mapper.at({this->lastGetViewAtCol, this->lastGetViewAtRow}); + if (optionalPage && this->view->viewPages[*optionalPage]->containsPoint(x, y, false)) { return this->view->viewPages[*optionalPage]; } - } return nullptr; From d561d14186ff568061ce55757707f44d16877a59 Mon Sep 17 00:00:00 2001 From: JJones780 <2329541+JJones780@users.noreply.github.com> Date: Mon, 26 Aug 2019 04:40:36 -0600 Subject: [PATCH 2/3] use binary search algorithm --- src/gui/Layout.cpp | 125 +++------------------------------------------ src/gui/Layout.h | 6 --- 2 files changed, 6 insertions(+), 125 deletions(-) diff --git a/src/gui/Layout.cpp b/src/gui/Layout.cpp index 21d5b468..93c329be 100644 --- a/src/gui/Layout.cpp +++ b/src/gui/Layout.cpp @@ -9,12 +9,6 @@ #include #include #include - -#include -typedef std::chrono::high_resolution_clock Clock; -#include - - /** * Padding outside the pages, including shadow */ @@ -31,12 +25,6 @@ constexpr size_t const XOURNAL_ROOM_FOR_SHADOW = 3; constexpr size_t const XOURNAL_PADDING_BETWEEN = 15; -double searchtime1 = 0; -double searchtime2 = 0; -double searchtime3 = 0; -double plotindex = 0; - - Layout::Layout(XournalView* view, ScrollHandling* scrollHandling) : view(view), scrollHandling(scrollHandling) @@ -210,8 +198,6 @@ void Layout::layoutPages(int width, int height) auto const rows = this->heightRows.size(); auto const columns = this->widthCols.size(); - this->lastGetViewAtRow = rows / 2; //reset to middle - this->lastGetViewAtCol = columns / 2; //add space around the entire page area to accomodate older Wacom tablets with limited sense area. int64_t const v_padding = @@ -352,117 +338,17 @@ XojPageView* Layout::getViewAt(int x, int y) XOJ_CHECK_TYPE(Layout); -// No need to check page cache as the Linear search below starts at cached position. -// Keep Binary search handy to check against. -// -// //Binary Search ... too much overhead makes this a slower option in our use case. -// auto rit = std::lower_bound( this->sizeRow.begin(), this->sizeRow.end(), y); -// int rb = rit - this->sizeRow.begin(); //get index -// auto cit = std::lower_bound( this->sizeCol.begin(), this->sizeCol.end(), x); -// int cb = cit - this->sizeCol.begin(); - - auto t0 = Clock::now(); - - auto fast_linear_search = [](std::vector const& container, size_t start, size_t value) { - auto row_i = std::next(begin(container), start); - if (row_i != begin(container) && value < *row_i) - { - //Todo: replace with c++14 - //Todo: rend(this->heightRows) - //Todo: std::make_reverse_iterator(c_i) - auto rbeg_i = std::reverse_iterator(row_i); - return std::find_if(rbeg_i, container.rend(), [value](int item) { return value > item; }).base(); - } - return std::find_if(row_i, end(container), [value](int item) { return value <= item; }); - }; - - auto const& row_i = fast_linear_search(this->heightRows, this->lastGetViewAtRow, y); - auto const& col_i = fast_linear_search(this->widthCols, this->lastGetViewAtCol, x); - - - auto t1 = Clock::now(); - - /* Linear Up or Down Search from last position: */ - /* This is over 7x faster than the above code */ - - // Rows: - int r = std::max(0, static_cast(this->lastGetViewAtRow) - 1); - if (r > 0 && y <= this->heightRows[r]) //search lower - { - for (r--; r >= 0; r--) - { - if (y > this->heightRows[r]) - { - break; // past region - it's back up one - } - } - r++; - } - else //search higher - { - for (; r < this->heightRows.size(); r++) - { - if (y <= this->heightRows[r]) break; // found region - } - } - - //Now for columns: - int c = std::max(0, static_cast(this->lastGetViewAtCol) - 1); - if (c > 0 && x <= this->widthCols[c]) //search lower - { - for (c--; c >= 0; c--) - { - if (x > this->widthCols[c]) - { - break; // past region - } - } - c++; - } - else - { - for (; c < this->widthCols.size(); c++) - { - if (x <= this->widthCols[c]) break; // found region - } - } - - - auto t2 = Clock::now(); - - //Binary Search ... too much overhead makes this a slower option in our use case. - /* This is over 2x faster than the first code block but 3x slower than the second code block*/ + //Binary Search: auto rit = std::lower_bound(this->heightRows.begin(), this->heightRows.end(), y); - int rb = rit - this->heightRows.begin(); //get index + int foundRow = rit - this->heightRows.begin(); //get index auto cit = std::lower_bound(this->widthCols.begin(), this->widthCols.end(), x); - int cb = cit - this->widthCols.begin(); + int foundCol = cit - this->widthCols.begin(); - auto t3 = Clock::now(); - searchtime1 += std::chrono::duration_cast(t1 - t0).count(); - searchtime2 += std::chrono::duration_cast(t2 - t1).count(); - searchtime3 += std::chrono::duration_cast(t3 - t2).count(); - - - if (plotindex == 0) + if (foundCol < this->widthCols.size() && foundRow < this->heightRows.size()) { - std::clog << " index \tagreeR \t agreeC \t new \told \t linear" << std::endl; - } - //remove these along with timing etc when done testing: - int r0 = std::distance(this->heightRows.cbegin(), row_i); - int c0 = std::distance(this->widthCols.cbegin(), col_i); - - std::clog << plotindex++ << '\t' << r0 << '=' << r << '=' << rb << '\t' << c0 << '=' << c << '=' << cb << '\t' - << searchtime1 << '\t' << searchtime2 << '\t' << searchtime3 << '\t' << std::endl; - - - if (col_i != end(this->widthCols) && row_i != end(this->heightRows)) - { - //Todo c++14+ cbegin(...); - this->lastGetViewAtRow = std::distance(this->heightRows.cbegin(), row_i); - this->lastGetViewAtCol = std::distance(this->widthCols.cbegin(), col_i); - auto optionalPage = this->mapper.at({this->lastGetViewAtCol, this->lastGetViewAtRow}); + auto optionalPage = this->mapper.at({foundCol, foundRow}); if (optionalPage && this->view->viewPages[*optionalPage]->containsPoint(x, y, false)) @@ -473,6 +359,7 @@ XojPageView* Layout::getViewAt(int x, int y) return nullptr; } + // Todo replace with boost::optional Layout::getIndexAtGridMap(size_t row, size_t col) // or std::optional Layout::getIndexAtGridMap(size_t row, size_t col) LayoutMapper::optional_size_t Layout::getIndexAtGridMap(size_t row, size_t col) diff --git a/src/gui/Layout.h b/src/gui/Layout.h index 347299f8..8a768ffe 100644 --- a/src/gui/Layout.h +++ b/src/gui/Layout.h @@ -137,12 +137,6 @@ private: size_t minWidth = 0; size_t minHeight = 0; - /** - * cache the last GetViewAt() row and column. - */ - size_t lastGetViewAtRow = 0; - size_t lastGetViewAtCol = 0; - /** * layoutPages invalidates the precalculation of recalculate * this bool prevents that layotPages can be called without a previously call to recalculate From 87573e33a80e3557ac73b8e4a9b1216a8f2d5ce7 Mon Sep 17 00:00:00 2001 From: JJones780 <2329541+JJones780@users.noreply.github.com> Date: Thu, 29 Aug 2019 04:15:53 -0600 Subject: [PATCH 3/3] review changes --- src/gui/Layout.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/gui/Layout.cpp b/src/gui/Layout.cpp index 93c329be..659d3cfb 100644 --- a/src/gui/Layout.cpp +++ b/src/gui/Layout.cpp @@ -341,20 +341,15 @@ XojPageView* Layout::getViewAt(int x, int y) //Binary Search: auto rit = std::lower_bound(this->heightRows.begin(), this->heightRows.end(), y); - int foundRow = rit - this->heightRows.begin(); //get index + int const foundRow = std::distance(this->heightRows.begin(), rit); auto cit = std::lower_bound(this->widthCols.begin(), this->widthCols.end(), x); - int foundCol = cit - this->widthCols.begin(); + int const foundCol = std::distance(this->widthCols.begin(), cit); + auto optionalPage = this->mapper.at({foundCol, foundRow}); - if (foundCol < this->widthCols.size() && foundRow < this->heightRows.size()) + if (optionalPage && this->view->viewPages[*optionalPage]->containsPoint(x, y, false)) { - auto optionalPage = this->mapper.at({foundCol, foundRow}); - - - if (optionalPage && this->view->viewPages[*optionalPage]->containsPoint(x, y, false)) - { - return this->view->viewPages[*optionalPage]; - } + return this->view->viewPages[*optionalPage]; } return nullptr;