Skip to content

Commit

Permalink
Fixed page offset bug
Browse files Browse the repository at this point in the history
Fixed a major bug that causes any kind of text interaction not to work for some books
  • Loading branch information
DavidLazarescu committed Oct 19, 2023
1 parent e57f069 commit 45d4ec0
Show file tree
Hide file tree
Showing 12 changed files with 166 additions and 6 deletions.
3 changes: 0 additions & 3 deletions src/adapters/controllers/book_controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ BookController::BookController(application::IBookService* bookService) :
QPointF left(rect.left(), rect.center().y());
QPointF right(rect.right(), rect.center().y());

left = utils::scalePointToCurrentZoom(left, 1, getZoom());
right = utils::scalePointToCurrentZoom(right, 1, getZoom());

emit selectText(pageNumber, left, right);
});

Expand Down
20 changes: 20 additions & 0 deletions src/adapters/controllers/page_controller.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "page_controller.hpp"
#include "fz_utils.hpp"
#include "mupdf/classes.h"
#include "mupdf/classes2.h"

using namespace application::core;

Expand All @@ -22,6 +23,16 @@ int PageController::getHeight()
return m_pageGenerator.getHeight() * m_matrix.d;
}

int PageController::getXOffset() const
{
return m_pageXOffset;
}

int PageController::getYOffset() const
{
return m_pageYOffset;
}

void PageController::setZoom(float zoom)
{
m_matrix.a = zoom;
Expand Down Expand Up @@ -50,6 +61,15 @@ QImage PageController::renderPage()
auto zoom = m_matrix.a;
m_pageImage = utils::qImageFromPixmap(m_pageGenerator.renderPage(zoom));

auto xOffset = m_pageGenerator.getPageXOffset();
auto yOffset = m_pageGenerator.getPageYOffset();
if(xOffset != m_pageXOffset || yOffset != m_pageYOffset)
{
m_pageXOffset = xOffset;
m_pageYOffset = yOffset;
emit pageOffsetsChanged(xOffset, yOffset);
}

m_pageImageOutdated = false;
return m_pageImage;
}
Expand Down
6 changes: 6 additions & 0 deletions src/adapters/controllers/page_controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ class PageController : public IPageController
int getWidth() override;
int getHeight() override;

int getXOffset() const override;
int getYOffset() const override;

void setZoom(float zoom) override;
float getZoom() override;

Expand All @@ -43,6 +46,9 @@ class PageController : public IPageController
application::core::PageGenerator m_pageGenerator;
mupdf::FzMatrix m_matrix;

int m_pageXOffset = 0;
int m_pageYOffset = 0;

// Image caching
bool m_pageImageOutdated = true;
QImage m_pageImage;
Expand Down
6 changes: 6 additions & 0 deletions src/adapters/interfaces/controllers/i_page_controller.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ class ADAPTERS_EXPORT IPageController : public QObject
virtual int getWidth() = 0;
virtual int getHeight() = 0;

virtual int getXOffset() const = 0;
virtual int getYOffset() const = 0;

virtual void setZoom(float zoom) = 0;
virtual float getZoom() = 0;

Expand All @@ -42,6 +45,9 @@ class ADAPTERS_EXPORT IPageController : public QObject
QPointF point) = 0;
virtual QString getTextFromSelection(const QPointF& start,
const QPointF& end) = 0;

signals:
void pageOffsetsChanged(int xOffset, int yOffset);
};

} // namespace adapters
54 changes: 54 additions & 0 deletions src/application/core/page_generator.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
#include "page_generator.hpp"
#include <math.h>
#include <cmath>
#include "fz_utils.hpp"
#include "mupdf/classes.h"
#include "mupdf/classes2.h"
#include "mupdf/fitz/geometry.h"

