Text node shrinks on each layout pass due to bug in YGRoundToPixelGrid #824

Closed
opened 2018-10-08 23:41:29 -07:00 by rigdern · 0 comments
rigdern commented 2018-10-08 23:41:29 -07:00 (Migrated from github.com)

Repro

If you run this program:

static YGSize measureText(
                           YGNodeRef node,
                           float width,
                           YGMeasureMode widthMode,
                           float height,
                           YGMeasureMode heightMode
                           ) {
  return (YGSize){
    .width = 10,
    .height = 10
  };
}

int main(int argc, const char * argv[]) {
  const YGConfigRef config = YGConfigNew();
  YGConfigSetPointScaleFactor(config, 2);
  
  const YGNodeRef root = YGNodeNewWithConfig(config);
  YGNodeStyleSetMargin(root, YGEdgeTop, -1.49);
  YGNodeStyleSetWidth(root, 500);
  YGNodeStyleSetHeight(root, 500);

  const YGNodeRef node0 = YGNodeNewWithConfig(config);
  YGNodeInsertChild(root, node0, 0);
  
  const YGNodeRef node1 = YGNodeNewWithConfig(config);
  YGNodeSetMeasureFunc(node1, measureText);
  YGNodeInsertChild(node0, node1, 0);
  
  for (int i = 0; i < 5; i++) {
    // dirty the tree so YGRoundToPixelGrid runs again
    YGNodeStyleSetMargin(root, YGEdgeLeft, i + 1);

    YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
    YGNodePrint(node1, YGPrintOptionsLayout);
    printf("\n");
  }
  
  YGNodeFreeRecursive(root);
  
  YGConfigFree(config);

  return 0;
}

you'll see this output:

<div layout="width: 500; height: 9.5; top: 0; left: 0;" ></div>
<div layout="width: 500; height: 9; top: 0; left: 0;" ></div>
<div layout="width: 500; height: 8.5; top: 0; left: 0;" ></div>
<div layout="width: 500; height: 8; top: 0; left: 0;" ></div>
<div layout="width: 500; height: 7.5; top: 0; left: 0;" ></div>

Unexpected result: On each layout pass node1's height shrinks even though the input tree hasn't changed in a way that should affect node1's height.

Analysis

There seem to be a couple of factors that make this bug possible:

  1. The rounded height calculated by YGRoundToPixelGrid in the previous layout pass is used as input to YGRoundToPixelGrid in the next layout pass. Consequently, the input to the layout pass is different enabling us to get a different result.
  2. The top of the node has a negative position and the bottom of the node has a positive position. This enables the top and bottom to round differently such that we lose 0.5 on each layout pass.

Here's an example of the bug in YGRoundToPixelGrid in action. YGRoundToPixelGrid has this expression:

  node->setLayoutDimension(
      YGRoundValueToPixelGrid(
          absoluteNodeBottom,
          pointScaleFactor,
          (textRounding && hasFractionalHeight),
          (textRounding && !hasFractionalHeight)) -
          YGRoundValueToPixelGrid(
              absoluteNodeTop, pointScaleFactor, false, textRounding),
      YGDimensionHeight);

Let's simplify it to:

  roundedNodeHeight =
      YGRoundValueToPixelGrid(
          absoluteNodeBottom,
          pointScaleFactor,
          (textRounding && hasFractionalHeight),
          (textRounding && !hasFractionalHeight)) -
          YGRoundValueToPixelGrid(
              absoluteNodeTop, pointScaleFactor, false, textRounding)

And we'll use Round(x) to mean YGRoundValueToPixelGrid(x, ...) (the rest of YGRoundValueToPixelGrid's arguments are omitted from Round to make it easier to focus on the important stuff for this bug). So the expression becomes:

roundedNodeHeight = Round(absoluteNodeBottom) - Round(absoluteNodeTop)

Layout pass 1

Now suppose that we have the following YGRoundToPixelGrid variables:

nodeHeight = 10
absoluteNodeTop = -1.49
absoluteNodeBottom = absoluteNodeTop + nodeHeight = -1.49 + 10 = 8.51

Then roundedNodeHeight is:

roundedNodeHeight = Round(absoluteNodeBottom) - Round(absoluteNodeTop)
                  = Round(8.51) - Round(-1.49) = 8.5 - -1.0 = 9.5

So the input node height was 10 and the output is 9.5 which is also used as input for the next layout pass.

Layout pass 2

nodeHeight = 9.5 // from the previous layout pass
absoluteNodeTop = -1.49
absoluteNodeBottom = absoluteNodeTop + nodeHeight = -1.49 + 9.5 = 8.01

Then roundedNodeHeight is:

roundedNodeHeight = Round(absoluteNodeBottom) - Round(absoluteNodeTop)
                  = Round(8.01) - Round(-1.49) = 8.0 - -1.0 = 9.0

