agens-no/AGGeometryKit

NSValue+AGQuad is not NSCoder friendly

Closed this issue · 10 comments

Hi, thank you for this very useful project.

I've run into a minor issue resolving which would make the library better: NSValue+AGQuad encoding is based on union type and thus not NSCoder friendly.

I'm not sure what is the most elegant way of fixing this.

A quick but ugly (perhaps not C99 correct) fix would be to manually change type encoding in NSValue+AGQuad.m:

NSValue *value = [NSValue value:&q withObjCType:@encode(double[8])];

Tricky! That sounds like a solution... This should definetly be covered by an unit test... You can create a unit test and implement the change and issue a pull request. Or I can do it after vacation. :) up to you

On second thought, it might not be worth fixing, at least not here. The whole reason I wanted AGQuad to be NSCoder compatible is that I'm storing it inside UIDocument. Since one is very likely to use NSUndoManager with UIDocument, there will still be a problem because we can't use C unions with NSInvocation. I'm not yet sure what is the simplest way to solve this - I'll work some more on my project and let you know how I approach this.

On a similar note - what is the reason to use AGPoint instead of using CGPoint directly?

I see... Maybe a bad idea performancewise? Anyway, here it goes: What about using NSStringFromAGQuad and make a counterpart? Then you can decode and encode a string?

We are using AGPoint for double precision when calculating the transform.
The union is useful for iterating each point as you would do with arrays. I'm sure we can get rid of that...

I'll look at it in a couple of weeks, after vacation. Meanwhile I think you got your alternatives. :)

I definetly find the use case valid though :)

Thank you for a response. It's not really important for me (I went with NSValue hack above), I just report what I've found for community's sake. Enjoy your vacation!

Great!

I'm back from vacation now. I've written a unit test where I got the same error as you. I have now fixed this issue by using a plain double[8] array... Let me know what you think and we can improve it. Maybe not the best way to do it performancewise, but at least the unit tests pass now for this case.

Thank you. I think it's fine; it's the only way to do it keeping the unions.