Skip to content

Commit

Permalink
Removing our own implementation of round-to-pixel algorithm
Browse files Browse the repository at this point in the history
Summary:
> Okay, I don't remember where we first met
> But hey, admitting is the first step

This issue has a looong history.
The original algorithm was introduced by Nick Lockwood (nicklockwood Hey Nick! We miss you!) a while ago and from the very beginning this has one small error that basically makes it useless (try to find it yourself, it's fun!)
The problem was discovered and fixed twice (D4133643, D4983054), but every time we found that our <Text> infra was not ready for this, so we reverted and abandoned the change. As part of the last attempt to finally solve the issue, I ported the algorithm to Yoga where it lives today and works very well for Lytho and CK.
For now, the vision is clear:
 * The basic algorithm should live in Yoga for unification and performance reasons.
 * We still have to have `absolutePostion` as part of this API because it might be useful for some components which implement its own custom/non-Yoga-based layout.
 * We have to enable it in RN eventually.

So, this is the first step: Removing old, broken code which we don't plan to fix and use.

Make React Native crisp again!

Reviewed By: fkgozali

Differential Revision: D6888662

fbshipit-source-id: 2e5098d9935dcbe05d66c777dad3a9ec8ac87ec3
  • Loading branch information
shergin authored and facebook-github-bot committed Feb 6, 2018
1 parent c19bc79 commit ceb1d1c
Showing 1 changed file with 2 additions and 49 deletions.
51 changes: 2 additions & 49 deletions React/Views/RCTShadowView.m
Original file line number Diff line number Diff line change
Expand Up @@ -154,35 +154,6 @@ static void RCTProcessMetaPropsBorder(const YGValue metaProps[META_PROP_COUNT],
YGNodeStyleSetBorder(node, YGEdgeAll, metaProps[META_PROP_ALL].value);
}

// The absolute stuff is so that we can take into account our absolute position when rounding in order to
// snap to the pixel grid. 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.left) + 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.

- (void)applyLayoutNode:(YGNodeRef)node
viewsWithNewFrame:(NSMutableSet<RCTShadowView *> *)viewsWithNewFrame
absolutePosition:(CGPoint)absolutePosition
Expand Down Expand Up @@ -218,26 +189,8 @@ - (void)applyLayoutWithFrame:(CGRect)frame
viewsWithUpdatedLayout:(NSMutableSet<RCTShadowView *> *)viewsWithUpdatedLayout
absolutePosition:(CGPoint)absolutePosition
{
CGPoint absoluteTopLeft = {
absolutePosition.x + frame.origin.x,
absolutePosition.y + frame.origin.y
};

CGPoint absoluteBottomRight = {
absolutePosition.x + frame.origin.x + frame.size.width,
absolutePosition.y + frame.origin.y + frame.size.height
};

CGRect roundedFrame = {{
RCTRoundPixelValue(frame.origin.x),
RCTRoundPixelValue(frame.origin.y),
}, {
RCTRoundPixelValue(absoluteBottomRight.x - absoluteTopLeft.x),
RCTRoundPixelValue(absoluteBottomRight.y - absoluteTopLeft.y)
}};

if (!CGRectEqualToRect(_frame, roundedFrame) || _layoutDirection != layoutDirection) {
_frame = roundedFrame;
if (!CGRectEqualToRect(_frame, frame) || _layoutDirection != layoutDirection) {
_frame = frame;
_layoutDirection = layoutDirection;
[viewsWithUpdatedLayout addObject:self];
}
Expand Down

0 comments on commit ceb1d1c

Please sign in to comment.