react / react/yoga

Why can't we round node width/height directly in roundLayoutResultsToPixelGrid()?

Open
#1,574 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Algorithm Discussion
Dominant language
C++
Stars
18.9k
Forks
1.6k
Avg merge
1m
Merged PRs (30d)
1

Description

Report

In PixelGrid.roundLayoutResultsToPixelGrid() it appears that we are calculating a node's rounded width/height as follows:

  1. Calculate the start position of the node.
const double absoluteNodeLeft = absoluteLeft + nodeLeft;
  1. Using the start position and computed width, calculate the end position of the node.
const double absoluteNodeRight = absoluteNodeLeft + nodeWidth;
  1. Calculate rounded node width/height as the difference between the rounded start/end positions of the node.
node->setLayoutDimension(
    roundValueToPixelGrid(
        absoluteNodeRight, 
        pointScaleFactor, 
        (textRounding && hasFractionalWidth), 
        (textRounding && !hasFractionalWidth)
    ) -
    roundValueToPixelGrid(
        absoluteNodeLeft, 
        pointScaleFactor, 
        false, 
        textRounding
     ),
     Dimension::Width);

It's not clear to me why we need to calculate rounded width/height in this way as opposed to rounding the width/height values directly, and it also seems like this may introduce rounding errors as noted in this previously opened issue (https://github.com/facebook/yoga/issues/901). Curious if the proposed solution on that issue has merit or if there are other issues not being considered.

As an iOS developer utilizing a fork of Yoga (really appreciate this project btw!) I'm also curious if you foresee any issues with tweaking the rounding logic on our fork to more closely mirror Apple's strategy of rounding down origin values and rounding up size values to the nearest pixel boundary. Are there other parts of the layout algorithm we would need to update for consistency?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in yoga/algorithm/PixelGrid.cpp at roundLayoutResultsToPixelGrid() and trace how absolute positions and dimensions are rounded. Compare the behavior with issue #901, its proposed PR #902, and Apple's CGRectIntegral strategy; done means the rounding rationale and any required consistent changes are established.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, ios
Domain
frontend, mobile-dev
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.