Skip to content

Commit aa5b296

Browse files
sherginfacebook-github-bot
authored andcommitted
New round-to-pixel-grid algorithm that fixes possible subpixel gaps between sibling nodes
Summary: This diff introduces new, little bit sophisticated round-to-pixel-grid algorithm. **Motivation:** Previous simple and straightforward solution works in most cases but sometimes produce the not-so-great result. A while ago Nick Lockwood described this problem and proposed the solution in RN's RCTShadowView class: For example, say you have the following structure: // +--------+---------+--------+ // | |+-------+| | // | || || | // | |+-------+| | // +--------+---------+--------+ Say the screen width is 320 pts so the three big views will get the following x bounds from our layout system: {0, 106.667}, {106.667, 213.333}, {213.333, 320} Assuming screen scale is 2, these numbers must be rounded to the nearest 0.5 to fit the pixel grid: {0, 106.5}, {106.5, 213.5}, {213.5, 320} You'll notice that the three widths are 106.5, 107, 106.5. This is great for the parent views but it gets trickier when we consider rounding for the subview. When we go to round the bounds for the subview in the middle, it's relative bounds are {0, 106.667} which gets rounded to {0, 106.5}. This will cause the subview to be one pixel smaller than it should be. This is why we need to pass in the absolute position in order to do the rounding relative to the screen's grid rather than the view's grid. After passing in the absolutePosition of {106.667, y}, we do the following calculations: absoluteLeft = round(absolutePosition.x + viewPosition.left) = round(106.667 + 0) = 106.5 absoluteRight = round(absolutePosition.x + viewPosition.left + viewSize.width) + round(106.667 + 0 + 106.667) = 213.5 width = 213.5 - 106.5 = 107 You'll notice that this is the same width we calculated for the parent view because we've taken its position into account. I believe this is awesome. I also believe that we have to decouple this logic from RN and put it into awesome Yoga. So I did it in this diff. **Fun fact:** The original implementation of this algorithm in RN had (and still have) a bug, which was found by Dustin dshahidehpour and fixed in D4133643. Therefore that diff was unlanded because it broke something unrelated inside RN text engine. I will fix that problem in RN later. **Why do we need to change test methodology?** Because the way we receive layout metrics from Chrome browser actually directly related to rounding problem. Previously we used `offsetHeight` and `offsetWidth` properties of the DOM node, which contain naively rounded values from `computedStyle` or `getBoundingClientRect`. (Which is we are trying to fix!) So, I added the new function that computes node size using two-step-rounding approach, conceptually similar to one that implemented in Yoga. Note: Chrome browser performs rounding layout as part of rendering process and actual values that can ve computed by counting actual pixel are different from these natively rounded ones. **Why do some tests now have different desired values?** These changes actually prove that my approach is correct and more useful for actual view rendering goals. So, let's take a look at test with changed values `rounding_fractial_input_3`: Previously: 64+25+24=114 (Incorrect!) Now: 65+24+25=114 (Correct!) Previously: 64+25+24=114 (Incorrect!) Now: 65+24+25=114 (Correct!) Reviewed By: emilsjolander Differential Revision: D4941266 fbshipit-source-id: 07500f5cc93c628219500e9e07291438e9d5d36c
1 parent f2b5d0f commit aa5b296

File tree

7 files changed

+1323
-60
lines changed

7 files changed

+1323
-60
lines changed

csharp/tests/Facebook.Yoga/YGRoundingTest.cs

Lines changed: 309 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -684,17 +684,17 @@ public void Test_rounding_fractial_input_3()
684684
Assert.AreEqual(0f, root_child0.LayoutX);
685685
Assert.AreEqual(0f, root_child0.LayoutY);
686686
Assert.AreEqual(100f, root_child0.LayoutWidth);
687-
Assert.AreEqual(64f, root_child0.LayoutHeight);
687+
Assert.AreEqual(65f, root_child0.LayoutHeight);
688688