So the input node height was 9.5 and the output is 9.0.

In this way, each layout pass will decrease the node's height by 0.5.

Adam Comella
Microsoft Corp.

### Repro If you run this program: ```c static YGSize measureText( YGNodeRef node, float width, YGMeasureMode widthMode, float height, YGMeasureMode heightMode ) { return (YGSize){ .width = 10, .height = 10 }; } int main(int argc, const char * argv[]) { const YGConfigRef config = YGConfigNew(); YGConfigSetPointScaleFactor(config, 2); const YGNodeRef root = YGNodeNewWithConfig(config); YGNodeStyleSetMargin(root, YGEdgeTop, -1.49); YGNodeStyleSetWidth(root, 500); YGNodeStyleSetHeight(root, 500); const YGNodeRef node0 = YGNodeNewWithConfig(config); YGNodeInsertChild(root, node0, 0); const YGNodeRef node1 = YGNodeNewWithConfig(config); YGNodeSetMeasureFunc(node1, measureText); YGNodeInsertChild(node0, node1, 0); for (int i = 0; i < 5; i++) { // dirty the tree so YGRoundToPixelGrid runs again YGNodeStyleSetMargin(root, YGEdgeLeft, i + 1); YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); YGNodePrint(node1, YGPrintOptionsLayout); printf("\n"); } YGNodeFreeRecursive(root); YGConfigFree(config); return 0; } ``` you'll see this output: ``` <div layout="width: 500; height: 9.5; top: 0; left: 0;" ></div> <div layout="width: 500; height: 9; top: 0; left: 0;" ></div> <div layout="width: 500; height: 8.5; top: 0; left: 0;" ></div> <div layout="width: 500; height: 8; top: 0; left: 0;" ></div> <div layout="width: 500; height: 7.5; top: 0; left: 0;" ></div> ``` **Unexpected result:** On each layout pass `node1's` height shrinks even though the input tree hasn't changed in a way that should affect `node1's` height. ### Analysis There seem to be a couple of factors that make this bug possible: 1. The rounded height calculated by `YGRoundToPixelGrid` in the previous layout pass is used as input to `YGRoundToPixelGrid` in the next layout pass. Consequently, the input to the layout pass is different enabling us to get a different result. 2. The top of the node has a negative position and the bottom of the node has a positive position. This enables the top and bottom to round differently such that we lose `0.5` on each layout pass. Here's an example of the bug in `YGRoundToPixelGrid` in action. `YGRoundToPixelGrid` has this expression: ```c node->setLayoutDimension( YGRoundValueToPixelGrid( absoluteNodeBottom, pointScaleFactor, (textRounding && hasFractionalHeight), (textRounding && !hasFractionalHeight)) - YGRoundValueToPixelGrid( absoluteNodeTop, pointScaleFactor, false, textRounding), YGDimensionHeight); ``` Let's simplify it to: ```c roundedNodeHeight = YGRoundValueToPixelGrid( absoluteNodeBottom, pointScaleFactor, (textRounding && hasFractionalHeight), (textRounding && !hasFractionalHeight)) - YGRoundValueToPixelGrid( absoluteNodeTop, pointScaleFactor, false, textRounding) ``` And we'll use `Round(x)` to mean `YGRoundValueToPixelGrid(x, ...)` (the rest of `YGRoundValueToPixelGrid's` arguments are omitted from `Round` to make it easier to focus on the important stuff for this bug). So the expression becomes: ``` roundedNodeHeight = Round(absoluteNodeBottom) - Round(absoluteNodeTop) ``` #### Layout pass 1 Now suppose that we have the following `YGRoundToPixelGrid` variables: ``` nodeHeight = 10 absoluteNodeTop = -1.49 absoluteNodeBottom = absoluteNodeTop + nodeHeight = -1.49 + 10 = 8.51 ``` Then `roundedNodeHeight` is: ``` roundedNodeHeight = Round(absoluteNodeBottom) - Round(absoluteNodeTop) = Round(8.51) - Round(-1.49) = 8.5 - -1.0 = 9.5 ``` So the input node height was `10` and the output is `9.5` which is also used as input for the next layout pass. #### Layout pass 2 ``` nodeHeight = 9.5 // from the previous layout pass absoluteNodeTop = -1.49 absoluteNodeBottom = absoluteNodeTop + nodeHeight = -1.49 + 9.5 = 8.01 ``` Then `roundedNodeHeight` is: ``` roundedNodeHeight = Round(absoluteNodeBottom) - Round(absoluteNodeTop) = Round(8.01) - Round(-1.49) = 8.0 - -1.0 = 9.0 ``` So the input node height was `9.5` and the output is `9.0`. In this way, each layout pass will decrease the node's height by `0.5`. Adam Comella Microsoft Corp.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#824
No description provided.