From 949df4da16db563840d17d5287cb0a685fbfc544 Mon Sep 17 00:00:00 2001 From: Israel Brewster Date: Sat, 30 May 2020 12:09:09 -0800 Subject: [PATCH] Fix aspectRatio and zoom range issues when zooming (#1093) * Check and enforce view limits in the setRange function * Check limits when setting aspectRatio - This change is required due to moving the limit checking out of the updateViewRange function. - If the original logic remained, aspect ratio could be lost due to "squshing" the requested view into the viewBox * Add tests for ViewBox zooming limits and aspect ratio * - Move test code to proper location and fix instantiation of QApplication Co-authored-by: Israel Brewster --- pyqtgraph/graphicsItems/ViewBox/ViewBox.py | 176 ++++++++------- .../ViewBox/tests/test_ViewBoxZoom.py | 200 ++++++++++++++++++ 2 files changed, 297 insertions(+), 79 deletions(-) create mode 100644 pyqtgraph/graphicsItems/ViewBox/tests/test_ViewBoxZoom.py diff --git a/pyqtgraph/graphicsItems/ViewBox/ViewBox.py b/pyqtgraph/graphicsItems/ViewBox/ViewBox.py index bf2bb5b5..94aa6243 100644 --- a/pyqtgraph/graphicsItems/ViewBox/ViewBox.py +++ b/pyqtgraph/graphicsItems/ViewBox/ViewBox.py @@ -282,7 +282,7 @@ class ViewBox(GraphicsWidget): #if scene is not None and hasattr(scene, 'sigPrepareForPaint'): #scene.sigPrepareForPaint.connect(self.prepareForPaint) #return ret - + def update(self, *args, **kwargs): self.prepareForPaint() GraphicsWidget.update(self, *args, **kwargs) @@ -398,12 +398,12 @@ class ViewBox(GraphicsWidget): """ if item.zValue() < self.zValue(): item.setZValue(self.zValue()+1) - + scene = self.scene() if scene is not None and scene is not item.scene(): scene.addItem(item) ## Necessary due to Qt bug: https://bugreports.qt-project.org/browse/QTBUG-18616 item.setParentItem(self.childGroup) - + if not ignoreBounds: self.addedItems.append(item) self.updateAutoRange() @@ -414,12 +414,12 @@ class ViewBox(GraphicsWidget): self.addedItems.remove(item) except: pass - + scene = self.scene() if scene is not None: scene.removeItem(item) item.setParentItem(None) - + self.updateAutoRange() def clear(self): @@ -431,19 +431,19 @@ class ViewBox(GraphicsWidget): def resizeEvent(self, ev): self._matrixNeedsUpdate = True self.updateMatrix() - + self.linkedXChanged() self.linkedYChanged() - + self.updateAutoRange() self.updateViewRange() - + self._matrixNeedsUpdate = True self.updateMatrix() - + self.background.setRect(self.rect()) self.borderRect.setRect(self.rect()) - + self.sigStateChanged.emit(self) self.sigResized.emit(self) self.childGroup.prepareGeometryChange() @@ -536,7 +536,11 @@ class ViewBox(GraphicsWidget): yOff = False if setRequested[1] else None self.enableAutoRange(x=xOff, y=yOff) changed.append(True) - + + limits = (self.state['limits']['xLimits'], self.state['limits']['yLimits']) + minRng = [self.state['limits']['xRange'][0], self.state['limits']['yRange'][0]] + maxRng = [self.state['limits']['xRange'][1], self.state['limits']['yRange'][1]] + for ax, range in changes.items(): mn = min(range) mx = max(range) @@ -564,6 +568,39 @@ class ViewBox(GraphicsWidget): mn -= p mx += p + # max range cannot be larger than bounds, if they are given + if limits[ax][0] is not None and limits[ax][1] is not None: + if maxRng[ax] is not None: + maxRng[ax] = min(maxRng[ax], limits[ax][1] - limits[ax][0]) + else: + maxRng[ax] = limits[ax][1] - limits[ax][0] + + # If we have limits, we will have at least a max range as well + if maxRng[ax] is not None or minRng[ax] is not None: + diff = mx - mn + if maxRng[ax] is not None and diff > maxRng[ax]: + delta = maxRng[ax] - diff + elif minRng[ax] is not None and diff < minRng[ax]: + delta = minRng[ax] - diff + else: + delta = 0 + + mn -= delta / 2. + mx += delta / 2. + + # Make sure our requested area is within limits, if any + if limits[ax][0] is not None or limits[ax][1] is not None: + lmn, lmx = limits[ax] + if lmn is not None and mn < lmn: + delta = lmn - mn # Shift the requested view to match our lower limit + mn = lmn + mx += delta + elif lmx is not None and mx > lmx: + delta = lmx - mx + mx = lmx + mn += delta + + # Set target range if self.state['targetRange'][ax] != [mn, mx]: self.state['targetRange'][ax] = [mn, mx] @@ -1443,40 +1480,6 @@ class ViewBox(GraphicsWidget): aspect = self.state['aspectLocked'] # size ratio / view ratio tr = self.targetRect() bounds = self.rect() - if aspect is not False and 0 not in [aspect, tr.height(), bounds.height(), bounds.width()]: - - ## This is the view range aspect ratio we have requested - targetRatio = tr.width() / tr.height() if tr.height() != 0 else 1 - ## This is the view range aspect ratio we need to obey aspect constraint - viewRatio = (bounds.width() / bounds.height() if bounds.height() != 0 else 1) / aspect - viewRatio = 1 if viewRatio == 0 else viewRatio - - # Decide which range to keep unchanged - #print self.name, "aspect:", aspect, "changed:", changed, "auto:", self.state['autoRange'] - if forceX: - ax = 0 - elif forceY: - ax = 1 - else: - # if we are not required to keep a particular axis unchanged, - # then make the entire target range visible - ax = 0 if targetRatio > viewRatio else 1 - - if ax == 0: - ## view range needs to be taller than target - dy = 0.5 * (tr.width() / viewRatio - tr.height()) - if dy != 0: - changed[1] = True - viewRange[1] = [self.state['targetRange'][1][0] - dy, self.state['targetRange'][1][1] + dy] - else: - ## view range needs to be wider than target - dx = 0.5 * (tr.height() * viewRatio - tr.width()) - if dx != 0: - changed[0] = True - viewRange[0] = [self.state['targetRange'][0][0] - dx, self.state['targetRange'][0][1] + dx] - - - # ----------- Make corrections for view limits ----------- limits = (self.state['limits']['xLimits'], self.state['limits']['yLimits']) minRng = [self.state['limits']['xRange'][0], self.state['limits']['yRange'][0]] @@ -1489,43 +1492,58 @@ class ViewBox(GraphicsWidget): # max range cannot be larger than bounds, if they are given if limits[axis][0] is not None and limits[axis][1] is not None: if maxRng[axis] is not None: - maxRng[axis] = min(maxRng[axis], limits[axis][1]-limits[axis][0]) + maxRng[axis] = min(maxRng[axis], limits[axis][1] - limits[axis][0]) else: - maxRng[axis] = limits[axis][1]-limits[axis][0] + maxRng[axis] = limits[axis][1] - limits[axis][0] - #print "\nLimits for axis %d: range=%s min=%s max=%s" % (axis, limits[axis], minRng[axis], maxRng[axis]) - #print "Starting range:", viewRange[axis] + if aspect is not False and 0 not in [aspect, tr.height(), bounds.height(), bounds.width()]: - # Apply xRange, yRange - diff = viewRange[axis][1] - viewRange[axis][0] - if maxRng[axis] is not None and diff > maxRng[axis]: - delta = maxRng[axis] - diff - changed[axis] = True - elif minRng[axis] is not None and diff < minRng[axis]: - delta = minRng[axis] - diff - changed[axis] = True + ## This is the view range aspect ratio we have requested + targetRatio = tr.width() / tr.height() if tr.height() != 0 else 1 + ## This is the view range aspect ratio we need to obey aspect constraint + viewRatio = (bounds.width() / bounds.height() if bounds.height() != 0 else 1) / aspect + viewRatio = 1 if viewRatio == 0 else viewRatio + + # Calculate both the x and y ranges that would be needed to obtain the desired aspect ratio + dy = 0.5 * (tr.width() / viewRatio - tr.height()) + dx = 0.5 * (tr.height() * viewRatio - tr.width()) + + rangeY = [self.state['targetRange'][1][0] - dy, self.state['targetRange'][1][1] + dy] + rangeX = [self.state['targetRange'][0][0] - dx, self.state['targetRange'][0][1] + dx] + + canidateRange = [rangeX, rangeY] + + # Decide which range to try to keep unchanged + #print self.name, "aspect:", aspect, "changed:", changed, "auto:", self.state['autoRange'] + if forceX: + ax = 0 + elif forceY: + ax = 1 else: - delta = 0 + # if we are not required to keep a particular axis unchanged, + # then try to make the entire target range visible + ax = 0 if targetRatio > viewRatio else 1 + target = 0 if ax == 1 else 1 + # See if this choice would cause out-of-range issues + if maxRng is not None or minRng is not None: + diff = canidateRange[target][1] - canidateRange[target][0] + if maxRng[target] is not None and diff > maxRng[target] or \ + minRng[target] is not None and diff < minRng[target]: + # tweak the target range down so we can still pan properly + self.state['targetRange'][ax] = canidateRange[ax] + ax = target # Switch the "fixed" axes - viewRange[axis][0] -= delta/2. - viewRange[axis][1] += delta/2. + if ax == 0: + ## view range needs to be taller than target + if dy != 0: + changed[1] = True + viewRange[1] = rangeY + else: + ## view range needs to be wider than target + if dx != 0: + changed[0] = True + viewRange[0] = rangeX - #print "after applying min/max:", viewRange[axis] - - # Apply xLimits, yLimits - mn, mx = limits[axis] - if mn is not None and viewRange[axis][0] < mn: - delta = mn - viewRange[axis][0] - viewRange[axis][0] += delta - viewRange[axis][1] += delta - changed[axis] = True - elif mx is not None and viewRange[axis][1] > mx: - delta = mx - viewRange[axis][1] - viewRange[axis][0] += delta - viewRange[axis][1] += delta - changed[axis] = True - - #print "after applying edge limits:", viewRange[axis] changed = [(viewRange[i][0] != self.state['viewRange'][i][0]) or (viewRange[i][1] != self.state['viewRange'][i][1]) for i in (0,1)] self.state['viewRange'] = viewRange @@ -1605,13 +1623,13 @@ class ViewBox(GraphicsWidget): self.window() except RuntimeError: ## this view has already been deleted; it will probably be collected shortly. return - + def view_key(view): return (view.window() is self.window(), view.name) - + ## make a sorted list of all named views nv = sorted(ViewBox.NamedViews.values(), key=view_key) - + if self in nv: nv.remove(self) diff --git a/pyqtgraph/graphicsItems/ViewBox/tests/test_ViewBoxZoom.py b/pyqtgraph/graphicsItems/ViewBox/tests/test_ViewBoxZoom.py new file mode 100644 index 00000000..4bad9ee1 --- /dev/null +++ b/pyqtgraph/graphicsItems/ViewBox/tests/test_ViewBoxZoom.py @@ -0,0 +1,200 @@ +# -*- coding: utf-8 -*- +import pyqtgraph as pg +import pytest + +app = pg.mkQApp() + +def test_zoom_normal(): + vb = pg.ViewBox() + testRange = pg.QtCore.QRect(0, 0, 10, 20) + vb.setRange(testRange, padding=0) + vbViewRange = vb.getState()['viewRange'] + assert vbViewRange == [[testRange.left(), testRange.right()], + [testRange.top(), testRange.bottom()]] + +def test_zoom_limit(): + """Test zooming with X and Y limits set""" + vb = pg.ViewBox() + vb.setLimits(xMin=0, xMax=10, yMin=0, yMax=10) + + # Try zooming within limits. Should return unmodified + testRange = pg.QtCore.QRect(0, 0, 9, 9) + vb.setRange(testRange, padding=0) + vbViewRange = vb.getState()['viewRange'] + assert vbViewRange == [[testRange.left(), testRange.right()], + [testRange.top(), testRange.bottom()]] + + # And outside limits. both view range and targetRange should be set to limits + testRange = pg.QtCore.QRect(-5, -5, 16, 20) + vb.setRange(testRange, padding=0) + + expected = [[0, 10], [0, 10]] + vbState = vb.getState() + + assert vbState['targetRange'] == expected + assert vbState['viewRange'] == expected + +def test_zoom_range_limit(): + """Test zooming with XRange and YRange limits set, but no X and Y limits""" + vb = pg.ViewBox() + vb.setLimits(minXRange=5, maxXRange=10, minYRange=5, maxYRange=10) + + # Try something within limits + testRange = pg.QtCore.QRect(-15, -15, 7, 7) + vb.setRange(testRange, padding=0) + + expected = [[testRange.left(), testRange.right()], + [testRange.top(), testRange.bottom()]] + + vbViewRange = vb.getState()['viewRange'] + assert vbViewRange == expected + + # and outside limits + testRange = pg.QtCore.QRect(-15, -15, 17, 17) + + # Code should center the required width reduction, so move each side by 3 + expected = [[testRange.left() + 3, testRange.right() - 3], + [testRange.top() + 3, testRange.bottom() - 3]] + + vb.setRange(testRange, padding=0) + vbViewRange = vb.getState()['viewRange'] + vbTargetRange = vb.getState()['targetRange'] + + assert vbViewRange == expected + assert vbTargetRange == expected + +def test_zoom_ratio(): + """Test zooming with a fixed aspect ratio set""" + vb = pg.ViewBox(lockAspect=1) + + # Give the viewbox a size of the proper aspect ratio to keep things easy + vb.setFixedHeight(10) + vb.setFixedWidth(10) + + # request a range with a good ratio + testRange = pg.QtCore.QRect(0, 0, 10, 10) + vb.setRange(testRange, padding=0) + expected = [[testRange.left(), testRange.right()], + [testRange.top(), testRange.bottom()]] + + viewRange = vb.getState()['viewRange'] + viewWidth = viewRange[0][1] - viewRange[0][0] + viewHeight = viewRange[1][1] - viewRange[1][0] + + # Assert that the width and height are equal, since we locked the aspect ratio at 1 + assert viewWidth == viewHeight + + # and for good measure, that it is the same as the test range + assert viewRange == expected + + # Now try to set to something with a different aspect ratio + testRange = pg.QtCore.QRect(0, 0, 10, 20) + vb.setRange(testRange, padding=0) + + viewRange = vb.getState()['viewRange'] + viewWidth = viewRange[0][1] - viewRange[0][0] + viewHeight = viewRange[1][1] - viewRange[1][0] + + # Don't really care what we got here, as long as the width and height are the same + assert viewWidth == viewHeight + +def test_zoom_ratio2(): + """Slightly more complicated zoom ratio test, where the view box shape does not match the ratio""" + vb = pg.ViewBox(lockAspect=1) + + # twice as wide as tall + vb.setFixedHeight(10) + vb.setFixedWidth(20) + + # more or less random requested range + testRange = pg.QtCore.QRect(0, 0, 10, 15) + vb.setRange(testRange, padding=0) + + viewRange = vb.getState()['viewRange'] + viewWidth = viewRange[0][1] - viewRange[0][0] + viewHeight = viewRange[1][1] - viewRange[1][0] + + # View width should be twice as wide as the height, + # since the viewbox is twice as wide as it is tall. + assert viewWidth == 2 * viewHeight + +def test_zoom_ratio_with_limits1(): + """Test zoom with both ratio and limits set""" + vb = pg.ViewBox(lockAspect=1) + + # twice as wide as tall + vb.setFixedHeight(10) + vb.setFixedWidth(20) + + # set some limits + vb.setLimits(xMin=-5, xMax=5, yMin=-5, yMax=5) + + # Try to zoom too tall + testRange = pg.QtCore.QRect(0, 0, 6, 10) + vb.setRange(testRange, padding=0) + + viewRange = vb.getState()['viewRange'] + viewWidth = viewRange[0][1] - viewRange[0][0] + viewHeight = viewRange[1][1] - viewRange[1][0] + + # Make sure our view is within limits and the proper aspect ratio + assert viewRange[0][0] >= -5 + assert viewRange[0][1] <= 5 + assert viewRange[1][0] >= -5 + assert viewRange[1][1] <= 5 + assert viewWidth == 2 * viewHeight + +def test_zoom_ratio_with_limits2(): + vb = pg.ViewBox(lockAspect=1) + + # twice as wide as tall + vb.setFixedHeight(10) + vb.setFixedWidth(20) + + # set some limits + vb.setLimits(xMin=-5, xMax=5, yMin=-5, yMax=5) + + # Same thing, but out-of-range the other way + testRange = pg.QtCore.QRect(0, 0, 16, 6) + vb.setRange(testRange, padding=0) + + viewRange = vb.getState()['viewRange'] + viewWidth = viewRange[0][1] - viewRange[0][0] + viewHeight = viewRange[1][1] - viewRange[1][0] + + # Make sure our view is within limits and the proper aspect ratio + assert viewRange[0][0] >= -5 + assert viewRange[0][1] <= 5 + assert viewRange[1][0] >= -5 + assert viewRange[1][1] <= 5 + assert viewWidth == 2 * viewHeight + +def test_zoom_ratio_with_limits_out_of_range(): + vb = pg.ViewBox(lockAspect=1) + + # twice as wide as tall + vb.setFixedHeight(10) + vb.setFixedWidth(20) + + # set some limits + vb.setLimits(xMin=-5, xMax=5, yMin=-5, yMax=5) + + # Request something completely out-of-range and out-of-aspect + testRange = pg.QtCore.QRect(10, 10, 25, 100) + vb.setRange(testRange, padding=0) + + viewRange = vb.getState()['viewRange'] + viewWidth = viewRange[0][1] - viewRange[0][0] + viewHeight = viewRange[1][1] - viewRange[1][0] + + # Make sure our view is within limits and the proper aspect ratio + assert viewRange[0][0] >= -5 + assert viewRange[0][1] <= 5 + assert viewRange[1][0] >= -5 + assert viewRange[1][1] <= 5 + assert viewWidth == 2 * viewHeight + + +if __name__ == "__main__": + setup_module(None) + test_zoom_ratio()