Fix potential crash when creating a shadow path in box

[kylebshr]

The CGPath init has an assertion when creating a rounded rect that can cause a crash if the path is created with a frame width smaller than 2 * corner radius. The UIBezierPath init has no issues with this (though your path might look a little weird) and is much safer to use here. Even though UIBezierPath doesn't crash in this case, I updated the behavior to max out the radius at min(width, height) / 2, so you don't get the nonsensical diamond shape.

[bencochran]

It looks like UIBezierPath essentially does cornerRadius = min(cornerRadius, bounds.width / 2, bounds.height / 2) (that is, if corner radius is too big, it makes a capsule or circle shape). This is different that the cornerRadius of the layer which makes a round-diamond-thing in this case.

So with this code:

let frame = CGRect(x: 150, y: 150, width: 100, height: 100)
let bounds = CGRect(origin: .zero, size: frame.size)
let radius: CGFloat = 80
let view = UIView(frame: frame)
view.backgroundColor = .blue
view.layer.cornerRadius = radius
view.layer.shadowPath = UIBezierPath(
    roundedRect: bounds,
    cornerRadius: radius
).cgPath
view.layer.shadowOpacity = 1
view.layer.shadowColor = UIColor.red.cgColor
view.layer.shadowRadius = 0
view.layer.shadowOffset = .zero

we get:

Wondering if we should mimic UIBezierPath’s behavior for our corner radius (which gives a nicer handling of this case, and makes the shadow match the contents). Generally something like:

let frame = CGRect(x: 150, y: 150, width: 100, height: 100)
let bounds = CGRect(origin: .zero, size: frame.size)
let radius: CGFloat = 80
let actualRadius = min(radius, bounds.width / 2, bounds.height / 2)
let view = UIView(frame: frame)
view.backgroundColor = UIColor.blue.withAlphaComponent(0.8)
view.layer.cornerRadius = actualRadius
view.layer.shadowPath = UIBezierPath(
    roundedRect: bounds,
    cornerRadius: actualRadius
).cgPath
view.layer.shadowOpacity = 1
view.layer.shadowColor = UIColor.red.cgColor
view.layer.shadowRadius = 0
view.layer.shadowOffset = .zero

to produce

[kylebshr]

As weird of a use-case as it might be, I don't know that we should limit someone from creating that diamond shape; what do you think about just not setting a shadowPath when the radii is too big? It's a performance optimization I added recently, and works fine without setting it

[kylebshr]

Weirdly though, I'm not seeing the same behavior you are @bencochran - check out the snapshot image. Using a UIBezierPath and it's matching the shape of the layer 🤔

[kylebshr]

This test crashes on main on iOS 11

[kyleve]

The snapshot looks real funky... is that intentional?

[kylebshr]

That's the "diamond" shape without clipping - but am going to update to use the min of width/height

[kyleve]

Ah right; looks way better now

[kyleve]

I think I agree w/ ben here; the diamond seems really unexpected to me.

[kylebshr]

Fair enough 👍 will update to prevent that

…

On Sep 16, 2020, at 4:43 PM, Kyle Van Essen ***@***.***> wrote:  I think I agree w/ ben here; the diamond seems really unexpected to me. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[kyleve]

Fwiw, while this is less code; its also a bit only based on the fact that bounds is always at 0,0. If it was not at 0,0, this would be reporting the middle of the frame in its coordinate space, whereas the / 2 on the width and height makes this explict.

[kylebshr]

Ah that makes sense.. just to make sure I understand, a rect of x: 10, y: 10, width: 20, height: 20 would have a midX of 15, right? Will go back to / 2 👍

[kyleve]

Yeah, it would

[kylebshr]

Alrighty, no more diamonds! Should be all set for review 👍

[kyleve]

Ah right; looks way better now