Skip to content

Commit 52ff903

Browse files
matthew-carrollmatthew-carroll
authored
[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. (#67)
1 parent fe9d7dc commit 52ff903

File tree

2 files changed

+155
-15
lines changed

2 files changed

+155
-15
lines changed

‎lib/src/flutter/flutter_pixel_alignment.dart‎

Lines changed: 149 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -290,17 +290,157 @@ class RenderPixelSnapFlex extends RenderFlex {
290290
);
291291
}
292292

293-
if (child.size.width != child.size.width.floorToDouble() ||
294-
child.size.height != child.size.height.floorToDouble()) {
293+
if (!child.size.isInteger) {
295294
// This child doesn't have an integer width/height - run layout again,
296-
// forcing the nearest smaller size.
297-
child.layout(
298-
BoxConstraints.tightFor(
299-
width: child.size.width.floorToDouble(),
300-
height: child.size.height.floorToDouble(),
301-
),
302-
);
295+
// forcing either slightly bigger or slightly smaller.
296+
//
297+
// There are a couple details that are sometimes important when changing
298+
// the size to an integer value.
299+
//
300+
// First, we must always be mindful of text with intrinsic sizing. Any
301+
// reduction in bounds to intrinsically sized text will force a relayout with
302+
// some kind of wrapping, because the text reported exactly the bounds for
303+
// its current layout. Thus, we want to prefer expanding size rather than
304+
// contracting size. But we need to do our best to handle size contraction,
305+
// too, in case we're dealing with a hard width or height constraint.
306+
//
307+
// Second, Columns and Rows are often intrinsically sized, too, in both
308+
// dimensions. That said, Columns are often given space to expand/contract
309+
// vertically, and Rows are often given space to expand/contract horizontally.
310+
// For this reason, we treat Columns and Rows differently for re-sizing - we
311+
// try to fit the cross-axis first, and then the main-axis second.
312+
if (direction == Axis.vertical) {
313+
_resizeChildForColumn(child);
314+
} else {
315+
_resizeChildForRow(child);
316+
}
303317
}
304318
}
305319
}
320+
321+
void _resizeChildForColumn(RenderBox child) {
322+
if (child.size.isInteger) {
323+
return;
324+
}
325+
326+
// This is a column. Start by working on the cross-axis, which is more likely
327+
// to be bounded than the main-axis.
328+
final widest = constraints.biggest.width;
329+
late final int newWidth;
330+
late final bool didShrinkWidth;
331+
if (!child.size.widthIsInteger) {
332+
if (child.size.width <= widest) {
333+
// We prefer to increase size rather than decrease size, because text
334+
// wrapping becomes a problem when shrinking intrinsic size. We can fit
335+
// a bigger width, so use that.
336+
newWidth = child.size.width.ceil();
337+
didShrinkWidth = false;
338+
} else {
339+
// We prefer wider over narrower, but we can't fit any wider. Shrink instead.
340+
newWidth = child.size.width.floor();
341+
didShrinkWidth = true;
342+
}
343+
} else {
344+
newWidth = child.size.width.toInt();
345+
didShrinkWidth = false;
346+
}
347+
348+
if (didShrinkWidth) {
349+
// Shrinking the width has a non-trivial chance of significantly impacting the
350+
// height, so run layout again with the new width and then deal with the height.
351+
child.layout(BoxConstraints.tightFor(width: newWidth.toDouble()), parentUsesSize: true);
352+
}
353+
354+
// Now do the main-axis.
355+
final tallest = constraints.biggest.height;
356+
late final int newHeight;
357+
if (!child.size.heightIsInteger) {
358+
if (child.size.height <= tallest) {
359+
newHeight = child.size.height.ceil();
360+
} else {
361+
newHeight = child.size.height.floor();
362+
}
363+
} else {
364+
newHeight = child.size.height.toInt();
365+
}
366+
367+
// Note: This layout process can fail if a situation arises in which both the
368+
// width and height need to contract, or if contracting the width produces a
369+
// much taller height that violates constraints. If this happens to you, please
370+
// file an issue on GitHub for flutter_test_goldens and provide us with the exact
371+
// situation that's breaking for you.
372+
child.layout(
373+
BoxConstraints.tightFor(
374+
width: newWidth.toDouble(),
375+
height: newHeight.toDouble(),
376+
),
377+
);
378+
}
379+
380+
void _resizeChildForRow(RenderBox child) {
381+
if (child.size.isInteger) {
382+
return;
383+
}
384+
385+
// This is a row. Start by working on the cross-axis, which is more likely
386+
// to be bounded than the main-axis.
387+
final tallest = constraints.biggest.height;
388+
late final int newHeight;
389+
late final bool didShrinkHeight;
390+
if (!child.size.heightIsInteger) {
391+
if (child.size.height <= tallest) {
392+
// We prefer to increase size rather than decrease size, because text
393+
// wrapping (and other layouts) becomes a problem when shrinking intrinsic
394+
// size. We can fit a bigger height, so use that.
395+
newHeight = child.size.height.ceil();
396+
didShrinkHeight = false;
397+
} else {
398+
// We prefer taller over shorter, but we can't fit any taller. Shrink instead.
399+
newHeight = child.size.height.floor();
400+
didShrinkHeight = true;
401+
}
402+
} else {
403+
newHeight = child.size.height.toInt();
404+
didShrinkHeight = false;
405+
}
406+
407+
if (didShrinkHeight) {
408+
// Shrinking the height has a non-trivial chance of significantly impacting the
409+
// width, so run layout again with the new height and then deal with the width.
410+
child.layout(BoxConstraints.tightFor(height: newHeight.toDouble()), parentUsesSize: true);
411+
}
412+
413+
// Now do the main-axis.
414+
final widest = constraints.biggest.width;
415+
late final int newWidth;
416+
if (!child.size.widthIsInteger) {
417+
if (child.size.width <= widest) {
418+
newWidth = child.size.width.ceil();
419+
} else {
420+
newWidth = child.size.width.floor();
421+
}
422+
} else {
423+
newWidth = child.size.width.toInt();
424+
}
425+
426+
// Note: This layout process can fail if a situation arises in which both the
427+
// width and height need to contract, or if contracting the width produces a
428+
// much taller height that violates constraints. If this happens to you, please
429+
// file an issue on GitHub for flutter_test_goldens and provide us with the exact
430+
// situation that's breaking for you.
431+
child.layout(
432+
BoxConstraints.tightFor(
433+
width: newWidth.toDouble(),
434+
height: newHeight.toDouble(),
435+
),
436+
);
437+
}
438+
}
439+
440+
extension on Size {
441+
bool get isInteger => widthIsInteger && heightIsInteger;
442+
443+
bool get widthIsInteger => width == width.floorToDouble();
444+
445+
bool get heightIsInteger => height == height.floorToDouble();
306446
}