689689
Assert.AreEqual(0f, root_child1.LayoutX);
690690
Assert.AreEqual(64f, root_child1.LayoutY);
691691
Assert.AreEqual(100f, root_child1.LayoutWidth);
692-
Assert.AreEqual(25f, root_child1.LayoutHeight);
692+
Assert.AreEqual(24f, root_child1.LayoutHeight);
693693

694694
Assert.AreEqual(0f, root_child2.LayoutX);
695695
Assert.AreEqual(89f, root_child2.LayoutY);
696696
Assert.AreEqual(100f, root_child2.LayoutWidth);
697-
Assert.AreEqual(24f, root_child2.LayoutHeight);
697+
Assert.AreEqual(25f, root_child2.LayoutHeight);
698698

699699
root.StyleDirection = YogaDirection.RTL;
700700
root.CalculateLayout();
@@ -707,17 +707,17 @@ public void Test_rounding_fractial_input_3()
707707
Assert.AreEqual(0f, root_child0.LayoutX);
708708
Assert.AreEqual(0f, root_child0.LayoutY);
709709
Assert.AreEqual(100f, root_child0.LayoutWidth);
710-
Assert.AreEqual(64f, root_child0.LayoutHeight);
710+
Assert.AreEqual(65f, root_child0.LayoutHeight);
711711

712712
Assert.AreEqual(0f, root_child1.LayoutX);
713713
Assert.AreEqual(64f, root_child1.LayoutY);
714714
Assert.AreEqual(100f, root_child1.LayoutWidth);
715-
Assert.AreEqual(25f, root_child1.LayoutHeight);
715+
Assert.AreEqual(24f, root_child1.LayoutHeight);
716716

717717
Assert.AreEqual(0f, root_child2.LayoutX);
718718
Assert.AreEqual(89f, root_child2.LayoutY);
719719
Assert.AreEqual(100f, root_child2.LayoutWidth);
720-
Assert.AreEqual(24f, root_child2.LayoutHeight);
720+
Assert.AreEqual(25f, root_child2.LayoutHeight);
721721
}
722722

723723
[Test]
@@ -793,5 +793,308 @@ public void Test_rounding_fractial_input_4()
793793
Assert.AreEqual(24f, root_child2.LayoutHeight);
794794
}
795795

