Changeset View
Standalone View
lib/documentview/documentview.cpp
Show All 35 Lines | |||||
36 | #include <QPropertyAnimation> | 36 | #include <QPropertyAnimation> | ||
37 | #include <QPointer> | 37 | #include <QPointer> | ||
38 | #include <QDebug> | 38 | #include <QDebug> | ||
39 | #include <QIcon> | 39 | #include <QIcon> | ||
40 | #include <QUrl> | 40 | #include <QUrl> | ||
41 | #include <QDrag> | 41 | #include <QDrag> | ||
42 | #include <QMimeData> | 42 | #include <QMimeData> | ||
43 | #include <QStyleHints> | 43 | #include <QStyleHints> | ||
44 | #include <QGestureEvent> | ||||
45 | #include <QTouchEvent> | ||||
44 | 46 | | |||
ngraham: Unnecessary whitespace change | |||||
45 | // KDE | 47 | // KDE | ||
46 | #include <KLocalizedString> | 48 | #include <KLocalizedString> | ||
47 | #include <KFileItem> | 49 | #include <KFileItem> | ||
48 | 50 | | |||
ngraham: Unnecessary whitespace change | |||||
49 | // Local | 51 | // Local | ||
50 | #include <lib/document/document.h> | 52 | #include <lib/document/document.h> | ||
51 | #include <lib/document/documentfactory.h> | 53 | #include <lib/document/documentfactory.h> | ||
52 | #include <lib/documentview/abstractrasterimageviewtool.h> | 54 | #include <lib/documentview/abstractrasterimageviewtool.h> | ||
53 | #include <lib/documentview/birdeyeview.h> | 55 | #include <lib/documentview/birdeyeview.h> | ||
54 | #include <lib/documentview/loadingindicator.h> | 56 | #include <lib/documentview/loadingindicator.h> | ||
55 | #include <lib/documentview/messageviewadapter.h> | 57 | #include <lib/documentview/messageviewadapter.h> | ||
56 | #include <lib/documentview/rasterimageview.h> | 58 | #include <lib/documentview/rasterimageview.h> | ||
57 | #include <lib/documentview/rasterimageviewadapter.h> | 59 | #include <lib/documentview/rasterimageviewadapter.h> | ||
58 | #include <lib/documentview/svgviewadapter.h> | 60 | #include <lib/documentview/svgviewadapter.h> | ||
59 | #include <lib/documentview/videoviewadapter.h> | 61 | #include <lib/documentview/videoviewadapter.h> | ||
60 | #include <lib/hud/hudbutton.h> | 62 | #include <lib/hud/hudbutton.h> | ||
61 | #include <lib/hud/hudwidget.h> | 63 | #include <lib/hud/hudwidget.h> | ||
62 | #include <lib/graphicswidgetfloater.h> | 64 | #include <lib/graphicswidgetfloater.h> | ||
63 | #include <lib/gvdebug.h> | 65 | #include <lib/gvdebug.h> | ||
64 | #include <lib/gwenviewconfig.h> | 66 | #include <lib/gwenviewconfig.h> | ||
65 | #include <lib/mimetypeutils.h> | 67 | #include <lib/mimetypeutils.h> | ||
66 | #include <lib/signalblocker.h> | 68 | #include <lib/signalblocker.h> | ||
67 | #include <lib/urlutils.h> | 69 | #include <lib/urlutils.h> | ||
68 | #include <lib/thumbnailview/dragpixmapgenerator.h> | 70 | #include <lib/thumbnailview/dragpixmapgenerator.h> | ||
69 | #include <lib/thumbnailprovider/thumbnailprovider.h> | 71 | #include <lib/thumbnailprovider/thumbnailprovider.h> | ||
72 | #include <transformimageoperation.h> | ||||
70 | 73 | | |||
71 | namespace Gwenview | 74 | namespace Gwenview | ||
72 | { | 75 | { | ||
73 | 76 | | |||
74 | #undef ENABLE_LOG | 77 | #undef ENABLE_LOG | ||
75 | #undef LOG | 78 | #undef LOG | ||
76 | //#define ENABLE_LOG | 79 | //#define ENABLE_LOG | ||
77 | #ifdef ENABLE_LOG | 80 | #ifdef ENABLE_LOG | ||
Show All 18 Lines | 97 | { | |||
96 | int mSortKey; // Used to sort views when displayed in compare mode | 99 | int mSortKey; // Used to sort views when displayed in compare mode | ||
97 | HudWidget* mHud; | 100 | HudWidget* mHud; | ||
98 | BirdEyeView* mBirdEyeView; | 101 | BirdEyeView* mBirdEyeView; | ||
99 | QPointer<QPropertyAnimation> mMoveAnimation; | 102 | QPointer<QPropertyAnimation> mMoveAnimation; | ||
100 | QPointer<QPropertyAnimation> mFadeAnimation; | 103 | QPointer<QPropertyAnimation> mFadeAnimation; | ||
101 | QGraphicsOpacityEffect* mOpacityEffect; | 104 | QGraphicsOpacityEffect* mOpacityEffect; | ||
102 | 105 | | |||
103 | LoadingIndicator* mLoadingIndicator; | 106 | LoadingIndicator* mLoadingIndicator; | ||
104 | 107 | | |||
ngraham: Unnecessary whitespace change | |||||
105 | QScopedPointer<AbstractDocumentViewAdapter> mAdapter; | 108 | QScopedPointer<AbstractDocumentViewAdapter> mAdapter; | ||
106 | QList<qreal> mZoomSnapValues; | 109 | QList<qreal> mZoomSnapValues; | ||
107 | Document::Ptr mDocument; | 110 | Document::Ptr mDocument; | ||
108 | DocumentView::Setup mSetup; | 111 | DocumentView::Setup mSetup; | ||
109 | bool mCurrent; | 112 | bool mCurrent; | ||
110 | bool mCompareMode; | 113 | bool mCompareMode; | ||
111 | int controlWheelAccumulatedDelta; | 114 | int controlWheelAccumulatedDelta; | ||
112 | 115 | | |||
113 | QPointF mDragStartPosition; | 116 | QPointF mDragStartPosition; | ||
114 | QPointer<ThumbnailProvider> mDragThumbnailProvider; | 117 | QPointer<ThumbnailProvider> mDragThumbnailProvider; | ||
115 | QPointer<QDrag> mDrag; | 118 | QPointer<QDrag> mDrag; | ||
119 | qint64 timestamp; | ||||
120 | qreal rotationsDelta = 25; | ||||
121 | qreal lastRotation; | ||||
rkflx: It's `Tap`, not `Tab` (applies everywhere). | |||||
122 | bool firstMoving; | ||||
116 | 123 | | |||
117 | void setCurrentAdapter(AbstractDocumentViewAdapter* adapter) | 124 | void setCurrentAdapter(AbstractDocumentViewAdapter* adapter) | ||
118 | { | 125 | { | ||
119 | Q_ASSERT(adapter); | 126 | Q_ASSERT(adapter); | ||
120 | mAdapter.reset(adapter); | 127 | mAdapter.reset(adapter); | ||
Member variables should be prefixed with a lowercase m. Also, some of the names are too generic, e.g. timestamp in the context of DocumentView is not descriptive enough. That said, I wonder why all of those variables are really necessary. Is there no Qt mechanism to differentiate between different gesture and to get at timestamps or positions of gestures? rkflx: Member variables should be prefixed with a lowercase `m`.
Also, some of the names are too… | |||||
121 | 128 | | |||
122 | adapter->widget()->setParentItem(q); | 129 | adapter->widget()->setParentItem(q); | ||
123 | resizeAdapterWidget(); | 130 | resizeAdapterWidget(); | ||
124 | 131 | | |||
125 | if (adapter->canZoom()) { | 132 | if (adapter->canZoom()) { | ||
126 | QObject::connect(adapter, SIGNAL(zoomChanged(qreal)), | 133 | QObject::connect(adapter, SIGNAL(zoomChanged(qreal)), | ||
127 | q, SLOT(slotZoomChanged(qreal))); | 134 | q, SLOT(slotZoomChanged(qreal))); | ||
128 | QObject::connect(adapter, SIGNAL(zoomInRequested(QPointF)), | 135 | QObject::connect(adapter, SIGNAL(zoomInRequested(QPointF)), | ||
▲ Show 20 Lines • Show All 299 Lines • ▼ Show 20 Line(s) | 430 | { | |||
428 | d->q = this; | 435 | d->q = this; | ||
429 | d->mLoadingIndicator = nullptr; | 436 | d->mLoadingIndicator = nullptr; | ||
430 | d->mBirdEyeView = nullptr; | 437 | d->mBirdEyeView = nullptr; | ||
431 | d->mCurrent = false; | 438 | d->mCurrent = false; | ||
432 | d->mCompareMode = false; | 439 | d->mCompareMode = false; | ||
433 | d->controlWheelAccumulatedDelta = 0; | 440 | d->controlWheelAccumulatedDelta = 0; | ||
434 | d->mDragStartPosition = QPointF(0, 0); | 441 | d->mDragStartPosition = QPointF(0, 0); | ||
435 | d->mDrag = nullptr; | 442 | d->mDrag = nullptr; | ||
443 | grabGesture(Qt::PinchGesture); | ||||
444 | setAcceptTouchEvents (true); | ||||
rkflx: Remove extra space before `(`. | |||||
436 | 445 | | |||
437 | // We use an opacity effect instead of using the opacity property directly, because the latter operates at | 446 | // We use an opacity effect instead of using the opacity property directly, because the latter operates at | ||
438 | // the painter level, which means if you draw multiple layers in paint(), all layers get the specified | 447 | // the painter level, which means if you draw multiple layers in paint(), all layers get the specified | ||
439 | // opacity, resulting in all layers being visible when 0 < opacity < 1. | 448 | // opacity, resulting in all layers being visible when 0 < opacity < 1. | ||
440 | // QGraphicsEffects on the other hand, operate after all painting is done, therefore 'flattening' all layers. | 449 | // QGraphicsEffects on the other hand, operate after all painting is done, therefore 'flattening' all layers. | ||
If the reason why this code is commented out is because the features haven't been implemented yet, please add a comment indicating this. ngraham: If the reason why this code is commented out is because the features haven't been implemented… | |||||
441 | // This is important for fade effects, where we don't want any background layers visible during the fade. | 450 | // This is important for fade effects, where we don't want any background layers visible during the fade. | ||
442 | d->mOpacityEffect = new QGraphicsOpacityEffect(this); | 451 | d->mOpacityEffect = new QGraphicsOpacityEffect(this); | ||
443 | d->mOpacityEffect->setOpacity(0); | 452 | d->mOpacityEffect->setOpacity(0); | ||
444 | setGraphicsEffect(d->mOpacityEffect); | 453 | setGraphicsEffect(d->mOpacityEffect); | ||
445 | 454 | | |||
446 | scene->addItem(this); | 455 | scene->addItem(this); | ||
447 | 456 | | |||
448 | d->setupHud(); | 457 | d->setupHud(); | ||
▲ Show 20 Lines • Show All 216 Lines • ▼ Show 20 Line(s) | 673 | { | |||
665 | d->setZoom(zoom); | 674 | d->setZoom(zoom); | ||
666 | } | 675 | } | ||
667 | 676 | | |||
668 | qreal DocumentView::zoom() const | 677 | qreal DocumentView::zoom() const | ||
669 | { | 678 | { | ||
670 | return d->mAdapter->zoom(); | 679 | return d->mAdapter->zoom(); | ||
671 | } | 680 | } | ||
672 | 681 | | |||
682 | bool DocumentView::event(QEvent* event) | ||||
I did not review event, gestureEvent and the corresponding functions in ThumbnailView in detail yet, but some general comments:
rkflx: I did not review `event`, `gestureEvent` and the corresponding functions in `ThumbnailView` in… | |||||
683 | { | ||||
684 | | ||||
685 | if (event->type() == QEvent::TouchBegin) { | ||||
686 | QTouchEvent *touchEvent = static_cast<QTouchEvent *>(event); | ||||
Coding style: opening braces in if statements go on the same line (do the same below, too). ngraham: Coding style: opening braces in `if` statements go on the same line (do the same below, too). | |||||
687 | event->accept(); | ||||
688 | d->timestamp = touchEvent->timestamp(); | ||||
689 | d->firstMoving = true; | ||||
ngraham: Coding style: add spaces around the `=` (do the same below, too). | |||||
690 | return true; | ||||
691 | } | ||||
692 | if (event->type() == QEvent::TouchUpdate) { | ||||
693 | QTouchEvent *touchEvent = static_cast<QTouchEvent *>(event); | ||||
694 | if (touchEvent->touchPoints().size() == 1) { | ||||
695 | qint64 now = touchEvent->timestamp(); | ||||
696 | if (d->firstMoving && now - d->timestamp >= 400) { | ||||
697 | d->startDragIfSensible(); | ||||
698 | } | ||||
699 | d->firstMoving = false; | ||||
Coding style: this line is very long and a bit difficult to read. Might be better formatted with variables, like so: const QPointF start = touchevent->touchPoints().at(0).startPos(); const QPointF end = touchevent->touchPoints().at(0).pos(); QPoint diff = start - end; ngraham: Coding style: this line is very long and a bit difficult to read. Might be better formatted… | |||||
700 | } | ||||
701 | return true; | ||||
702 | } | ||||
703 | if (event->type() == QEvent::TouchEnd) { | ||||
704 | d->firstMoving = false; | ||||
705 | QTouchEvent* touchEvent = static_cast<QTouchEvent *>(event); | ||||
Coding style: comments should be in English. @rkflx (or many others) can probably provide an English translation for you. ngraham: Coding style: comments should be in English. @rkflx (or many others) can probably provide an… | |||||
706 | const QPointF start = touchEvent->touchPoints().at(0).startPos(); | ||||
ngraham: Coding style: space after the `<` | |||||
707 | const QPointF end = touchEvent->touchPoints().at(0).pos(); | ||||
708 | QPointF diff = start - end; | ||||
ngraham: Coding style: indent this line (do the same below, too). | |||||
709 | qint64 now = touchEvent->timestamp(); | ||||
710 | qint64 time = now - d->timestamp; | ||||
711 | if (touchEvent->touchPoints().size() == 1 && diff.manhattanLength() > 200 && time < 500) { | ||||
712 | if (diff.x() < 0 && abs(diff.x()) >= abs(diff.y())) { | ||||
713 | d->mAdapter->previousImageRequested(); | ||||
714 | } | ||||
715 | if (diff.x() > 0 && abs(diff.x()) >= abs(diff.y())) { | ||||
716 | d->mAdapter->nextImageRequested(); | ||||
717 | } | ||||
718 | } | ||||
719 | } | ||||
720 | | ||||
ngraham: Coding style: unnecessary whitespace | |||||
721 | if ( event->type() == QEvent::Gesture ) { | ||||
722 | return gestureEvent(static_cast<QGestureEvent*>( event )); | ||||
723 | } | ||||
724 | return QGraphicsWidget::event( event ); | ||||
725 | } | ||||
726 | | ||||
727 | bool DocumentView::gestureEvent( QGestureEvent* event ) | ||||
728 | { | ||||
729 | QPinchGesture *pinch = static_cast<QPinchGesture*>(event->gesture(Qt::PinchGesture)); | ||||
730 | | ||||
731 | if (pinch) | ||||
732 | { | ||||
Coding style: insufficient indentation for this level and everything below it ngraham: Coding style: insufficient indentation for this level and everything below it | |||||
733 | event->accept(); | ||||
734 | if (pinch->state() == Qt::GestureStarted) { | ||||
735 | d->lastRotation = 0; | ||||
736 | } | ||||
737 | if (pinch->state() == Qt::GestureUpdated) { | ||||
738 | if (pinch->changeFlags() & QPinchGesture::ScaleFactorChanged) { | ||||
739 | if (d->mAdapter->canZoom()) { | ||||
740 | qreal currentZoom = d->mAdapter->zoom(); | ||||
741 | qreal zoom = currentZoom * pinch->scaleFactor(); | ||||
742 | qreal diff = currentZoom > zoom ? currentZoom - zoom : zoom - currentZoom; | ||||
743 | if (diff > 0.001) { | ||||
744 | d->setZoom(zoom,mapToScene (pinch->lastCenterPoint())); | ||||
745 | } | ||||
746 | } | ||||
747 | } | ||||
ngraham: coding style: add whitespace around the `,` | |||||
748 | | ||||
749 | if (pinch->changeFlags() & QPinchGesture::RotationAngleChanged) { | ||||
750 | if (pinch->rotationAngle()-d->lastRotation > d->rotationsDelta) { | ||||
751 | TransformImageOperation *op = new TransformImageOperation(ROT_90); | ||||
752 | op->applyToDocument(d->mDocument); | ||||
753 | d->lastRotation = pinch->rotationAngle(); | ||||
754 | } | ||||
755 | else if (-(pinch->rotationAngle()-d->lastRotation) > d->rotationsDelta) { | ||||
756 | TransformImageOperation *op = new TransformImageOperation(ROT_270); | ||||
757 | op->applyToDocument(d->mDocument); | ||||
758 | d->lastRotation = pinch->rotationAngle(); | ||||
759 | } | ||||
760 | } | ||||
761 | } | ||||
762 | if (pinch->state() == Qt::GestureFinished) { | ||||
763 | d->lastRotation = 0; | ||||
764 | } | ||||
765 | return true; | ||||
766 | } | ||||
767 | return false; | ||||
768 | } | ||||
769 | | ||||
673 | void DocumentView::resizeEvent(QGraphicsSceneResizeEvent *event) | 770 | void DocumentView::resizeEvent(QGraphicsSceneResizeEvent *event) | ||
674 | { | 771 | { | ||
675 | d->resizeAdapterWidget(); | 772 | d->resizeAdapterWidget(); | ||
676 | d->updateZoomSnapValues(); | 773 | d->updateZoomSnapValues(); | ||
677 | QGraphicsWidget::resizeEvent(event); | 774 | QGraphicsWidget::resizeEvent(event); | ||
678 | } | 775 | } | ||
679 | 776 | | |||
680 | void DocumentView::wheelEvent(QGraphicsSceneWheelEvent* event) | 777 | void DocumentView::wheelEvent(QGraphicsSceneWheelEvent* event) | ||
681 | { | 778 | { | ||
682 | if (d->mAdapter->canZoom() && event->modifiers() & Qt::ControlModifier) { | 779 | if (d->mAdapter->canZoom() && event->modifiers() & Qt::ControlModifier) { | ||
683 | d->controlWheelAccumulatedDelta += event->delta(); | 780 | d->controlWheelAccumulatedDelta += event->delta(); | ||
684 | // Ctrl + wheel => zoom in or out | 781 | // Ctrl + wheel => zoom in or out | ||
685 | if (d->controlWheelAccumulatedDelta >= QWheelEvent::DefaultDeltasPerStep) { | 782 | if (d->controlWheelAccumulatedDelta >= QWheelEvent::DefaultDeltasPerStep) { | ||
686 | zoomIn(event->pos()); | 783 | zoomIn(event->pos()); | ||
687 | d->controlWheelAccumulatedDelta = 0; | 784 | d->controlWheelAccumulatedDelta = 0; | ||
ngraham: Should probably be `sensitivityModifier` | |||||
688 | } else if (d->controlWheelAccumulatedDelta <= -QWheelEvent::DefaultDeltasPerStep) { | 785 | } else if (d->controlWheelAccumulatedDelta <= -QWheelEvent::DefaultDeltasPerStep) { | ||
689 | zoomOut(event->pos()); | 786 | zoomOut(event->pos()); | ||
690 | d->controlWheelAccumulatedDelta = 0; | 787 | d->controlWheelAccumulatedDelta = 0; | ||
691 | } | 788 | } | ||
692 | return; | 789 | return; | ||
693 | } | 790 | } | ||
694 | if (GwenviewConfig::mouseWheelBehavior() == MouseWheelBehavior::Browse | 791 | if (GwenviewConfig::mouseWheelBehavior() == MouseWheelBehavior::Browse | ||
695 | && event->modifiers() == Qt::NoModifier) { | 792 | && event->modifiers() == Qt::NoModifier) { | ||
▲ Show 20 Lines • Show All 188 Lines • ▼ Show 20 Line(s) | 977 | if (event->type() == QEvent::GraphicsSceneMousePress) { | |||
884 | } | 981 | } | ||
885 | QMetaObject::invokeMethod(this, "emitFocused", Qt::QueuedConnection); | 982 | QMetaObject::invokeMethod(this, "emitFocused", Qt::QueuedConnection); | ||
886 | } else if (event->type() == QEvent::GraphicsSceneHoverMove) { | 983 | } else if (event->type() == QEvent::GraphicsSceneHoverMove) { | ||
887 | if (d->mBirdEyeView) { | 984 | if (d->mBirdEyeView) { | ||
888 | d->mBirdEyeView->onMouseMoved(); | 985 | d->mBirdEyeView->onMouseMoved(); | ||
889 | } | 986 | } | ||
890 | } else if (event->type() == QEvent::GraphicsSceneMouseMove) { | 987 | } else if (event->type() == QEvent::GraphicsSceneMouseMove) { | ||
891 | const QGraphicsSceneMouseEvent* mouseEvent = static_cast<QGraphicsSceneMouseEvent*>(event); | 988 | const QGraphicsSceneMouseEvent* mouseEvent = static_cast<QGraphicsSceneMouseEvent*>(event); | ||
892 | const qreal dragDistance = (mouseEvent->pos() - d->mDragStartPosition).manhattanLength(); | 989 | const qreal dragDistance = (mouseEvent->pos() - d->mDragStartPosition).manhattanLength(); | ||
rkflx: Space before `{` missing. | |||||
ngraham: Which older versions? If it's older than 5.9, we don't need this. | |||||
can we keep this for the moment, because I need it on my working system (OpenSUSE Leap, Qt 5.9.4, KDE 5.45) ? steffenh: can we keep this for the moment, because I need it on my working system (OpenSUSE Leap, Qt 5.9. | |||||
ngraham: OK. | |||||
893 | const qreal minDistanceToStartDrag = QGuiApplication::styleHints()->startDragDistance(); | 990 | const qreal minDistanceToStartDrag = QGuiApplication::styleHints()->startDragDistance(); | ||
894 | if (!d->canPan() && dragDistance >= minDistanceToStartDrag) { | 991 | if (!d->canPan() && dragDistance >= minDistanceToStartDrag && mouseEvent->source() == Qt::MouseEventNotSynthesized) { | ||
rkflx: Is this still needed given the check above? | |||||
895 | d->startDragIfSensible(); | 992 | d->startDragIfSensible(); | ||
896 | } | 993 | } | ||
897 | } | 994 | } | ||
898 | return false; | 995 | return false; | ||
899 | } | 996 | } | ||
900 | 997 | | |||
901 | AbstractRasterImageViewTool* DocumentView::currentTool() const | 998 | AbstractRasterImageViewTool* DocumentView::currentTool() const | ||
902 | { | 999 | { | ||
▲ Show 20 Lines • Show All 56 Lines • Show Last 20 Lines |
Unnecessary whitespace change