‎test/flutter/pixel_snapping_test.dart‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ void main() {
9999

100100
// Ensure a whole-pixel offset.
101101
expect(tester.getTopLeft(find.byKey(item1Key)), Offset(7, 38));
102-
expect(tester.getSize(find.byKey(item1Key)), Size(23, 23));
102+
expect(tester.getSize(find.byKey(item1Key)), Size(24, 24));
103103
expect(tester.getTopLeft(find.byKey(item2Key)), Offset(38, 38));
104-
expect(tester.getSize(find.byKey(item2Key)), Size(23, 23));
104+
expect(tester.getSize(find.byKey(item2Key)), Size(24, 24));
105105
expect(tester.getTopLeft(find.byKey(item3Key)), Offset(69, 38));
106-
expect(tester.getSize(find.byKey(item3Key)), Size(23, 23));
106+
expect(tester.getSize(find.byKey(item3Key)), Size(24, 24));
107107
});
108108

109109
testWidgets("PixelSnapColumn", (tester) async {
@@ -133,11 +133,11 @@ void main() {
133133

134134
// Ensure a whole-pixel offset.
135135
expect(tester.getTopLeft(find.byKey(item1Key)), Offset(38, 7));
136-
expect(tester.getSize(find.byKey(item1Key)), Size(23, 23));
136+
expect(tester.getSize(find.byKey(item1Key)), Size(24, 24));
137137
expect(tester.getTopLeft(find.byKey(item2Key)), Offset(38, 38));
138-
expect(tester.getSize(find.byKey(item2Key)), Size(23, 23));
138+
expect(tester.getSize(find.byKey(item2Key)), Size(24, 24));
139139
expect(tester.getTopLeft(find.byKey(item3Key)), Offset(38, 69));
140-
expect(tester.getSize(find.byKey(item3Key)), Size(23, 23));
140+
expect(tester.getSize(find.byKey(item3Key)), Size(24, 24));
141141
});
142142
});
143143
}

0 commit comments

Comments
 (0)