CGRectInset() returning NaN values

[indragiek]

When using any operator that uses CGRectInset() internally (e.g. -insetWidth:height:), if the rect being inset is too small to apply the inset, CGRectInset() will return a rect with some values set to NaN. This is especially dangerous when binding to rcl_frame or rcl_bounds as NSView throws an exception when attempting to set a frame with non-finite values.

What would be a good solution for this? One way is to just not use CGRectInset() and apply the insets manually (yielding negative values for some members, which won't throw an exception) or keep CGRectInset() and check for NaN values afterwards, replacing them with 0's or something suitable.

[indragiek]

#70 is caused by this same problem. Fixing this has the additional bonus of resolving that issue 🤘

[jspahrsummers]

You're probably seeing CGRectNull (or, at least, a rectangle passing the CGRectIsNull test).

yielding negative values for some members, which won't throw an exception

Most CGRect functions will standardize the provided rectangle, which would turn a negative width/height into a flipped rectangle. That's an incorrect (and probably very confusing) result.

check for NaN values afterwards, replacing them with 0's or something suitable

This could also cause problems, since some views don't gracefully handle having a zero size.

---

TBH, I don't have a good answer to this. No potential solution here (including "don't do that") is really satisfactory, since they all cause incorrect or obnoxious behavior of some sort.

[indragiek]

What about adding an operator that lets you pass in a parameter of a rect rect signal to use in place of CGRectNull?

[jspahrsummers]

To really do it right, it'd have to be an error not to provide a null mapping.

I guess we should just use an empty rectangle. That's what division does right now.

[indragiek]

Are you proposing that the null mapping be accepted as a parameter into any method that could yield a CGRectNull value or that it be an operator that can be applied to any signal that sends CGRects?

[jspahrsummers]

I'm saying that in order for a mapping to be valuable, it would have to be required anywhere a CGRectNull might appear — otherwise, nobody will know the variant is there, and we're back to the same shitty exceptions.

Of course, that'd be a huge burden for maintainers and consumers of the framework, and still doesn't prevent a null rectangle from slipping through when binding to frame/rcl_frame, so I think falling back to an empty rectangle is better.

[indragiek]

otherwise, nobody will know it's there, and we're back to the same shitty exceptions.

I think I disagree there, because this is nothing new to anyone who uses CGRectInset by itself. It's logical to assume that a signal operator that wraps CGRectInset would have the same caveats. An optional mapping using a separate operator would eliminate the burden of having to enforce it and give people an easy option for resolving the problem in cases where they know that it's possible for a CGRectNull value to be generated.

[indragiek]

Also, applying said operator at the end of the signal chain would guarantee that it does not get passed through to rcl_frame.

[jspahrsummers]

We can adjust the inset operator, but it absolutely should have a required nullRect: parameter (using a constant rectangle, since it's just meant as a fallback). If the user doesn't care about that case, they can pass in CGRectNull anyways.