Fix some Qt deprecation warnings, memory leaks; clean up related code #206
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vedgy
commented
Feb 10, 2021
| @@ -141,31 +141,34 @@ void MagnifyingGlass::updateImage() | |||
| } | |||
| void MagnifyingGlass::wheelEvent(QWheelEvent *event) | |||
| { | |||
| // TODO: consider handling horizontal scrolling differently. | |||
| const auto delta = event->angleDelta().x() + event->angleDelta().y(); | |||
| const bool grow = delta < 0; | |||
There was a problem hiding this comment.
The zoom in/out direction of the Magnifying glass is opposite to the direction in other applications. Is this intentional or an oversight that should be fixed by initializing grow with delta > 0? If this is intentional, the reason should be explained in a comment.
Add a local elapsedMs variable to improve readability and performance. Remove some of the redundant parentheses in a long `if` statement to improve readability.
QTime's start(), restart() and elapsed() member functions have been deprecated since Qt 5.14 and were removed in Qt 6. Don't leak the timer object in ScrollManagement. Add a local elapsedMs variable to improve readability and performance. Remove some of the redundant parentheses to improve readability.
GoToFlow and ComicFlow are the only two classes that use YACReaderFlow. They both handled wheelEvent() manually in practically the same way as YACReaderFlow does now, except for time and minimum move checks in ScrollManagement::getMovement(). This new implementation is equivalent to and consistent with the OpenGL version in YACReaderFlowGL::wheelEvent().
QWheelEvent::orientation() and QWheelEvent::delta() are deprecated. The documentation for both suggests using QWheelEvent::angleDelta() instead. Keep treating the horizontal mouse wheel the same as vertical in Comic and Page Flow: comics and pages move horizontally, so horizontal scrolling is even more natural than vertical in these cases. Viewer handles horizontal scrolling separately - to scroll the comic page to the right or to the left. So ScrollManagement must ignore horizontal delta in this case. For this purpose the new parameter in ScrollManagement::getMovement() determines which orientations are taken into account: horizontal, vertical or both. Remove redundant parentheses in long `if` statements to improve readability.
Keep treating horizontal and vertical scrolling equivalently.
vedgy
force-pushed
the
fix-some-qt-deprecation-warnings
branch
from
fc01882
to
c8f0be9
Compare
luisangelsm
force-pushed
the
develop
branch
from
6ccfc51
to
63fcde8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the commit messages for details.