From a0028a1a4df5d9a488b779d6fa6bc83caacd0f0f Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Wed, 9 Jul 2025 12:25:07 -0700 Subject: [PATCH] [ADJUSTMENT] - Prefer to expand pixel snapping sizes rather than shrink, because shrinking intrinsically sized things can force a very different result on the other axis. --- lib/src/flutter/flutter_pixel_alignment.dart | 158 +++++++++++++++++-- test/flutter/pixel_snapping_test.dart | 12 +- 2 files changed, 155 insertions(+), 15 deletions(-) diff --git a/lib/src/flutter/flutter_pixel_alignment.dart b/lib/src/flutter/flutter_pixel_alignment.dart index 3da7c68..9759e43 100644 --- a/lib/src/flutter/flutter_pixel_alignment.dart +++ b/lib/src/flutter/flutter_pixel_alignment.dart @@ -290,17 +290,157 @@ class RenderPixelSnapFlex extends RenderFlex { ); } - if (child.size.width != child.size.width.floorToDouble() || - child.size.height != child.size.height.floorToDouble()) { + if (!child.size.isInteger) { // This child doesn't have an integer width/height - run layout again, - // forcing the nearest smaller size. - child.layout( - BoxConstraints.tightFor( - width: child.size.width.floorToDouble(), - height: child.size.height.floorToDouble(), - ), - ); + // forcing either slightly bigger or slightly smaller. + // + // There are a couple details that are sometimes important when changing + // the size to an integer value. + // + // First, we must always be mindful of text with intrinsic sizing. Any + // reduction in bounds to intrinsically sized text will force a relayout with + // some kind of wrapping, because the text reported exactly the bounds for + // its current layout. Thus, we want to prefer expanding size rather than + // contracting size. But we need to do our best to handle size contraction, + // too, in case we're dealing with a hard width or height constraint. + // + // Second, Columns and Rows are often intrinsically sized, too, in both + // dimensions. That said, Columns are often given space to expand/contract + // vertically, and Rows are often given space to expand/contract horizontally. + // For this reason, we treat Columns and Rows differently for re-sizing - we + // try to fit the cross-axis first, and then the main-axis second. + if (direction == Axis.vertical) { + _resizeChildForColumn(child); + } else { + _resizeChildForRow(child); + } } } } + + void _resizeChildForColumn(RenderBox child) { + if (child.size.isInteger) { + return; + } + + // This is a column. Start by working on the cross-axis, which is more likely + // to be bounded than the main-axis. + final widest = constraints.biggest.width; + late final int newWidth; + late final bool didShrinkWidth; + if (!child.size.widthIsInteger) { + if (child.size.width <= widest) { + // We prefer to increase size rather than decrease size, because text + // wrapping becomes a problem when shrinking intrinsic size. We can fit + // a bigger width, so use that. + newWidth = child.size.width.ceil(); + didShrinkWidth = false; + } else { + // We prefer wider over narrower, but we can't fit any wider. Shrink instead. + newWidth = child.size.width.floor(); + didShrinkWidth = true; + } + } else { + newWidth = child.size.width.toInt(); + didShrinkWidth = false; + } + + if (didShrinkWidth) { + // Shrinking the width has a non-trivial chance of significantly impacting the + // height, so run layout again with the new width and then deal with the height. + child.layout(BoxConstraints.tightFor(width: newWidth.toDouble()), parentUsesSize: true); + } + + // Now do the main-axis. + final tallest = constraints.biggest.height; + late final int newHeight; + if (!child.size.heightIsInteger) { + if (child.size.height <= tallest) { + newHeight = child.size.height.ceil(); + } else { + newHeight = child.size.height.floor(); + } + } else { + newHeight = child.size.height.toInt(); + } + + // Note: This layout process can fail if a situation arises in which both the + // width and height need to contract, or if contracting the width produces a + // much taller height that violates constraints. If this happens to you, please + // file an issue on GitHub for flutter_test_goldens and provide us with the exact + // situation that's breaking for you. + child.layout( + BoxConstraints.tightFor( + width: newWidth.toDouble(), + height: newHeight.toDouble(), + ), + ); + } + + void _resizeChildForRow(RenderBox child) { + if (child.size.isInteger) { + return; + } + + // This is a row. Start by working on the cross-axis, which is more likely + // to be bounded than the main-axis. + final tallest = constraints.biggest.height; + late final int newHeight; + late final bool didShrinkHeight; + if (!child.size.heightIsInteger) { + if (child.size.height <= tallest) { + // We prefer to increase size rather than decrease size, because text + // wrapping (and other layouts) becomes a problem when shrinking intrinsic + // size. We can fit a bigger height, so use that. + newHeight = child.size.height.ceil(); + didShrinkHeight = false; + } else { + // We prefer taller over shorter, but we can't fit any taller. Shrink instead. + newHeight = child.size.height.floor(); + didShrinkHeight = true; + } + } else { + newHeight = child.size.height.toInt(); + didShrinkHeight = false; + } + + if (didShrinkHeight) { + // Shrinking the height has a non-trivial chance of significantly impacting the + // width, so run layout again with the new height and then deal with the width. + child.layout(BoxConstraints.tightFor(height: newHeight.toDouble()), parentUsesSize: true); + } + + // Now do the main-axis. + final widest = constraints.biggest.width; + late final int newWidth; + if (!child.size.widthIsInteger) { + if (child.size.width <= widest) { + newWidth = child.size.width.ceil(); + } else { + newWidth = child.size.width.floor(); + } + } else { + newWidth = child.size.width.toInt(); + } + + // Note: This layout process can fail if a situation arises in which both the + // width and height need to contract, or if contracting the width produces a + // much taller height that violates constraints. If this happens to you, please + // file an issue on GitHub for flutter_test_goldens and provide us with the exact + // situation that's breaking for you. + child.layout( + BoxConstraints.tightFor( + width: newWidth.toDouble(), + height: newHeight.toDouble(), + ), + ); + } +} + +extension on Size { + bool get isInteger => widthIsInteger && heightIsInteger; + + bool get widthIsInteger => width == width.floorToDouble(); + + bool get heightIsInteger => height == height.floorToDouble(); } diff --git a/test/flutter/pixel_snapping_test.dart b/test/flutter/pixel_snapping_test.dart index 3a0843e..52f69d6 100644 --- a/test/flutter/pixel_snapping_test.dart +++ b/test/flutter/pixel_snapping_test.dart @@ -99,11 +99,11 @@ void main() { // Ensure a whole-pixel offset. expect(tester.getTopLeft(find.byKey(item1Key)), Offset(7, 38)); - expect(tester.getSize(find.byKey(item1Key)), Size(23, 23)); + expect(tester.getSize(find.byKey(item1Key)), Size(24, 24)); expect(tester.getTopLeft(find.byKey(item2Key)), Offset(38, 38)); - expect(tester.getSize(find.byKey(item2Key)), Size(23, 23)); + expect(tester.getSize(find.byKey(item2Key)), Size(24, 24)); expect(tester.getTopLeft(find.byKey(item3Key)), Offset(69, 38)); - expect(tester.getSize(find.byKey(item3Key)), Size(23, 23)); + expect(tester.getSize(find.byKey(item3Key)), Size(24, 24)); }); testWidgets("PixelSnapColumn", (tester) async { @@ -133,11 +133,11 @@ void main() { // Ensure a whole-pixel offset. expect(tester.getTopLeft(find.byKey(item1Key)), Offset(38, 7)); - expect(tester.getSize(find.byKey(item1Key)), Size(23, 23)); + expect(tester.getSize(find.byKey(item1Key)), Size(24, 24)); expect(tester.getTopLeft(find.byKey(item2Key)), Offset(38, 38)); - expect(tester.getSize(find.byKey(item2Key)), Size(23, 23)); + expect(tester.getSize(find.byKey(item2Key)), Size(24, 24)); expect(tester.getTopLeft(find.byKey(item3Key)), Offset(38, 69)); - expect(tester.getSize(find.byKey(item3Key)), Size(23, 23)); + expect(tester.getSize(find.byKey(item3Key)), Size(24, 24)); }); }); }