GlyphsSDK icon indicating copy to clipboard operation
GlyphsSDK copied to clipboard

We need a named constant for 9223372036854775808

Open justanotherfoundry opened this issue 2 years ago • 8 comments

It seems Glyph internally uses 9223372036854775808, or LLONG_MAX as ObjC calls it, in certain scenarios to indicate “no value” or similar. I believe it would be much cleaner to have a named constant for this, and proper documentation.

For example, in my ObjC code, I am checking the return value of kerningForFontMasterID:LeftKey:RightKey:direction: against LLONG_MAX to determine whether a kerning pair exists. This has always felt like a dirty hack as it is undocumented and doesn’t use a clearly named constant. The Python wrapper simply uses if value > 1000000:. That’s even worse IMHO, as if the value came from an unreliable external source, and needs a sanity check.

The unit tests check some values against 9223372036854775807 (that’s a 7 at the end, i.e. LLONG_MAX-1). Why is this? What is the meaning?

Can I kindly ask to solve this in a cleaner, less hacky way by simply providing a named constant (in ObjC as well as Python). Thanks! – Tim

justanotherfoundry avatar Nov 15 '22 16:11 justanotherfoundry

9223372036854775807 is NSNotFound, a constant that Apple’s APIs are frequently using to indicate that there is no normal value (since in Objective-C ints, points, etc. cannot be nil).

florianpircher avatar Nov 16 '22 08:11 florianpircher

Thanks, that’s interesting.

This means:

  • In some cases, Glyphs internally uses LLONG_MAX.
  • In some cases, Glyphs internally uses NSNotFound (which is different from LLONG_MAX). At least, this is what the Python unit tests suggest.
  • Both of this is undocumented.
  • The Python object wrapper checks against 1000000.

That’s not satisfying, is it?

justanotherfoundry avatar Nov 16 '22 10:11 justanotherfoundry

In some cases, Glyphs internally uses NSNotFound (which is different from LLONG_MAX)

Where are you getting 9223372036854775808? LLONG_MAX is also 9223372036854775807.

florianpircher avatar Nov 16 '22 10:11 florianpircher

Now, that’s confusing. It took me a while to solve that puzzle:

kerningForFontMasterID:LeftKey:RightKey:direction: returns a CGFloat a.k.a. double • in my code, checking that returned double against LLONG_MAX a.k.a. NSNotFound returns true • printing that value shows 9223372036854775808, i.e. NSNotFound + 1

Can’t be? Yes, it can. The reason is – as far as I figured out – that NSNotFound can’t be stored in a double. Apparently the method tries to return NSNotFound but it actually returns NSNotFound + 1. During my test for equality, NSNotFound is lossily cast to a double, this is why the values appear to be identical even though they aren’t. So, I was unconsciously testing whether the method returns a lossy cast of NSNotFound by comparing it to another lossy cast of NSNotFound.

To me, this shows how hacky and fragile the current solution is. I feel uncomfortable working like this.

justanotherfoundry avatar Nov 16 '22 11:11 justanotherfoundry

The workaround is to check whether a value is equal to or less than NSNotFound. Still hacky, but there is no good solution to this in Objective-C. NaN values have their own set of problems, especially when working with Apple APIs. CGFLOAT_MAX works in some cases, but is unusual as a ‘nothing’ value on Apples platforms.

florianpircher avatar Nov 16 '22 11:11 florianpircher

Comparing floats is always a bit tricky. That's why I picked a (random) big (but smaller than NSNotFound) number as a threshold in the python code. As far as I understand, kerning values are stored as Int16, so the max value is 0x7FFF. So any value between that and NSNotFound is fine as threshold.

schriftgestalt avatar Nov 16 '22 22:11 schriftgestalt

Thanks for the explanations. I wouldn’t say comparing floats is always tricky, though. It would have been easy to define a large integer value (one that can be represented by a 32-bit float) as a named constant, let’s say NO_KERNING_VALUE or UNSPECIFIED_VALUE, then all code could safely check for equality instead of using a random threshold. This would make the code robust and readable.

justanotherfoundry avatar Nov 21 '22 07:11 justanotherfoundry

It might be better wrap the relevant methods so that they return None instead of NSNotFound.

But we can add a constant like MAX_KERN that you need to compare with a < instead of ==.

schriftgestalt avatar Nov 21 '22 09:11 schriftgestalt