namespace application::core
{
Expand Down Expand Up @@ -79,6 +81,39 @@ mupdf::FzPixmap PageGenerator::renderPage(float zoom)
auto pixmap = getEmptyPixmap(matrix);
auto drawDevice = mupdf::fz_new_draw_device(mupdf::FzMatrix(), pixmap);

// Determine the page offset the first time we render the page
if(m_pageXOffset == 0 && m_pageYOffset == 0)
{
int restoredXOffset = round(pixmap.x() / zoom);
int restoredYOffset = round(pixmap.y() / zoom);
if(restoredXOffset != 0 && restoredYOffset != 0)
{
setPageOffsets(restoredXOffset, restoredYOffset);

std::vector<fz_rect> newSymbolBounds;
for(auto& rect : m_symbolBounds)
{
auto newRect = fz_make_rect(
rect.x0 - m_pageXOffset, rect.y0 - m_pageYOffset,
rect.x1 - m_pageXOffset, rect.y1 - m_pageYOffset);

newSymbolBounds.emplace_back(newRect);
}
m_symbolBounds = newSymbolBounds;


for(auto& link : m_links)
{
auto newLinkRect =
mupdf::fz_make_rect(link.rect().x0 - m_pageXOffset,
link.rect().y0 - m_pageYOffset,
link.rect().x1 - m_pageXOffset,
link.rect().y1 - m_pageYOffset);
link.fz_set_link_rect(newLinkRect);
}
}
}

mupdf::FzCookie cookie;
mupdf::FzRect rect = mupdf::FzRect::Fixed_INFINITE;
m_displayList.fz_run_display_list(drawDevice, matrix, rect, cookie);
Expand All @@ -103,6 +138,15 @@ mupdf::FzPixmap PageGenerator::getEmptyPixmap(
return pixmap;
}

void PageGenerator::setPageOffsets(int xOffset, int yOffset)
{
m_pageXOffset = xOffset;
m_pageYOffset = yOffset;

m_textSelector.setPageXOffset(xOffset);
m_textSelector.setPageYOffset(yOffset);
}

void imageCleanupHandler(void* data)
{
unsigned char* samples = static_cast<unsigned char*>(data);
Expand All @@ -123,6 +167,16 @@ int PageGenerator::getHeight() const
return (bbox.y1 - bbox.y0);
}

int PageGenerator::getPageXOffset() const
{
return m_pageXOffset;
}

int PageGenerator::getPageYOffset() const
{
return m_pageYOffset;
}

QList<mupdf::FzQuad>& PageGenerator::getBufferedSelectionRects()
{
return m_bufferedSelectionRects;
Expand Down
6 changes: 6 additions & 0 deletions src/application/core/page_generator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ class APPLICATION_EXPORT PageGenerator
int getWidth() const;
int getHeight() const;

int getPageXOffset() const;
int getPageYOffset() const;

mupdf::FzPixmap renderPage(float zoom);
void setInvertColor(bool newInvertColor);

Expand All @@ -46,6 +49,7 @@ class APPLICATION_EXPORT PageGenerator
void setupSymbolBounds();
void setupLinks();
mupdf::FzPixmap getEmptyPixmap(const mupdf::FzMatrix& matrix) const;
void setPageOffsets(int xOffset, int yOffset);

const mupdf::FzDocument* m_document;
std::unique_ptr<mupdf::FzPage> m_page;
Expand All @@ -56,6 +60,8 @@ class APPLICATION_EXPORT PageGenerator
QList<mupdf::FzLink> m_links;
std::vector<fz_rect> m_symbolBounds;
bool m_invertColor = false;
int m_pageXOffset = 0;
int m_pageYOffset = 0;
};

} // namespace application::core
1 change: 1 addition & 0 deletions src/application/core/utils/book_searcher.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "book_searcher.hpp"
#include <QDebug>
#include "mupdf/fitz/geometry.h"
#include "qnumeric.h"

namespace application::core::utils
Expand Down
41 changes: 41 additions & 0 deletions src/application/core/utils/text_selector.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "text_selector.hpp"
#include <mupdf/functions.h>
#include <QDebug>
#include "mupdf/fitz/geometry.h"

