From 42bb6acf56471e7308926e6b242fbbdd4103f306 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Mon, 5 Jun 2017 08:46:04 -0400 Subject: [PATCH] Simplify some Viewer code, and fix a few bugs The content rect was always identical to the window rect, so most of the related code did nothing. The translation limit code is always useful (to avoid dragging the slide way off-screen with the mouse), so always include it. The auto-scaling to fit the screen is also still useful, but just base it on the window rect. The zoom code has four state variables, only used two of them, and one was a trivially derived computation. Fold most of that work into computeMatrix. (The translation was always zero -- we never changed the zoom center.) Include fDefaultMatrix in the matrix from computeMatrix, rather than needing to apply it specially to the canvas. Don't apply the inverse default matrix to touch or mouse points. The absolute positions of those touch points is not important, but because that matrix includes scale (and sometimes very large or very small scale), it just had the effect of greatly amplifying or damping the drag speed. Without it, the slide always pans at the speed of the touch/mouse drag -- which seems more desirable. The use of the inverse default matrix was a clever trick, but it caused the translation (applied to the global mtx) to be scaled, so the slide was always pinned incorrectly. Instead, supply the unmodified window rect and the default matrix, so the trans limit code can do the obvious correct thing: xform the slide bounds completely, then limit the translation that will be applied after that. Slides are now correctly pinned to screen edge regardless of how much zoom is present in the default matrix. Note: There are still several bugs related to all of this code, but given the web of xform state, it's hard to unravel. The touch gesture still doesn't know about viewer's zoom, so that's ignored when doing the pinning. Beyond that, it doesn't even know about window resize - it only configures the translation limit when setting up a slide. I had a fix for all of this (doing the translation limiting in computeMatrix), but then the touch gesture doesn't know about it, and can accumulate drag motion that needs to be un-dragged to get back on-screen, even though the slide is never really translated that far. SkTouchGesture is in include. No one uses it except viewer: TBR=bsalomon@google.com Bug: skia: Change-Id: I460cc07c3de6d36e63826f57d359faf1facf5ab3 Reviewed-on: https://skia-review.googlesource.com/18524 Reviewed-by: Brian Osman Reviewed-by: Yuqian Li Commit-Queue: Brian Osman --- include/views/SkTouchGesture.h | 5 +- src/views/SkTouchGesture.cpp | 5 +- tools/viewer/Viewer.cpp | 82 +++++-------------- tools/viewer/Viewer.h | 4 - tools/viewer/sk_app/Window.h | 2 - tools/viewer/sk_app/android/Window_android.h | 4 - .../sk_app/android/surface_glue_android.cpp | 1 - 7 files changed, 27 insertions(+), 76 deletions(-) diff --git a/include/views/SkTouchGesture.h b/include/views/SkTouchGesture.h index 4d4c0312d3..6776de7267 100644 --- a/include/views/SkTouchGesture.h +++ b/include/views/SkTouchGesture.h @@ -43,7 +43,8 @@ public: const SkMatrix& localM(); const SkMatrix& globalM() const { return fGlobalM; } - void setTransLimit(const SkRect& contentRect, const SkRect& windowRect); + void setTransLimit(const SkRect& contentRect, const SkRect& windowRect, + const SkMatrix& preTouchM); private: enum State { @@ -62,7 +63,7 @@ private: SkTDArray fTouches; State fState; - SkMatrix fLocalM, fGlobalM; + SkMatrix fLocalM, fGlobalM, fPreTouchM; SkFlingState fFlinger; double fLastUpMillis; SkPoint fLastUpP; diff --git a/src/views/SkTouchGesture.cpp b/src/views/SkTouchGesture.cpp index 752828e37f..cd7388b6e1 100644 --- a/src/views/SkTouchGesture.cpp +++ b/src/views/SkTouchGesture.cpp @@ -331,10 +331,12 @@ bool SkTouchGesture::handleDblTap(float x, float y) { return found; } -void SkTouchGesture::setTransLimit(const SkRect& contentRect, const SkRect& windowRect) { +void SkTouchGesture::setTransLimit(const SkRect& contentRect, const SkRect& windowRect, + const SkMatrix& preTouchMatrix) { fIsTransLimited = true; fContentRect = contentRect; fWindowRect = windowRect; + fPreTouchM = preTouchMatrix; } void SkTouchGesture::limitTrans() { @@ -343,6 +345,7 @@ void SkTouchGesture::limitTrans() { } SkRect scaledContent = fContentRect; + fPreTouchM.mapRect(&scaledContent); fGlobalM.mapRect(&scaledContent); const SkScalar ZERO = 0; diff --git a/tools/viewer/Viewer.cpp b/tools/viewer/Viewer.cpp index d1e48dda09..3503e6481f 100644 --- a/tools/viewer/Viewer.cpp +++ b/tools/viewer/Viewer.cpp @@ -250,10 +250,7 @@ Viewer::Viewer(int argc, char** argv, void* platformData) , fBackendType(sk_app::Window::kNativeGL_BackendType) , fColorMode(ColorMode::kLegacy) , fColorSpacePrimaries(gSrgbPrimaries) - , fZoomCenterX(0.0f) - , fZoomCenterY(0.0f) , fZoomLevel(0.0f) - , fZoomScale(SK_Scalar1) { static SkTaskGroup::Enabler kTaskGroupEnabler; SkGraphics::Init(); @@ -619,26 +616,20 @@ void Viewer::setupCurrentSlide(int previousSlide) { fGesture.reset(); fDefaultMatrix.reset(); - fDefaultMatrixInv.reset(); - if (fWindow->supportsContentRect() && fWindow->scaleContentToFit()) { - const SkRect contentRect = fWindow->getContentRect(); - const SkISize slideSize = fSlides[fCurrentSlide]->getDimensions(); - const SkRect slideBounds = SkRect::MakeIWH(slideSize.width(), slideSize.height()); - if (contentRect.width() > 0 && contentRect.height() > 0) { - fDefaultMatrix.setRectToRect(slideBounds, contentRect, SkMatrix::kStart_ScaleToFit); - SkAssertResult(fDefaultMatrix.invert(&fDefaultMatrixInv)); + const SkISize slideSize = fSlides[fCurrentSlide]->getDimensions(); + const SkRect slideBounds = SkRect::MakeIWH(slideSize.width(), slideSize.height()); + const SkRect windowRect = SkRect::MakeIWH(fWindow->width(), fWindow->height()); + + // Start with a matrix that scales the slide to the available screen space + if (fWindow->scaleContentToFit()) { + if (windowRect.width() > 0 && windowRect.height() > 0) { + fDefaultMatrix.setRectToRect(slideBounds, windowRect, SkMatrix::kStart_ScaleToFit); } } - if (fWindow->supportsContentRect()) { - const SkISize slideSize = fSlides[fCurrentSlide]->getDimensions(); - SkRect windowRect = fWindow->getContentRect(); - fDefaultMatrixInv.mapRect(&windowRect); - fGesture.setTransLimit(SkRect::MakeWH(SkIntToScalar(slideSize.width()), - SkIntToScalar(slideSize.height())), - windowRect); - } + // Prevent the user from dragging content so far outside the window they can't find it again + fGesture.setTransLimit(slideBounds, windowRect, fDefaultMatrix); this->updateTitle(); this->updateUIState(); @@ -653,35 +644,18 @@ void Viewer::setupCurrentSlide(int previousSlide) { void Viewer::changeZoomLevel(float delta) { fZoomLevel += delta; - if (fZoomLevel > 0) { - fZoomLevel = SkMinScalar(fZoomLevel, MAX_ZOOM_LEVEL); - fZoomScale = fZoomLevel + SK_Scalar1; - } else if (fZoomLevel < 0) { - fZoomLevel = SkMaxScalar(fZoomLevel, MIN_ZOOM_LEVEL); - fZoomScale = SK_Scalar1 / (SK_Scalar1 - fZoomLevel); - } else { - fZoomScale = SK_Scalar1; - } + fZoomLevel = SkScalarPin(fZoomLevel, MIN_ZOOM_LEVEL, MAX_ZOOM_LEVEL); } SkMatrix Viewer::computeMatrix() { SkMatrix m; - m.reset(); - if (fZoomLevel) { - SkPoint center; - //m = this->getLocalMatrix();//.invert(&m); - m.mapXY(fZoomCenterX, fZoomCenterY, ¢er); - SkScalar cx = center.fX; - SkScalar cy = center.fY; - - m.setTranslate(-cx, -cy); - m.postScale(fZoomScale, fZoomScale); - m.postTranslate(cx, cy); - } - - m.preConcat(fGesture.localM()); + SkScalar zoomScale = (fZoomLevel < 0) ? SK_Scalar1 / (SK_Scalar1 - fZoomLevel) + : SK_Scalar1 + fZoomLevel; + m = fGesture.localM(); m.preConcat(fGesture.globalM()); + m.preConcat(fDefaultMatrix); + m.preScale(zoomScale, zoomScale); return m; } @@ -737,12 +711,6 @@ void Viewer::setColorMode(ColorMode colorMode) { void Viewer::drawSlide(SkCanvas* canvas) { SkAutoCanvasRestore autorestore(canvas, false); - if (fWindow->supportsContentRect()) { - SkRect contentRect = fWindow->getContentRect(); - canvas->clipRect(contentRect); - canvas->translate(contentRect.fLeft, contentRect.fTop); - } - // By default, we render directly into the window's surface/canvas SkCanvas* slideCanvas = canvas; fLastImage.reset(); @@ -785,7 +753,6 @@ void Viewer::drawSlide(SkCanvas* canvas) { int count = slideCanvas->save(); slideCanvas->clear(SK_ColorWHITE); - slideCanvas->concat(fDefaultMatrix); slideCanvas->concat(computeMatrix()); // Time the painting logic of the slide double startTime = SkTime::GetMSecs(); @@ -853,18 +820,17 @@ void Viewer::onPaint(SkCanvas* canvas) { bool Viewer::onTouch(intptr_t owner, Window::InputState state, float x, float y) { void* castedOwner = reinterpret_cast(owner); - SkPoint touchPoint = fDefaultMatrixInv.mapXY(x, y); switch (state) { case Window::kUp_InputState: { fGesture.touchEnd(castedOwner); break; } case Window::kDown_InputState: { - fGesture.touchBegin(castedOwner, touchPoint.fX, touchPoint.fY); + fGesture.touchBegin(castedOwner, x, y); break; } case Window::kMove_InputState: { - fGesture.touchMoved(castedOwner, touchPoint.fX, touchPoint.fY); + fGesture.touchMoved(castedOwner, x, y); break; } } @@ -873,19 +839,17 @@ bool Viewer::onTouch(intptr_t owner, Window::InputState state, float x, float y) } bool Viewer::onMouse(float x, float y, Window::InputState state, uint32_t modifiers) { - - SkPoint touchPoint = fDefaultMatrixInv.mapXY(x, y); switch (state) { case Window::kUp_InputState: { fGesture.touchEnd(nullptr); break; } case Window::kDown_InputState: { - fGesture.touchBegin(nullptr, touchPoint.fX, touchPoint.fY); + fGesture.touchBegin(nullptr, x, y); break; } case Window::kMove_InputState: { - fGesture.touchMoved(nullptr, touchPoint.fX, touchPoint.fY); + fGesture.touchMoved(nullptr, x, y); break; } } @@ -908,12 +872,6 @@ void Viewer::drawStats(SkCanvas* canvas) { SkPaint paint; canvas->save(); - if (fWindow->supportsContentRect()) { - SkRect contentRect = fWindow->getContentRect(); - canvas->clipRect(contentRect); - canvas->translate(contentRect.fLeft, contentRect.fTop); - } - canvas->clipRect(rect); paint.setColor(SK_ColorBLACK); canvas->drawRect(rect, paint); diff --git a/tools/viewer/Viewer.h b/tools/viewer/Viewer.h index 09a8946ac9..fac280ff59 100644 --- a/tools/viewer/Viewer.h +++ b/tools/viewer/Viewer.h @@ -87,10 +87,7 @@ private: SkColorSpacePrimaries fColorSpacePrimaries; // transform data - SkScalar fZoomCenterX; - SkScalar fZoomCenterY; SkScalar fZoomLevel; - SkScalar fZoomScale; sk_app::CommandSet fCommands; @@ -98,7 +95,6 @@ private: // identity unless the window initially scales the content to fit the screen. SkMatrix fDefaultMatrix; - SkMatrix fDefaultMatrixInv; SkTArray> fDeferredActions; diff --git a/tools/viewer/sk_app/Window.h b/tools/viewer/sk_app/Window.h index c5d8c346af..752dc47548 100644 --- a/tools/viewer/sk_app/Window.h +++ b/tools/viewer/sk_app/Window.h @@ -38,8 +38,6 @@ public: void inval(); virtual bool scaleContentToFit() const { return false; } - virtual bool supportsContentRect() const { return false; } - virtual SkRect getContentRect() { return SkRect::MakeEmpty(); } enum BackendType { kNativeGL_BackendType, diff --git a/tools/viewer/sk_app/android/Window_android.h b/tools/viewer/sk_app/android/Window_android.h index 062f2fec2d..791801140e 100644 --- a/tools/viewer/sk_app/android/Window_android.h +++ b/tools/viewer/sk_app/android/Window_android.h @@ -32,13 +32,9 @@ public: void paintIfNeeded(); bool scaleContentToFit() const override { return true; } - bool supportsContentRect() const override { return true; } - SkRect getContentRect() override { return fContentRect; } - void setContentRect(int l, int t, int r, int b) { fContentRect.set(l,t,r,b); } private: SkiaAndroidApp* fSkiaAndroidApp = nullptr; - SkRect fContentRect; BackendType fBackendType; }; diff --git a/tools/viewer/sk_app/android/surface_glue_android.cpp b/tools/viewer/sk_app/android/surface_glue_android.cpp index 178f04d074..4fb6c3d9ac 100644 --- a/tools/viewer/sk_app/android/surface_glue_android.cpp +++ b/tools/viewer/sk_app/android/surface_glue_android.cpp @@ -123,7 +123,6 @@ int SkiaAndroidApp::message_callback(int fd, int events, void* data) { window_android->initDisplay(skiaAndroidApp->fNativeWindow); } window_android->onResize(width, height); - window_android->setContentRect(0, 0, width, height); window_android->paintIfNeeded(); break; }