From af3b373d23ca1517d084f68485a1de26b9df4d76 Mon Sep 17 00:00:00 2001 From: Thomas Moerschell <144177788+tmoerschell@users.noreply.github.com> Date: Fri, 27 Oct 2023 17:48:01 +0200 Subject: [PATCH 1/2] Merge PR #5270 (fix page layout) --- src/core/gui/Layout.cpp | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/core/gui/Layout.cpp b/src/core/gui/Layout.cpp index 82e82144d668..2d3efa3c0a2c 100644 --- a/src/core/gui/Layout.cpp +++ b/src/core/gui/Layout.cpp @@ -271,10 +271,10 @@ void Layout::layoutPages(int width, int height) { auto& v = this->view->viewPages[*optionalPage]; v->setMappedRowCol(strict_cast(r), strict_cast(c)); // store row and column for e.g. proper arrow key navigation - auto vDisplayWidth = v->getDisplayWidthDouble(); + const auto vDisplayWidth = v->getDisplayWidthDouble(); + const auto vDisplayHeight = v->getDisplayHeightDouble(); { auto paddingLeft = 0.0; - auto paddingRight = 0.0; auto columnPadding = this->pc.widthCols[c] - vDisplayWidth; if (isPairedPages && len > 1) { @@ -282,26 +282,21 @@ void Layout::layoutPages(int width, int height) { if (c % 2 == 0) { // align right paddingLeft = XOURNAL_PADDING_BETWEEN - XOURNAL_ROOM_FOR_SHADOW + columnPadding; - paddingRight = XOURNAL_ROOM_FOR_SHADOW; } else { // align left paddingLeft = XOURNAL_ROOM_FOR_SHADOW; - paddingRight = XOURNAL_PADDING_BETWEEN - XOURNAL_ROOM_FOR_SHADOW + columnPadding; } } else { // not paired page mode - center paddingLeft = XOURNAL_PADDING_BETWEEN / 2.0 + columnPadding / 2.0; // center justify - paddingRight = XOURNAL_PADDING_BETWEEN - paddingLeft + columnPadding / 2.0; } - x += paddingLeft; + // center page vertically + const auto paddingTop = (this->pc.heightRows[r] - vDisplayHeight) / 2.0; - v->setX(floor_cast(x)); // set the page position - v->setY(floor_cast(y)); - - x += vDisplayWidth + paddingRight; + v->setX(floor_cast(x + paddingLeft)); // set the page position + v->setY(floor_cast(y + paddingTop)); } - } else { - x += this->pc.widthCols[c] + XOURNAL_PADDING_BETWEEN; } + x += this->pc.widthCols[c] + XOURNAL_PADDING_BETWEEN; } x = borderX; y += this->pc.heightRows[r] + XOURNAL_PADDING_BETWEEN; From 017450c88a17966b7b9464e6f0ecd2e7e15fcffb Mon Sep 17 00:00:00 2001 From: Thomas Moerschell <144177788+tmoerschell@users.noreply.github.com> Date: Fri, 27 Oct 2023 18:23:27 +0200 Subject: [PATCH 2/2] Fix incorrect zoom centers when not on active page --- src/core/control/zoom/ZoomControl.cpp | 14 ++-- src/core/gui/Layout.cpp | 101 +++++++++++++++++++------- src/core/gui/Layout.h | 27 ++++--- 3 files changed, 99 insertions(+), 43 deletions(-) diff --git a/src/core/control/zoom/ZoomControl.cpp b/src/core/control/zoom/ZoomControl.cpp index 9ead8defe1a6..597c23d18f9a 100644 --- a/src/core/control/zoom/ZoomControl.cpp +++ b/src/core/control/zoom/ZoomControl.cpp @@ -146,24 +146,22 @@ void ZoomControl::startZoomSequence(utl::Point zoomCenter) { this->zoomWidgetPos = zoomCenter; // widget space coordinates of the zoomCenter! this->zoomSequenceStart = this->zoom; - // * set unscaledPixels padding value - size_t currentPageIdx = this->view->getCurrentPage(); - // To get the layout, we need to call view->getWidget(), which isn't const. // As such, we get the view and determine `unscaledPixels` here, rather than // in `getScrollPositionAfterZoom`. GtkWidget* widget = view->getWidget(); Layout* layout = gtk_xournal_get_layout(widget); - // Not everything changes size as we zoom in/out. The padding, for example, - // remains constant! (changed when page changes, but the error stays small enough) - this->unscaledPixels = {static_cast(layout->getPaddingLeftOfPage(currentPageIdx)), - static_cast(layout->getPaddingAbovePage(currentPageIdx))}; - // * set initial scrollPosition value auto const& rect = getVisibleRect(); auto const& view_pos = utl::Point{rect.x, rect.y}; + // * set unscaledPixels padding value + // Not everything changes size as we zoom in/out. The padding, for example, + // remains constant! + this->unscaledPixels = {layout->getPaddingLeftOf(zoomCenter.x + view_pos.x), + layout->getPaddingAbove(zoomCenter.y + view_pos.y)}; + // Use this->zoomWidgetPos to zoom into a location other than the top-left (e.g. where // the user pinched). this->scrollPosition = (view_pos + this->zoomWidgetPos - this->unscaledPixels) / this->zoom; diff --git a/src/core/gui/Layout.cpp b/src/core/gui/Layout.cpp index 2d3efa3c0a2c..c9ace19c5978 100644 --- a/src/core/gui/Layout.cpp +++ b/src/core/gui/Layout.cpp @@ -251,12 +251,12 @@ void Layout::layoutPages(int width, int height) { auto const centeringYBorder = (height - as_signed(pc.minHeight)) / 2; using SBig = decltype(as_signed(h_padding * centeringXBorder)); - auto const borderX = static_cast(std::max(h_padding, centeringXBorder)); - auto const borderY = static_cast(std::max(v_padding, centeringYBorder)); + this->borderX = static_cast(std::max(h_padding, centeringXBorder)); + this->borderY = static_cast(std::max(v_padding, centeringYBorder)); // initialize here and x again in loop below. - auto x = borderX; - auto y = borderY; + auto x = this->borderX; + auto y = this->borderY; // Iterate over ALL possible rows and columns. @@ -298,7 +298,7 @@ void Layout::layoutPages(int width, int height) { } x += this->pc.widthCols[c] + XOURNAL_PADDING_BETWEEN; } - x = borderX; + x = this->borderX; y += this->pc.heightRows[r] + XOURNAL_PADDING_BETWEEN; } @@ -307,13 +307,13 @@ void Layout::layoutPages(int width, int height) { // accumulated - absolute pixel location for use by getViewAt() and updateVisibility() - auto totalWidth = borderX; + auto totalWidth = this->borderX; std::transform( begin(this->pc.widthCols), end(this->pc.widthCols), begin(this->colXStart), [&totalWidth](auto&& widthCol) { return strict_cast>(totalWidth += widthCol + XOURNAL_PADDING_BETWEEN); }); - auto totalHeight = borderY; + auto totalHeight = this->borderY; std::transform(begin(this->pc.heightRows), end(this->pc.heightRows), begin(this->rowYStart), [&totalHeight](auto&& heightRow) { return strict_cast>( @@ -322,53 +322,104 @@ void Layout::layoutPages(int width, int height) { } -auto Layout::getPaddingAbovePage(size_t pageIndex) const -> int { +auto Layout::getPaddingAbove(double y) const -> double { + auto row_it = std::lower_bound(this->rowYStart.begin(), this->rowYStart.end(), static_cast(std::floor(y))); + auto const row = + std::min(static_cast(std::distance(this->rowYStart.begin(), row_it)), this->rowYStart.size() - 1); + const Settings* settings = this->view->getControl()->getSettings(); // User-configured padding above all pages. - auto paddingAbove = XOURNAL_PADDING; + double paddingAbove = XOURNAL_PADDING; if (settings->getUnlimitedScrolling()) { - paddingAbove += static_cast(gtk_adjustment_get_page_size(scrollHandling->getVertical())); + paddingAbove += gtk_adjustment_get_page_size(scrollHandling->getVertical()); } else if (settings->getAddVerticalSpace()) { paddingAbove += settings->getAddVerticalSpaceAmountAbove(); } - // (x, y) coordinate pair gives grid indicies. This handles paired pages - // and different page layouts for us. - auto pageYLocation = this->mapper.at(pageIndex).row; - return strict_cast(as_signed(pageYLocation) * XOURNAL_PADDING_BETWEEN + as_signed(paddingAbove)); + // Padding between pages + paddingAbove += static_cast(row * XOURNAL_PADDING_BETWEEN); + + // Additional padding when the position is between two pages + const double distanceToNextRow = rowYStart[row] - y; + if (distanceToNextRow < XOURNAL_PADDING_BETWEEN) { + paddingAbove += XOURNAL_PADDING_BETWEEN - distanceToNextRow; + } + + return paddingAbove; } -auto Layout::getPaddingLeftOfPage(size_t pageIndex) const -> int { +auto Layout::getPaddingLeftOf(double x) const -> double { + auto columnt_it = std::lower_bound(this->colXStart.begin(), this->colXStart.end(), static_cast(std::floor(x))); + auto const column = std::min(static_cast(std::distance(this->colXStart.begin(), columnt_it)), + this->colXStart.size() - 1); + bool isPairedPages = this->mapper.isPairedPages(); const Settings* settings = this->view->getControl()->getSettings(); - auto paddingBefore = XOURNAL_PADDING; + // User-configured padding left of all pages. + double paddingBefore = XOURNAL_PADDING; if (settings->getUnlimitedScrolling()) { - paddingBefore += static_cast(gtk_adjustment_get_page_size(scrollHandling->getHorizontal())); + paddingBefore += gtk_adjustment_get_page_size(scrollHandling->getHorizontal()); } else if (settings->getAddHorizontalSpace()) { paddingBefore += settings->getAddHorizontalSpaceAmountLeft(); } - auto const pageXLocation = as_signed(this->mapper.at(pageIndex).col); + double distancePastColumnStart; + if (column > 0) { + distancePastColumnStart = x - colXStart[column - 1]; + } else { + distancePastColumnStart = x - borderX; + } // No page pairing or we haven't rendered enough pages in the row for // page pairing to have an effect, if (!isPairedPages) { - return strict_cast(pageXLocation * XOURNAL_PADDING_BETWEEN + XOURNAL_PADDING_BETWEEN / 2 + - as_signed(paddingBefore)); + // Padding between pages + paddingBefore += static_cast(column * XOURNAL_PADDING_BETWEEN); + + // There is some padding before the page + paddingBefore += std::min(distancePastColumnStart, XOURNAL_PADDING_BETWEEN / 2.0); + + // And we may have to add some padding after the actual page + double distancePastPageEnd = x - (colXStart[column] - XOURNAL_PADDING_BETWEEN / 2.0); + if (distancePastPageEnd > 0) { + paddingBefore += distancePastPageEnd; + } + + return paddingBefore; } else { - auto columnPadding = - XOURNAL_PADDING_BETWEEN + strict_cast(pageXLocation / 2) * (XOURNAL_PADDING_BETWEEN * 2); - if (pageXLocation % 2 == 0) { - return strict_cast(columnPadding - XOURNAL_ROOM_FOR_SHADOW + paddingBefore); + // Padding between pages + paddingBefore += strict_cast(column / 2) * (XOURNAL_PADDING_BETWEEN * 2); + + if (column % 2 == 0) { + // There is some padding before the page + paddingBefore += std::min(distancePastColumnStart, + static_cast(XOURNAL_PADDING_BETWEEN - XOURNAL_ROOM_FOR_SHADOW)); + // And we may have to add some padding after the actual page + double distancePastPageEnd = x - (colXStart[column] - XOURNAL_ROOM_FOR_SHADOW); + if (distancePastPageEnd > 0) { + paddingBefore += distancePastPageEnd; + } + + return paddingBefore; } else { - return strict_cast(columnPadding + XOURNAL_ROOM_FOR_SHADOW + paddingBefore); + // There is some padding before the page + paddingBefore += std::min(distancePastColumnStart, + static_cast(XOURNAL_PADDING_BETWEEN + XOURNAL_ROOM_FOR_SHADOW)); + // And we may have to add some padding after the actual page + double distancePastPageEnd = x - (colXStart[column] - XOURNAL_PADDING_BETWEEN + XOURNAL_ROOM_FOR_SHADOW); + if (distancePastPageEnd > 0) { + paddingBefore += distancePastPageEnd; + } + + return paddingBefore; } } } + void Layout::setLayoutSize(int width, int height) { this->scrollHandling->setLayoutSize(width, height); } void Layout::scrollRelative(double x, double y) { diff --git a/src/core/gui/Layout.h b/src/core/gui/Layout.h index 9ed085725885..87b1e3ba17b1 100644 --- a/src/core/gui/Layout.h +++ b/src/core/gui/Layout.h @@ -120,27 +120,32 @@ class Layout final { std::optional getPageIndexAtGridMap(size_t row, size_t col); /** - * @brief Get the total padding above the page at the given index. + * @brief Get the total padding above the given position. * * The size of this padding does not scale with pages as the user zooms in and out. * As such, it is often necessary to get _just_ this padding. + * This does not include padding added to center the page on the screen. * - * @param pageIndex is the index of the XojPageView as returned by [getIndexAtGridMap] - * @return the sum of the padding between pages above the page with the given index - * and any padding the user added vertically above all pages (i.e. in settings). + * @param y the vertical position in relative coordinates, up to where the padding + * should be calculated + * @return the sum of the padding between pages above the given position, plus any + * padding the user added vertically above all pages (from settings) */ - int getPaddingAbovePage(size_t pageIndex) const; + double getPaddingAbove(double y) const; /** * @brief Get the static padding added to the left of the current page. * - * This does not include padding added to center the page on the screen. + * The size of this padding does not scale with pages as the user zooms in and out. + * As such, it is often necessary to get _just_ this padding. + * This does not include padding added to center the page on the screen. * - * @param pageIndex is the index of the XojPageView, as given by [getIndexAtGridMap] - * @return the sum of the padding between pages left of the page at [pageIndex] and - * any horizontal padding the user decided to add (from settings) + * @param x the horizontal position in relative coordinates, up to where the padding + * should be calculated + * @return the sum of the padding between pages left of the given position, plus any + * padding the user added horizontally before all pages (from settings) */ - int getPaddingLeftOfPage(size_t pageIndex) const; + double getPaddingLeftOf(double x) const; protected: // Todo(Fabian): move to ScrollHandling also it must not depend on Layout @@ -178,4 +183,6 @@ class Layout final { mutable PreCalculated pc{}; mutable std::vector colXStart; mutable std::vector rowYStart; + double borderX; + double borderY; };