namespace application::core::utils
{
Expand All @@ -13,6 +15,10 @@ void TextSelector::generateSelectionRects(QList<mupdf::FzQuad>& container,
mupdf::FzPoint start,
mupdf::FzPoint end)
{
start = mupdf::ll_fz_make_point(start.x + m_pageXOffset,
start.y + m_pageYOffset);
end = mupdf::ll_fz_make_point(end.x + m_pageXOffset, end.y + m_pageYOffset);

const int max = 1000;
fz_quad hits[max];
int n = mupdf::ll_fz_highlight_selection(
Expand All @@ -21,17 +27,30 @@ void TextSelector::generateSelectionRects(QList<mupdf::FzQuad>& container,
for(int i = 0; i < n; ++i)
{
fz_quad hit = hits[i];
// apply offsets to create new fz_quad
hit = fz_make_quad(hit.ul.x - m_pageXOffset, hit.ul.y - m_pageYOffset,
hit.ur.x - m_pageXOffset, hit.ur.y - m_pageYOffset,
hit.ll.x - m_pageXOffset, hit.ll.y - m_pageYOffset,
hit.lr.x - m_pageXOffset, hit.lr.y - m_pageYOffset);
container.append(hit);
}
}

FzPointPair TextSelector::getPositionsForWordSelection(mupdf::FzPoint start,
mupdf::FzPoint end)
{
start = mupdf::ll_fz_make_point(start.x + m_pageXOffset,
start.y + m_pageYOffset);
end = mupdf::ll_fz_make_point(end.x + m_pageXOffset, end.y + m_pageYOffset);

// This modifies the fzBegin and fzEnd.
mupdf::ll_fz_snap_selection(m_textPage->m_internal, start.internal(),
end.internal(), FZ_SELECT_WORDS);

start = mupdf::ll_fz_make_point(start.x - m_pageXOffset,
start.y - m_pageYOffset);
end = mupdf::ll_fz_make_point(end.x - m_pageXOffset, end.y - m_pageYOffset);

return { start, end };
}

Expand All @@ -40,18 +59,40 @@ FzPointPair TextSelector::getPositionsForLineSelection(mupdf::FzPoint point)
mupdf::FzPoint start(point.x, point.y);
mupdf::FzPoint end = start;

start = mupdf::ll_fz_make_point(start.x + m_pageXOffset,
start.y + m_pageYOffset);
end = mupdf::ll_fz_make_point(end.x + m_pageXOffset, end.y + m_pageYOffset);

// This modifies the fzPoint
mupdf::ll_fz_snap_selection(m_textPage->m_internal, start.internal(),
end.internal(), FZ_SELECT_LINES);

start = mupdf::ll_fz_make_point(start.x - m_pageXOffset,
start.y - m_pageYOffset);
end = mupdf::ll_fz_make_point(end.x - m_pageXOffset, end.y - m_pageYOffset);

return { start, end };
}

std::string TextSelector::getTextFromSelection(mupdf::FzPoint start,
mupdf::FzPoint end)
{
start = mupdf::ll_fz_make_point(start.x + m_pageXOffset,
start.y + m_pageYOffset);
end = mupdf::ll_fz_make_point(end.x + m_pageXOffset, end.y + m_pageYOffset);

auto text = m_textPage->fz_copy_selection(start, end, 1);
return text;
}

void TextSelector::setPageXOffset(int newXOffset)
{
m_pageXOffset = newXOffset;
}

void TextSelector::setPageYOffset(int newYOffset)
{
m_pageYOffset = newYOffset;
}

} // namespace application::core::utils
5 changes: 5 additions & 0 deletions src/application/core/utils/text_selector.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@ class TextSelector
FzPointPair getPositionsForLineSelection(mupdf::FzPoint point);
std::string getTextFromSelection(mupdf::FzPoint start, mupdf::FzPoint end);

void setPageXOffset(int newXOffset);
void setPageYOffset(int newYOffset);

private:
mupdf::FzStextPage* m_textPage;
int m_pageXOffset = 0;
int m_pageYOffset = 0;
};

} // namespace application::core::utils
25 changes: 23 additions & 2 deletions src/presentation/modules/CppElements/page_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,24 @@ void PageView::setBookController(BookController* newBookController)
if(pageNumber != m_pageNumber)
return;

m_selectionStart = QPointF(left.x(), left.y());
m_selectionEnd = QPointF(right.x(), right.y());
// The offsets are only valid once the page was rendered, make
// sure that it is always rendered first.
m_pageController->renderPage();

auto xOffset = m_pageController->getXOffset();
auto yOffset = m_pageController->getYOffset();
left = QPoint(left.x() - xOffset, left.y() - yOffset);
right = QPoint(right.x() - xOffset, right.y() - yOffset);

// The points received from this signal are relative to a zoom
// of 1, but all of the methods in this class handle points as
// if they have the currentZoom applied, so we need to scale it.
auto zoom = m_pageController->getZoom();
left = utils::scalePointToCurrentZoom(left, 1, zoom);
right = utils::scalePointToCurrentZoom(right, 1, zoom);

m_selectionStart = left;
m_selectionEnd = right;

createSelection();
});
Expand Down Expand Up @@ -600,4 +616,9 @@ void PageView::setColorInverted(bool newColorInverted)
m_firstTimeColorInverted = false;
}

float PageView::getYOffset() const
{
return m_pageController->getYOffset();
}

} // namespace cpp_elements
3 changes: 3 additions & 0 deletions src/presentation/modules/CppElements/page_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@ class PRESENTATION_EXPORT PageView : public QQuickItem
int implicitHeight READ getImplicitHeight NOTIFY implicitHeightChanged)
Q_PROPERTY(int pageNumber READ getPageNumber WRITE setPageNumber CONSTANT)
Q_PROPERTY(bool colorInverted WRITE setColorInverted)
Q_PROPERTY(float yOffset READ getYOffset CONSTANT)

public:
PageView();

int getImplicitWidth() const;
int getImplicitHeight() const;

float getYOffset() const;

bool disableHoverEvents() const;
void setDisableHoverEvents(bool newDisableHoverEvents);

Expand Down
2 changes: 1 addition & 1 deletion src/presentation/readingPage/MDocumentView.qml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Pane {
target: BookController

function onGoToPosition(pageNumber, y) {
root.setPage(pageNumber, y)
root.setPage(pageNumber, y - pageView.currentItem.yOffset)
}

function onTextSelectionFinished(centerX, topY) {
Expand Down

0 comments on commit 45d4ec0

Please sign in to comment.