Changeset View
Standalone View
lib/documentview/documentview.cpp
Show First 20 Lines • Show All 624 Lines • ▼ Show 20 Line(s) | 620 | void DocumentView::toggleZoomToFit() { | |||
---|---|---|---|---|---|
625 | } | 625 | } | ||
626 | } | 626 | } | ||
627 | 627 | | |||
628 | void DocumentView::setZoomToFill(bool on) | 628 | void DocumentView::setZoomToFill(bool on) | ||
629 | { | 629 | { | ||
630 | if (on == d->mAdapter->zoomToFill()) { | 630 | if (on == d->mAdapter->zoomToFill()) { | ||
631 | return; | 631 | return; | ||
632 | } | 632 | } | ||
633 | d->mAdapter->setZoomToFill(on); | 633 | d->mAdapter->setZoomToFill(on, d->cursorPosition()); | ||
634 | } | 634 | } | ||
635 | 635 | | |||
636 | void DocumentView::toggleZoomToFill() { | 636 | void DocumentView::toggleZoomToFill() { | ||
637 | const bool zoomToFillOn = d->mAdapter->zoomToFill(); | 637 | const bool zoomToFillOn = d->mAdapter->zoomToFill(); | ||
638 | d->mAdapter->setZoomToFill(!zoomToFillOn); | 638 | d->mAdapter->setZoomToFill(!zoomToFillOn, d->cursorPosition()); | ||
639 | if (zoomToFillOn) { | 639 | if (zoomToFillOn) { | ||
640 | d->setZoom(1., d->cursorPosition()); | 640 | d->setZoom(1., d->cursorPosition()); | ||
641 | } | 641 | } | ||
642 | } | 642 | } | ||
643 | 643 | | |||
644 | bool DocumentView::zoomToFit() const | 644 | bool DocumentView::zoomToFit() const | ||
645 | { | 645 | { | ||
646 | return d->mAdapter->zoomToFit(); | 646 | return d->mAdapter->zoomToFit(); | ||
647 | } | 647 | } | ||
648 | 648 | | |||
649 | bool DocumentView::zoomToFill() const | 649 | bool DocumentView::zoomToFill() const | ||
650 | { | 650 | { | ||
651 | return d->mAdapter->zoomToFill(); | 651 | return d->mAdapter->zoomToFill(); | ||
652 | } | 652 | } | ||
653 | 653 | | |||
654 | void DocumentView::zoomActualSize() | 654 | void DocumentView::zoomActualSize() | ||
655 | { | 655 | { | ||
656 | d->uncheckZoomToFit(); | 656 | d->uncheckZoomToFit(); | ||
657 | d->uncheckZoomToFill(); | 657 | d->uncheckZoomToFill(); | ||
658 | d->mAdapter->setZoom(1.); | 658 | d->mAdapter->setZoom(1., d->cursorPosition()); | ||
659 | } | 659 | } | ||
660 | 660 | | |||
661 | void DocumentView::zoomIn(const QPointF& center) | 661 | void DocumentView::zoomIn(QPointF center) | ||
662 | { | 662 | { | ||
663 | if (center == QPointF(-1, -1)) { | ||||
664 | center = d->cursorPosition(); | ||||
665 | } | ||||
663 | qreal currentZoom = d->mAdapter->zoom(); | 666 | qreal currentZoom = d->mAdapter->zoom(); | ||
664 | 667 | | |||
665 | Q_FOREACH(qreal zoom, d->mZoomSnapValues) { | 668 | Q_FOREACH(qreal zoom, d->mZoomSnapValues) { | ||
666 | if (zoom > currentZoom + REAL_DELTA) { | 669 | if (zoom > currentZoom + REAL_DELTA) { | ||
667 | d->setZoom(zoom, center); | 670 | d->setZoom(zoom, center); | ||
668 | return; | 671 | return; | ||
muhlenpfordt: If zoom in/out is triggered by a mouse press event in `AbstractImageView` we can use the given… | |||||
Good call. Should I also add this to toggleZoomToFill then, which can also be triggered by a shortcut or by a click? rkflx: Good call. Should I also add this to `toggleZoomToFill` then, which can also be triggered by a… | |||||
Sure, I think using the mouse event position is more reliable and definitely faster. muhlenpfordt: Sure, I think using the mouse event position is more reliable and definitely faster. | |||||
669 | } | 672 | } | ||
670 | } | 673 | } | ||
671 | } | 674 | } | ||
672 | 675 | | |||
673 | void DocumentView::zoomOut(const QPointF& center) | 676 | void DocumentView::zoomOut(QPointF center) | ||
674 | { | 677 | { | ||
678 | if (center == QPointF(-1, -1)) { | ||||
679 | center = d->cursorPosition(); | ||||
680 | } | ||||
675 | qreal currentZoom = d->mAdapter->zoom(); | 681 | qreal currentZoom = d->mAdapter->zoom(); | ||
676 | 682 | | |||
677 | QListIterator<qreal> it(d->mZoomSnapValues); | 683 | QListIterator<qreal> it(d->mZoomSnapValues); | ||
678 | it.toBack(); | 684 | it.toBack(); | ||
679 | while (it.hasPrevious()) { | 685 | while (it.hasPrevious()) { | ||
680 | qreal zoom = it.previous(); | 686 | qreal zoom = it.previous(); | ||
681 | if (zoom < currentZoom - REAL_DELTA) { | 687 | if (zoom < currentZoom - REAL_DELTA) { | ||
682 | d->setZoom(zoom, center); | 688 | d->setZoom(zoom, center); | ||
683 | return; | 689 | return; | ||
The ternary operator is great for choosing between two options based on a condition, e.g. shiftPressed ? fill() : fit(). However here you are trying to update a value (which is also part of the condition) if it is not valid yet, and which in theory can also be used in other parts of the function. IMO in that case it makes sense to write an explicit if right at the top of the function. Or is this just my personal preference? Commit as you wish ;) rkflx: The ternary operator is great for //choosing// between two options based on a condition, e.g. | |||||
What do you mean with "update a value"? It's just a switch for the second argument of setZoom(). void DocumentView::zoomOut(QPointF center) { if (center == QPointF(-1, -1)) { center = d->cursorPosition(); } muhlenpfordt: What do you mean with "update a value"? It's just a switch for the second argument of `setZoom… | |||||
It was just nitpicking in search for the safest, most readable and easiest to understand code. I commented because to me it seemed a bit strange to write inline what is essentially code for replacing the value of center with something more valid. Don't overthink this, just commit what you prefer ;)
Yes, that's what I was going for. rkflx: It was just nitpicking in search for the safest, most readable and easiest to understand code. | |||||
684 | } | 690 | } | ||
685 | } | 691 | } | ||
686 | } | 692 | } | ||
687 | 693 | | |||
688 | void DocumentView::slotZoomChanged(qreal zoom) | 694 | void DocumentView::slotZoomChanged(qreal zoom) | ||
689 | { | 695 | { | ||
690 | d->updateCaption(); | 696 | d->updateCaption(); | ||
691 | zoomChanged(zoom); | 697 | zoomChanged(zoom); | ||
▲ Show 20 Lines • Show All 318 Lines • Show Last 20 Lines |
If zoom in/out is triggered by a mouse press event in AbstractImageView we can use the given position and don't need to recalculate it.