796+
[Test]
797+
public void Test_rounding_inner_node_controversy_horizontal()
798+
{
799+
YogaConfig config = new YogaConfig();
800+
config.SetExperimentalFeatureEnabled(YogaExperimentalFeature.Rounding, true);
801+
802+
YogaNode root = new YogaNode(config);
803+
root.FlexDirection = YogaFlexDirection.Row;
804+
root.Width = 320;
805+
806+
YogaNode root_child0 = new YogaNode(config);
807+
root_child0.FlexGrow = 1;
808+
root_child0.Height = 10;
809+
root.Insert(0, root_child0);
810+
811+
YogaNode root_child1 = new YogaNode(config);
812+
root_child1.FlexGrow = 1;
813+
root_child1.Height = 10;
814+
root.Insert(1, root_child1);
815+
816+
YogaNode root_child1_child0 = new YogaNode(config);
817+
root_child1_child0.FlexGrow = 1;
818+
root_child1_child0.Height = 10;
819+
root_child1.Insert(0, root_child1_child0);
820+
821+
YogaNode root_child2 = new YogaNode(config);
822+
root_child2.FlexGrow = 1;
823+
root_child2.Height = 10;
824+
root.Insert(2, root_child2);
825+
root.StyleDirection = YogaDirection.LTR;
826+
root.CalculateLayout();
827+
828+
Assert.AreEqual(0f, root.LayoutX);
829+
Assert.AreEqual(0f, root.LayoutY);
830+
Assert.AreEqual(320f, root.LayoutWidth);
831+
Assert.AreEqual(10f, root.LayoutHeight);
832+
833+
Assert.AreEqual(0f, root_child0.LayoutX);
834+
Assert.AreEqual(0f, root_child0.LayoutY);
835+
Assert.AreEqual(107f, root_child0.LayoutWidth);
836+
Assert.AreEqual(10f, root_child0.LayoutHeight);
837+
838+
Assert.AreEqual(107f, root_child1.LayoutX);
839+
Assert.AreEqual(0f, root_child1.LayoutY);
840+
Assert.AreEqual(106f, root_child1.LayoutWidth);
841+
Assert.AreEqual(10f, root_child1.LayoutHeight);
842+
843+
Assert.AreEqual(0f, root_child1_child0.LayoutX);
844+
Assert.AreEqual(0f, root_child1_child0.LayoutY);
845+
Assert.AreEqual(106f, root_child1_child0.LayoutWidth);
846+
Assert.AreEqual(10f, root_child1_child0.LayoutHeight);
847+
848+
Assert.AreEqual(213f, root_child2.LayoutX);
849+
Assert.AreEqual(0f, root_child2.LayoutY);
850+
Assert.AreEqual(107f, root_child2.LayoutWidth);
851+
Assert.AreEqual(10f, root_child2.LayoutHeight);
852+
853+
root.StyleDirection = YogaDirection.RTL;
854+
root.CalculateLayout();
855+
856+
Assert.AreEqual(0f, root.LayoutX);
857+
Assert.AreEqual(0f, root.LayoutY);
858+
Assert.AreEqual(320f, root.LayoutWidth);
859+
Assert.AreEqual(10f, root.LayoutHeight);
860+
861+
Assert.AreEqual(213f, root_child0.LayoutX);
862+
Assert.AreEqual(0f, root_child0.LayoutY);
863+
Assert.AreEqual(107f, root_child0.LayoutWidth);
864+
Assert.AreEqual(10f, root_child0.LayoutHeight);
865+
866+
Assert.AreEqual(107f, root_child1.LayoutX);
867+
Assert.AreEqual(0f, root_child1.LayoutY);
868+
Assert.AreEqual(106f, root_child1.LayoutWidth);
869+
Assert.AreEqual(10f, root_child1.LayoutHeight);
870+
871+
Assert.AreEqual(0f, root_child1_child0.LayoutX);
872+
Assert.AreEqual(0f, root_child1_child0.LayoutY);
873+
Assert.AreEqual(106f, root_child1_child0.LayoutWidth);
874+
Assert.AreEqual(10f, root_child1_child0.LayoutHeight);
875+
876+
Assert.AreEqual(0f, root_child2.LayoutX);
877+
Assert.AreEqual(0f, root_child2.LayoutY);
878+
Assert.AreEqual(107f, root_child2.LayoutWidth);
879+
Assert.AreEqual(10f, root_child2.LayoutHeight);
880+
}
881+
882+
[Test]
883+
public void Test_rounding_inner_node_controversy_vertical()
884+
{
885+
YogaConfig config = new YogaConfig();
886+
config.SetExperimentalFeatureEnabled(YogaExperimentalFeature.Rounding, true);
887+
888+
YogaNode root = new YogaNode(config);
889+
root.Height = 320;
890+
891+
YogaNode root_child0 = new YogaNode(config);
892+
root_child0.FlexGrow = 1;
893+
root_child0.Width = 10;
894+
root.Insert(0, root_child0);
895+
896+
YogaNode root_child1 = new YogaNode(config);
897+
root_child1.FlexGrow = 1;
898+
root_child1.Width = 10;
899+
root.Insert(1, root_child1);
900+
901+
YogaNode root_child1_child0 = new YogaNode(config);
902+
root_child1_child0.FlexGrow = 1;
903+
root_child1_child0.Width = 10;
904+
root_child1.Insert(0, root_child1_child0);
905+
906+
YogaNode root_child2 = new YogaNode(config);
907+
root_child2.FlexGrow = 1;
908+
root_child2.Width = 10;
909+
root.Insert(2, root_child2);
910+
root.StyleDirection = YogaDirection.LTR;
911+
root.CalculateLayout();
912+
913+
Assert.AreEqual(0f, root.LayoutX);
914+
Assert.AreEqual(0f, root.LayoutY);
915+
Assert.AreEqual(10f, root.LayoutWidth);
916+
Assert.AreEqual(320f, root.LayoutHeight);
917+
918+
Assert.AreEqual(0f, root_child0.LayoutX);
919+
Assert.AreEqual(0f, root_child0.LayoutY);
920+
Assert.AreEqual(10f, root_child0.LayoutWidth);
921+
Assert.AreEqual(107f, root_child0.LayoutHeight);
922+
923+
Assert.AreEqual(0f, root_child1.LayoutX);
924+
Assert.AreEqual(107f, root_child1.LayoutY);
925+
Assert.AreEqual(10f, root_child1.LayoutWidth);
926+
Assert.AreEqual(106f, root_child1.LayoutHeight);
927+
928+
Assert.AreEqual(0f, root_child1_child0.LayoutX);
929+
Assert.AreEqual(0f, root_child1_child0.LayoutY);
930+
Assert.AreEqual(10f, root_child1_child0.LayoutWidth);
931+
Assert.AreEqual(106f, root_child1_child0.LayoutHeight);
932+
933+
Assert.AreEqual(0f, root_child2.LayoutX);
934+
Assert.AreEqual(213f, root_child2.LayoutY);
935+
Assert.AreEqual(10f, root_child2.LayoutWidth);
936+
Assert.AreEqual(107f, root_child2.LayoutHeight);
937+
938+
root.StyleDirection = YogaDirection.RTL;
939+
root.CalculateLayout();
940+
941+
Assert.AreEqual(0f, root.LayoutX);
942+
Assert.AreEqual(0f, root.LayoutY);
943+
Assert.AreEqual(10f, root.LayoutWidth);
944+
Assert.AreEqual(320f, root.LayoutHeight);
945+
946+
Assert.AreEqual(0f, root_child0.LayoutX);
947+
Assert.AreEqual(0f, root_child0.LayoutY);
948+
Assert.AreEqual(10f, root_child0.LayoutWidth);
949+
Assert.AreEqual(107f, root_child0.LayoutHeight);
950+
951+
Assert.AreEqual(0f, root_child1.LayoutX);
952+
Assert.AreEqual(107f, root_child1.LayoutY);
953+
Assert.AreEqual(10f, root_child1.LayoutWidth);
954+
Assert.AreEqual(106f, root_child1.LayoutHeight);
955+
956+
Assert.AreEqual(0f, root_child1_child0.LayoutX);
957+
Assert.AreEqual(0f, root_child1_child0.LayoutY);
958+
Assert.AreEqual(10f, root_child1_child0.LayoutWidth);
959+
Assert.AreEqual(106f, root_child1_child0.LayoutHeight);
960+
961+
Assert.AreEqual(0f, root_child2.LayoutX);
962+
Assert.AreEqual(213f, root_child2.LayoutY);
963+
Assert.AreEqual(10f, root_child2.LayoutWidth);
964+
Assert.AreEqual(107f, root_child2.LayoutHeight);
965+
}
966+
967+
[Test]
968+
public void Test_rounding_inner_node_controversy_combined()
969+
{
970+
YogaConfig config = new YogaConfig();
971+
config.SetExperimentalFeatureEnabled(YogaExperimentalFeature.Rounding, true);
972+
973+
YogaNode root = new YogaNode(config);
974+
root.FlexDirection = YogaFlexDirection.Row;
975+
root.Width = 640;
976+
root.Height = 320;
977+
978+
YogaNode root_child0 = new YogaNode(config);
979+
root_child0.FlexGrow = 1;
980+
root_child0.Height = 100.Percent();
981+
root.Insert(0, root_child0);
982+
983+
YogaNode root_child1 = new YogaNode(config);
984+
root_child1.FlexGrow = 1;
985+
root_child1.Height = 100.Percent();
986+
root.Insert(1, root_child1);
987+
988+
YogaNode root_child1_child0 = new YogaNode(config);
989+
root_child1_child0.FlexGrow = 1;
990+
root_child1_child0.Width = 100.Percent();
991+
root_child1.Insert(0, root_child1_child0);
992+
993+
YogaNode root_child1_child1 = new YogaNode(config);
994+
root_child1_child1.FlexGrow = 1;
995+
root_child1_child1.Width = 100.Percent();
996+
root_child1.Insert(1, root_child1_child1);
997+
998+
YogaNode root_child1_child1_child0 = new YogaNode(config);
999+
root_child1_child1_child0.FlexGrow = 1;
1000+
root_child1_child1_child0.Width = 100.Percent();
1001+
root_child1_child1.Insert(0, root_child1_child1_child0);
1002+
1003+
YogaNode root_child1_child2 = new YogaNode(config);
1004+
root_child1_child2.FlexGrow = 1;
1005+
root_child1_child2.Width = 100.Percent();
1006+
root_child1.Insert(2, root_child1_child2);
1007+
1008+
YogaNode root_child2 = new YogaNode(config);
1009+
root_child2.FlexGrow = 1;
1010+
root_child2.Height = 100.Percent();
1011+
root.Insert(2, root_child2);
1012+
root.StyleDirection = YogaDirection.LTR;
1013+
root.CalculateLayout();
1014+
1015+
Assert.AreEqual(0f, root.LayoutX);
1016+
Assert.AreEqual(0f, root.LayoutY);
1017+
Assert.AreEqual(640f, root.LayoutWidth);
1018+
Assert.AreEqual(320f, root.LayoutHeight);
1019+
1020+
Assert.AreEqual(0f, root_child0.LayoutX);
1021+
Assert.AreEqual(0f, root_child0.LayoutY);
1022+
Assert.AreEqual(213f, root_child0.LayoutWidth);
1023+
Assert.AreEqual(320f, root_child0.LayoutHeight);
1024+
1025+
Assert.AreEqual(213f, root_child1.LayoutX);
1026+
Assert.AreEqual(0f, root_child1.LayoutY);
1027+
Assert.AreEqual(214f, root_child1.LayoutWidth);
1028+
Assert.AreEqual(320f, root_child1.LayoutHeight);
1029+
1030+
Assert.AreEqual(0f, root_child1_child0.LayoutX);
1031+
Assert.AreEqual(0f, root_child1_child0.LayoutY);
1032+
Assert.AreEqual(214f, root_child1_child0.LayoutWidth);
1033+
Assert.AreEqual(107f, root_child1_child0.LayoutHeight);
1034+
1035+
Assert.AreEqual(0f, root_child1_child1.LayoutX);
1036+
Assert.AreEqual(107f, root_child1_child1.LayoutY);
1037+
Assert.AreEqual(214f, root_child1_child1.LayoutWidth);
1038+
Assert.AreEqual(106f, root_child1_child1.LayoutHeight);
1039+
1040+
Assert.AreEqual(0f, root_child1_child1_child0.LayoutX);
1041+
Assert.AreEqual(0f, root_child1_child1_child0.LayoutY);
1042+
Assert.AreEqual(214f, root_child1_child1_child0.LayoutWidth);
1043+
Assert.AreEqual(106f, root_child1_child1_child0.LayoutHeight);
1044+
1045+
Assert.AreEqual(0f, root_child1_child2.LayoutX);
1046+
Assert.AreEqual(213f, root_child1_child2.LayoutY);
1047+
Assert.AreEqual(214f, root_child1_child2.LayoutWidth);
1048+
Assert.AreEqual(107f, root_child1_child2.LayoutHeight);
1049+
1050+
Assert.AreEqual(427f, root_child2.LayoutX);
1051+
Assert.AreEqual(0f, root_child2.LayoutY);
1052+
Assert.AreEqual(213f, root_child2.LayoutWidth);
1053+
Assert.AreEqual(320f, root_child2.LayoutHeight);
1054+
1055+
root.StyleDirection = YogaDirection.RTL;
1056+
root.CalculateLayout();
1057+
1058+
Assert.AreEqual(0f, root.LayoutX);
1059+
Assert.AreEqual(0f, root.LayoutY);
1060+
Assert.AreEqual(640f, root.LayoutWidth);
1061+
Assert.AreEqual(320f, root.LayoutHeight);
1062+
1063+
Assert.AreEqual(427f, root_child0.LayoutX);
1064+
Assert.AreEqual(0f, root_child0.LayoutY);
1065+
Assert.AreEqual(213f, root_child0.LayoutWidth);
1066+
Assert.AreEqual(320f, root_child0.LayoutHeight);
1067+
1068+
Assert.AreEqual(213f, root_child1.LayoutX);
1069+
Assert.AreEqual(0f, root_child1.LayoutY);
1070+
Assert.AreEqual(214f, root_child1.LayoutWidth);
1071+
Assert.AreEqual(320f, root_child1.LayoutHeight);
1072+
1073+
Assert.AreEqual(0f, root_child1_child0.LayoutX);
1074+
Assert.AreEqual(0f, root_child1_child0.LayoutY);
1075+
Assert.AreEqual(214f, root_child1_child0.LayoutWidth);
1076+
Assert.AreEqual(107f, root_child1_child0.LayoutHeight);
1077+
1078+
Assert.AreEqual(0f, root_child1_child1.LayoutX);
1079+
Assert.AreEqual(107f, root_child1_child1.LayoutY);
1080+
Assert.AreEqual(214f, root_child1_child1.LayoutWidth);
1081+
Assert.AreEqual(106f, root_child1_child1.LayoutHeight);
1082+
1083+
Assert.AreEqual(0f, root_child1_child1_child0.LayoutX);
1084+
Assert.AreEqual(0f, root_child1_child1_child0.LayoutY);
1085+
Assert.AreEqual(214f, root_child1_child1_child0.LayoutWidth);
1086+
Assert.AreEqual(106f, root_child1_child1_child0.LayoutHeight);
1087+
1088+
Assert.AreEqual(0f, root_child1_child2.LayoutX);
1089+
Assert.AreEqual(213f, root_child1_child2.LayoutY);
1090+
Assert.AreEqual(214f, root_child1_child2.LayoutWidth);
1091+
Assert.AreEqual(107f, root_child1_child2.LayoutHeight);
1092+
1093+
Assert.AreEqual(0f, root_child2.LayoutX);
1094+
Assert.AreEqual(0f, root_child2.LayoutY);
1095+
Assert.AreEqual(213f, root_child2.LayoutWidth);
1096+
Assert.AreEqual(320f, root_child2.LayoutHeight);
1097+
}
1098+
7961099
}
7971100
}

