Avoid using UIView::frame in favor of bounds/center #691

2017-12-21T10:59:29-08:00

lelandrichardson commented

2017-12-21 10:59:29 -08:00

(Migrated from github.com)

Currently YogaKit sets UIView's frame property directly, which is actually a computed property. This works fine when the UIView doesnt have a transform property set, but when it does then UIKit will invert the transform, set the center/bounds then reapply the transform in such a way that the transformed view's frame will be the frame that was set. This is not usually what you want... you usually want to set the frame of the non-transformed view, and then apply the transform on that frame without affecting layout.

I believe this will improve performance since bounds and center are not calculated properties, but I have not measured/verified.

For people not using transforms, this should be a non-breaking change. If someone is using transforms with YogaKit, this may change the behavior, although I would argue this will produce the expected results (ie, transforms not being affected/altered by layout).

Currently YogaKit sets UIView's `frame` property directly, which is actually a computed property. This works fine when the `UIView` doesnt have a `transform` property set, but when it does then UIKit will invert the transform, set the center/bounds then reapply the transform in such a way that the transformed view's frame will be the frame that was set. This is not usually what you want... you usually want to set the frame of the non-transformed view, and then apply the transform on that frame without affecting layout. I believe this will improve performance since `bounds` and `center` are not calculated properties, but I have not measured/verified. For people not using transforms, this should be a non-breaking change. If someone is using transforms with YogaKit, this may change the behavior, although I would argue this will produce the expected results (ie, transforms not being affected/altered by layout).

👍 3

lucdion (Migrated from github.com) reviewed 2018-01-20 11:23:25 -08:00

YogaKit/Source/YGLayout.m

						
				@@ -460,0 +467,4 @@

				  };

				  view.bounds = (CGRect) {

				    .origin = CGPointZero,

lucdion (Migrated from github.com) commented

2018-01-20 11:23:24 -08:00

Hi @lelandrichardson, to handle correctly the view's layer.anchorPoint and to keep the view's bounds.origin, you should update your code to something like that. This thing is little tricky. UIScrollView's bounds.origin is not .zero when a contentOffset is set.

// NOTE: The center is offset by the layer.anchorPoint, so we have to take it into account.
    view.center = (CGPoint) {
        .x = YGRoundPixelValue(x + (width * view.layer.anchorPoint.x)),
        .y = YGRoundPixelValue(y + (height * view.layer.anchorPoint.y)),
    };

    // NOTE: We must set only the bounds' size and keep the origin.
    view.bounds = (CGRect) {
        .origin = view.bounds.origin,
        .size = {
            .width = YGRoundPixelValue(width),
            .height = YGRoundPixelValue(height),
        },
    };

Hi @lelandrichardson, to handle correctly the view's layer.anchorPoint and to keep the view's bounds.origin, you should update your code to something like that. This thing is little tricky. UIScrollView's bounds.origin is not .zero when a contentOffset is set. ``` // NOTE: The center is offset by the layer.anchorPoint, so we have to take it into account. view.center = (CGPoint) { .x = YGRoundPixelValue(x + (width * view.layer.anchorPoint.x)), .y = YGRoundPixelValue(y + (height * view.layer.anchorPoint.y)), }; // NOTE: We must set only the bounds' size and keep the origin. view.bounds = (CGRect) { .origin = view.bounds.origin, .size = { .width = YGRoundPixelValue(width), .height = YGRoundPixelValue(height), }, }; ```

NickGerleman commented

2022-10-04 06:25:20 -07:00

(Migrated from github.com)

It looks like this PR has had an unactioned request for changes for some time. Going to take the liberty of closing it out, but feel free to reopen.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DaddyFrosty/yoga#691