This patch hold (if possibly) the zoom center below the mouse position (zoom with Ctrl + Mouse Wheel and Middle mouse button) or the center position from the pinch gesture.
BUG: 390707
No Linters Available |
No Unit Test Coverage |
Buildable 13181 | |
Build 13199: arc lint + arc unit |
Using cursor position for Ctrl+Scroll, but not for Ctrl+Plus/Minus makes sense to me.
Can this be extended to using the initial cursor position for middle-button-drag zooming?
Use initial cursor position for middle-button-drag zooming
rework the calculation for newScroll, because d->zoomFactor don't hold the exact zoom factor.
Interactivity is great, a big improvement! Code looks sane to me too but I'd like a review from a Okular person before we land this.
there's a few types
activ -> active
beginn -> begin
and please make centerBeing and centerEnd const
Also it doesn't work? or not how i would expect it?
I'm doing ctrl+wheel zoom here with the mouse over the a and ends up being over the p.
I was expecting it to end up being over the a too?
Hi @aacid,
thanks for testing,
Also it doesn't work? or not how i would expect it?
I'm doing ctrl+wheel zoom here with the mouse over the a and ends up being over the p.
I was expecting it to end up being over the a too?
to begin the width of the document is smaller than the viewport with, so we have no horizontal scrollbar and we can not make a horizontal scrolling.
You can see in your picture the zoom from 100 to 400 the mouse start and ends up being over the p, here we have a scrollbar and can make a horizontal scrolling.
Hmm, now it doesn't work for me anymore. It zooms in to a location near the cursor/center pinch point, but not exactly there.
Sure, but at some point there will be a scrollbar, and at that point you can make it scroll to the "correct", position, no?
Do you mean it should always try to scroll where the zoom was triggered the first time after Ctrl was pressed the last time? I think it makes sense, but only marginally more than scolling to the position of the individual scroll event.
I just tried this patch, and zooming with the middle mouse button is unusable now. The view heavily jaggs arround all the time, you can’t see anything. Maybe make viewport->update() a queued connection in slotRelayoutPages()?
Otherwise it works like I expect.
Maybe it makes sense to stick the cursor position while zooming with the middle mouse button, so the zoom center is shown. (I know that only by Spore.)
Do you mean it should always try to scroll where the zoom was triggered the first time after Ctrl was pressed the last time?
That is what i was expecting this to do yes, if it's supposed to center on the mouse position, it should do that regardless of if there were scrollbars or not
Oops, sorry I lost track of this patch. Can you rebase it please so we can continue the review?
Behaviorally this works great, but visually there's now a big regression: the content itself is no longer visible during the zoom operation; the view blanks out and displays the default white background.
Yes, I have found this, if okular is build / using new poppler versions, but I don't know why. (poppler version poppler-0.77.0 and poppler-0.62.0 is working)
Please move as a Merge Request in https://invent.kde.org/kde/okular
We have pre-commit CI and lots of checks including clazy and clang-tidy there so it's a much better place for doing the review/approval/merge of the code.