gentest/fixtures/YGRoundingTest.html

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,3 +62,34 @@
6262
<div style="height: 10px; flex-grow:1;"></div>
6363
<div style="height: 10px; flex-grow:1;"></div>
6464
</div>
65+
66+
<div id="rounding_inner_node_controversy_horizontal" experiments="Rounding" style="width: 320px; flex-direction: row;">
67+
<div style="height: 10px; flex-grow:1;"></div>
68+
<div style="height: 10px; flex-grow:1;">
69+
<div style="height: 10px; flex-grow:1;">
70+
</div>
71+
</div>
72+
<div style="height: 10px; flex-grow:1;"></div>
73+
</div>
74+
75+
<div id="rounding_inner_node_controversy_vertical" experiments="Rounding" style="height: 320px;">
76+
<div style="width: 10px; flex-grow:1;"></div>
77+
<div style="width: 10px; flex-grow:1;">
78+
<div style="width: 10px; flex-grow:1;">
79+
</div>
80+
</div>
81+
<div style="width: 10px; flex-grow:1;"></div>
82+
</div>
83+
84+
<div id="rounding_inner_node_controversy_combined" experiments="Rounding" style="width: 640px; height: 320px; flex-direction: row;">
85+
<div style="height: 100%; flex-grow:1;"></div>
86+
<div style="height: 100%; flex-grow:1; flex-direction: column;">
87+
<div style="width: 100%; flex-grow:1;"></div>
88+
<div style="width: 100%; flex-grow:1;">
89+
<div style="flex-grow:1; width: 100%;">
90+
</div>
91+
</div>
92+
<div style="width: 100%; flex-grow:1;"></div>
93+
</div>
94+
<div style="height: 100%; flex-grow:1;"></div>
95+
</div>

0 commit comments

Comments
 (0)