diff --git a/android/src/toga_android/widgets/canvas.py b/android/src/toga_android/widgets/canvas.py index b37fef5325..b30c8261ca 100644 --- a/android/src/toga_android/widgets/canvas.py +++ b/android/src/toga_android/widgets/canvas.py @@ -30,9 +30,10 @@ class State(NamedTuple): fill: Paint stroke: Paint + transform: Matrix def __deepcopy__(self, memo): - return type(self)(Paint(self.fill), Paint(self.stroke)) + return type(self)(Paint(self.fill), Paint(self.stroke), Matrix()) class Context: @@ -40,7 +41,6 @@ def __init__(self, impl, native): self.native = native self.impl = impl self.path = Path() - self.reset_transform() # Backwards compatibility for Toga <= 0.5.3 self.in_fill = False @@ -57,7 +57,8 @@ def __init__(self, impl, native): stroke.setStrokeWidth(2.0) stroke.setColor(BLACK) - self.states = [State(fill, stroke)] + self.states = [State(fill, stroke, Matrix())] + self.reset_transform() @property def state(self): @@ -71,6 +72,8 @@ def save(self): def restore(self): self.native.restore() + # Transform active path to current coordinates + self.path.transform(self.state.transform) self.states.pop() # Setting attributes @@ -171,14 +174,57 @@ def stroke(self): def rotate(self, radians): self.native.rotate(degrees(radians)) + # Update state transform + self.state.transform.postRotate(degrees(radians)) + + # Transform active path to current coordinates + inverse = Matrix() + inverse.setRotate(-degrees(radians)) + self.path.transform(inverse) + def scale(self, sx, sy): + # Can't apply inverse transform if scale is 0, + # so use a small epsilon which will almost be the same + if sx == 0: + sx = 2**-24 + if sy == 0: + sy = 2**-24 + self.native.scale(sx, sy) + # Update state transform + self.state.transform.postScale(sx, sy) + + # Transform active path to current coordinates + inverse = Matrix() + inverse.setScale(1 / sx, 1 / sy) + self.path.transform(inverse) + def translate(self, tx, ty): self.native.translate(tx, ty) + # Update state transform + self.state.transform.postTranslate(tx, ty) + + # Transform active path to current coordinates + inverse = Matrix() + inverse.setTranslate(-tx, -ty) + self.path.transform(inverse) + def reset_transform(self): self.native.setMatrix(None) + + # current matrix needs to unwind all previous states + # can't just ask for current total transform as `getMatrix` is deprecated + for state in reversed(self.states): + self.path.transform(state.transform) + inverse = Matrix() + # if we can't invert, ignore for now + if state.transform.invert(inverse): # pragma: no branch + # Update current state transform + self.state.transform.postConcat(inverse) + + # Rescale to standard units self.scale(self.impl.dpi_scale, self.impl.dpi_scale) # Text diff --git a/changes/2206.bugfix.md b/changes/2206.bugfix.md new file mode 100644 index 0000000000..239dd78002 --- /dev/null +++ b/changes/2206.bugfix.md @@ -0,0 +1 @@ +The Android, Qt and Winforms canvases now match the behavior of the HTML Canvas when a transform is applied while preparing a path. diff --git a/changes/4110.removal.md b/changes/4110.removal.md new file mode 100644 index 0000000000..b81369da4f --- /dev/null +++ b/changes/4110.removal.md @@ -0,0 +1 @@ +The behaviour of the Canvas widget when a drawing was scaled by a factor of 0 in either axis was unspecified and varied across backends and HTML Canvas implementations. The Canvas widget now follows a consistent, reasonable behavior when this happens which should produce similar results on all backends. On some backends, to avoid errors or other drawing problems, a very small scale factor is used instead of 0 when asked to scale by 0. diff --git a/gtk/src/toga_gtk/widgets/canvas.py b/gtk/src/toga_gtk/widgets/canvas.py index 4f5f46bd37..892514ad70 100644 --- a/gtk/src/toga_gtk/widgets/canvas.py +++ b/gtk/src/toga_gtk/widgets/canvas.py @@ -155,6 +155,13 @@ def rotate(self, radians): self.native.rotate(radians) def scale(self, sx, sy): + # Cairo throws an exception if scale is 0, + # so use a small epsilon which will almost be the same + if sx == 0: + sx = 2**-24 + if sy == 0: + sy = 2**-24 + self.native.scale(sx, sy) def translate(self, tx, ty): diff --git a/qt/src/toga_qt/widgets/canvas.py b/qt/src/toga_qt/widgets/canvas.py index e70a37208d..ad14f97f7f 100644 --- a/qt/src/toga_qt/widgets/canvas.py +++ b/qt/src/toga_qt/widgets/canvas.py @@ -28,9 +28,10 @@ class State: - """Doesn't need to track transform, only fill/stroke-related properties.""" + """Track transform and fill/stroke-related properties.""" def __init__(self, former_state=None): + self.transform = QTransform() if former_state: self.fill_style = former_state.fill_style self.stroke = QPen(former_state.stroke) @@ -63,6 +64,8 @@ def save(self): self.native.save() def restore(self): + # Transform active path to current coordinates + self._path = self.state.transform.map(self._path) self.states.pop() self.native.restore() @@ -183,15 +186,52 @@ def stroke(self): def rotate(self, radians): self.native.rotate(degrees(radians)) + # Update state transform + self.state.transform.rotateRadians(radians) + + # Transform active path to current coordinates + inverse = QTransform() + inverse.rotateRadians(-radians) + self._path = inverse.map(self._path) + def scale(self, sx, sy): + # Can't apply inverse transform if scale is 0, + # so use a small epsilon which will almost be the same + if sx == 0: + sx = 2**-24 + if sy == 0: + sy = 2**-24 + self.native.scale(sx, sy) + # Update state transform + self.state.transform.scale(sx, sy) + + # Transform active path to current coordinates + inverse = QTransform() + inverse.scale(1 / sx, 1 / sy) + self._path = inverse.map(self._path) + def translate(self, tx, ty): self.native.translate(tx, ty) + # Update state transform + self.state.transform.translate(tx, ty) + + # Transform active path to current coordinates + inverse = QTransform() + inverse.translate(-tx, -ty) + self._path = inverse.map(self._path) + def reset_transform(self): + transform = self.native.transform() self.native.resetTransform() + # Update state transform + inverse, _ = transform.inverted() + self._path = transform.map(self._path) + self.state.transform *= inverse + # Text def write_text(self, text, x, y, font, baseline, line_height): left_offset, top_offset, _, bottom_offset, scaled_line_height = ( diff --git a/testbed/src/testbed/resources/canvas/singular_transforms.png b/testbed/src/testbed/resources/canvas/singular_transforms.png new file mode 100644 index 0000000000..60cd8ad9f5 Binary files /dev/null and b/testbed/src/testbed/resources/canvas/singular_transforms.png differ diff --git a/testbed/src/testbed/resources/canvas/transforms_mid_path.png b/testbed/src/testbed/resources/canvas/transforms_mid_path.png new file mode 100644 index 0000000000..1c9aa790c7 Binary files /dev/null and b/testbed/src/testbed/resources/canvas/transforms_mid_path.png differ diff --git a/testbed/tests/widgets/test_canvas.py b/testbed/tests/widgets/test_canvas.py index 70156c7f8a..351b8ae924 100644 --- a/testbed/tests/widgets/test_canvas.py +++ b/testbed/tests/widgets/test_canvas.py @@ -697,6 +697,96 @@ async def test_transforms(canvas, probe): assert_reference(probe, "transforms") +async def test_transforms_mid_path(canvas, probe): + "Transforms can be applied mid-path" + + # draw a series of rotated rectangles + canvas.root_state.begin_path() + canvas.root_state.translate(100, 100) + with canvas.root_state.state() as ctx: + for _ in range(12): + ctx.rect(50, 0, 10, 10) + ctx.scale(1.1, 1) + ctx.rotate(math.pi / 6) + + canvas.root_state.fill() + canvas.root_state.stroke(GOLDENROD) + + # draw a series of line segments + canvas.root_state.begin_path() + canvas.root_state.move_to(25, 0) + for _ in range(12): + canvas.root_state.line_to(25, 0) + canvas.root_state.rotate(math.pi / 6) + canvas.root_state.translate(5, 3) + canvas.root_state.close_path() + canvas.root_state.reset_transform() + canvas.root_state.move_to(110, 100) + canvas.root_state.scale(5, 1) + canvas.root_state.ellipse(20, 100, 2, 20, 0, 0, 2 * pi) + canvas.root_state.stroke(CORNFLOWERBLUE) + + await probe.redraw("Transforms can be applied") + assert_reference(probe, "transforms_mid_path", threshold=0.015) + + +async def test_singular_transforms(canvas, probe): + "Singular transforms behave reasonably" + ctx = canvas.root_state + + ctx.begin_path() + with ctx.state() as ctx2: + # flip about the line x = y + ctx2.rotate(-pi / 2) + ctx2.scale(-1, 1) + + ctx2.move_to(40, 20) + ctx2.line_to(80, 20) + ctx2.line_to(100, 30) + + with ctx2.state() as ctx3: + # Apply a scale factor of zero + ctx3.scale(0.9, 0) + ctx3.line_to(180, 20) + + ctx2.rotate(pi / 4) + ctx2.line_to(180, 20) + + ctx2.stroke(GOLDENROD, line_width=8) + + # Same shape, but not flipped, using reset_transform() + ctx.begin_path() + + ctx.move_to(40, 20) + ctx.line_to(80, 20) + ctx.line_to(100, 30) + + # Apply a scale factor of zero + ctx.scale(0.9, 0) + ctx.line_to(180, 20) + # Total transform is singular + ctx.reset_transform() + + ctx.rotate(pi / 4) + ctx.line_to(180, 20) + + ctx.stroke(CORNFLOWERBLUE, line_width=8) + + ctx.reset_transform() + ctx.begin_path() + ctx.scale(0, 0.9) + ctx.translate(50, 50) + + ctx.rect(0, 0, 25, 25) + + # Should draw nothing. + ctx.fill() + ctx.stroke(line_width=10) + + await probe.redraw("Transforms can be applied") + assert_reference(probe, "singular_transforms") + + @pytest.mark.xfail( condition=os.environ.get("RUNNING_IN_CI") != "true", reason="Canvas tests are unstable outside of CI. Manual inspection may be required", diff --git a/winforms/src/toga_winforms/widgets/canvas.py b/winforms/src/toga_winforms/widgets/canvas.py index 93a1ea3049..ac6f0cf9c9 100644 --- a/winforms/src/toga_winforms/widgets/canvas.py +++ b/winforms/src/toga_winforms/widgets/canvas.py @@ -1,4 +1,3 @@ -from copy import deepcopy from math import degrees import System.Windows.Forms as WinForms @@ -42,24 +41,29 @@ class State: we used it. And it would still need to be kept in a list. """ - def __init__(self, matrix, brush, pen): - self.matrix = matrix + def __init__(self, previous_state, brush, pen, singular=False): + # This is the previous graphics state, so we can restore. + self.previous_state = previous_state self.brush = brush self.pen = pen + # When we are in a singular state, should not draw anything + self.singular = singular + self.transform = Matrix() @classmethod def for_impl(cls, impl): return cls( - matrix=Matrix(), + previous_state=None, brush=SolidBrush(BLACK), pen=Pen(BLACK, impl.scale_in(2.0, rounding=None)), ) - def __deepcopy__(self, memo): + def new_state(self, previous_state): return type(self)( - matrix=self.matrix.Clone(), + previous_state=previous_state, brush=self.brush.Clone(), pen=self.pen.Clone(), + singular=self.singular, ) @@ -99,6 +103,15 @@ def get_last_point(self, default_x, default_y): else: return PointF(default_x, default_y) + def transform_path(self, matrix): + """Transform the current path using a matrix.""" + for path in self.paths: + path.Transform(matrix) + if self.start_point: + points = Array[PointF]([self.start_point]) + matrix.TransformPoints(points) + self.start_point = points[0] + # Context management @property @@ -106,10 +119,13 @@ def state(self): return self.states[-1] def save(self): - self.states.append(deepcopy(self.state)) + graphics_state = self.native.Save() + self.states.append(self.state.new_state(graphics_state)) def restore(self): - self.states.pop() + state = self.states.pop() + self.native.Restore(state.previous_state) + self.transform_path(state.transform) # Setting attributes @@ -218,32 +234,79 @@ def rect(self, x, y, width, height): # Drawing Paths def fill(self, fill_rule): + if self.state.singular: + # draw nothing + return for path in self.paths: if fill_rule == FillRule.EVENODD: path.FillMode = FillMode.Alternate else: # Default to NONZERO path.FillMode = FillMode.Winding - path.Transform(self.state.matrix) self.native.FillPath(self.state.brush, path) def stroke(self): + if self.state.singular: + # draw nothing + return for path in self.paths: - path.Transform(self.state.matrix) self.native.DrawPath(self.state.pen, path) # Transformations def rotate(self, radians): - self.state.matrix.Rotate(degrees(radians)) + self.native.RotateTransform(degrees(radians)) + + # Update state transform + self.state.transform.Rotate(degrees(radians)) + + # Transform active path to current coordinates + inverse = Matrix() + inverse.Rotate(-degrees(radians)) + self.transform_path(inverse) def scale(self, sx, sy): - self.state.matrix.Scale(sx, sy) + # Can't apply inverse transform if scale is 0, + # so use a small epsilon which will almost be the same + if sx == 0: + sx = 2**-24 + self.state.singular = True + if sy == 0: + sy = 2**-24 + self.state.singular = True + + self.native.ScaleTransform(sx, sy) + + # Update state transform + self.state.transform.Scale(sx, sy) + + # Transform active path to current coordinates + inverse = Matrix() + inverse.Scale(1 / sx, 1 / sy) + self.transform_path(inverse) def translate(self, tx, ty): - self.state.matrix.Translate(tx, ty) + self.native.TranslateTransform(tx, ty) + + # Update state transform + self.state.transform.Translate(tx, ty) + + # Transform active path to current coordinates + inverse = Matrix() + inverse.Translate(-tx, -ty) + self.transform_path(inverse) def reset_transform(self): - self.state.matrix.Reset() + matrix = self.native.Transform + self.native.ResetTransform() + + # Transform active path to current coordinates + self.transform_path(matrix) + + # Update state transform + matrix.Invert() + self.state.transform.Multiply(matrix) + + self.state.singular = False self.scale(self.impl.dpi_scale, self.impl.dpi_scale) # Text @@ -287,10 +350,7 @@ def _text_path(self, text, x, y, font, baseline, line_height): ) def draw_image(self, image, x, y, width, height): - self.native.ResetTransform() - self.native.Transform = self.state.matrix self.native.DrawImage(image._impl.native, x, y, width, height) - self.native.ResetTransform() class Canvas(Box):