BUG: 159772
https://bugs.kde.org/show_bug.cgi?id=159772
Adopted patch by David Benjamin + some minor documentation.
Details
Details
- Reviewers
tcanabrava - Group Reviewers
KDE Edu - Commits
- R334:acf37fcffd9d: Implement zooming with Ctrl+mouse wheel
- Plot something.
- Use Ctrl+mouse wheel to zoom in/out.
- Use toolbar or menu to zoom in/out.
Diff Detail
Diff Detail
- Repository
- R334 KmPlot
- Lint
Lint Skipped - Unit
Unit Tests Skipped
| kmplot/view.cpp | ||
|---|---|---|
| 3691–3712 | Because switching to QPropertyAnimation cannot be done without complete rewriting half of KmPlot classes and breaks the whole current structure. There should be implemented some property of View to hold the current viewport to tie it up with QPropertyAnimation, then it should be connected with some timer and updates, and maybe other things, I'm not a skilled Qt programmer... It is easy to use QEasingCurve and QPropertyAnimation when the widget is ready for this. View is a different thing now. It might be the stunning implementation with 6 lines, but it is impossible now. The proposed patch just works. People wanted this for more than 10 years (21 votes). Sorry. | |
| kmplot/view.h | ||
| 637–640 | "TopLeft" and "BottomRight". Fixed in the new version. | |
Comment Actions
repaint will trigger a reppaint forcefully, if you use update() are we skipping frames?
after this and the old style connect are working I'd give a +1.
what happens if you scroll up and down too quick for the screen to repaint?
| kmplot/view.cpp | ||
|---|---|---|
| 3709 | old style connect. | |
Comment Actions
No.
after this and the old style connect are working I'd give a +1.
what happens if you scroll up and down too quick for the screen to repaint?
Values on the axes grow exponentially. It might happen that my computer is too fast for this testing (Intel Core 2 E4700) but all I can see is a series of rectangular borders moving in/out (f(x) = x^3 -3*x^2 with its first and second derivatives + integral, default scaling).
Comment Actions
QEasingCurve + a QPropertyAnimation version. Short, compiles but does not work at all. I'd be grateful for the answer on why it does not work.
Comment Actions
Working patch with QEasingCurve and QPropertyAnimation.
Sorry for these constant changes. It seems that I'm not a very fast student. :'(
Can someone tell which of the proposed ways to fix the bug is better?
Thanks in advance for your advice.
| kmplot/view.cpp | ||
|---|---|---|
| 3646 ↗ | (On Diff #46220) | You either need to delete the existing one or not create one every single time otherwise we end up with lots of qpropertyanimations when we don't really need that many, no? |
| kmplot/view.cpp | ||
|---|---|---|
| 3401 | You may want to accumulate the deltas, some touchpads send lots of events with very small deltas, so just doing >0 and <0 can make it very difficult to control, see https://cgit.kde.org/okular.git/commit/?id=7a50ce0edfc9be8bd23441e52a4f3a0c60f7e60f | |
| kmplot/view.h | ||
| 86 | What is this propety and the new functions for? They are never called, no? | |
| kmplot/view.h | ||
|---|---|---|
| 86 | They are required for QPropertyAnimation | |
Comment Actions
- The property and functions are for QPropertyAnimation (proposed by Tomaz Canabrava to replace a good bunch of hand-written code by KmPlot authors), as it was already mentioned by Christoph Feck (thanks).
- I have tried to add QWheelEvent::DefaultDeltasPerStep threshold to mitigate potential unwanted behavior.
Comment Actions
You need to accumulate them, otherwise people where all the deltas are small will never get it to work.
Comment Actions
Something along the lines, at least works with my mouse and does not break things on the common machines.
Comment Actions
Trying to mimic zeroing of the accumulated scrolling with mouse button clicking in Okular.
Comment Actions
I *have not tried* it.
I think the code is in good enough shape now.
If you're confident with your testing is good enough, for me you can commit it (with the const fix)
If you want someone else to give it a test say so and i'll test it a bit.
| kmplot/view.cpp | ||
|---|---|---|
| 3689 ↗ | (On Diff #46220) | this function should probably be const |
Comment Actions
Many thanks for all reviews.
It would be great if someone can test the patch on a machine with touchpad. Regretfully, I have no notebook to test.
Should there be no testers and no objections I will try to push the patch to the repo in a week.
Many thanks in advance for your help.
Comment Actions
Dam something has changed and now mousemoveevent is also being called. It seems a Qt regression to me, but who's going to fix Qt... :D
Meh, that means that okular is also broken too (ok, not anymore, just fixed it in https://cgit.kde.org/okular.git/commit/?id=ba1c2beb70f0ebe3eed2ddda6709eabb2422e7ea )
If you apply https://paste.kde.org/p0s0avu28 it works fine for me using my laptop touchpad.
Should there be no testers and no objections I will try to push the patch to the repo in a week.
Many thanks in advance for your help.