From: Panayotis K. <pan...@pa...> - 2011-10-10 11:33:04
|
On 10 Οκτ 2011, at 4:37 π.μ., Aaron VonderHaar wrote: > Wow, that's brilliant work! This is a great direction to be moving in. > > I took a quick look and had a few comments: > > ---------------------------------- > UIColor.java: > /** > * - (UIColor *)initWithHue:(CGFloat)hue > saturation:(CGFloat)saturation brightness:(CGFloat)brightness > alpha:(CGFloat)alpha; > * - (UIColor *)initWithRed:(CGFloat)red green:(CGFloat)green > blue:(CGFloat)blue alpha:(CGFloat)alpha; > */ > public UIColor(float arg1, float arg2, float arg3, float arg4, > UIColor.Colorspace colorspace) {} > > This looks a bit strange to me... it's a ctor that maps to two objc > methods. So apparently the inner enum colorspace is automatically > generated just for the purpose of distinguishing these constructors? > > I think it would make more sense to have the generated colorspace > parameter be the first parameter rather than the last. Exactly. Since we do not have something like "name override" in Java constructors, we need to distinguish them somehow. As you said, these enums are created semi-automatically, with external help (to provide the actual enum type). If you can think of a better approach, I am all ears! Of course, having the enum in the beginning is very easy to do, as long as we all agree to that. > ---------------------------------- > CGFont.java: > /** > * bool CGFontGetGlyphBBoxes(CGFontRef font, const CGGlyph glyphs[], > size_t count, CGRect bboxes[]) ; > */ > public boolean getGlyphBBoxes(short[] glyphs, int count, > Reference<CGRect> bboxes) > > The mapping from CGRect[] to Reference<CGRect> at first glance didn't > seem correct, but inspecting Reference.java that it is basically a > stripped down type of List. Could we use array or ArrayLists instead > of making a new container class? If not, could Reference be renamed > to make it clear that it represents an array of values rather than a > single value. Although I see that both "const SomeType arg[]" and > "const SomeType* arg" are translated to "References<SomeType>", since > they are functionally identical in C, but are semantically different. > (There are many instances of this.) Yes, this is a very serious issue. As you said, in terms of C, there is no way to distinguish [] from * Not only that, but in many locations the API defines a * for a table, or even a nil terminated list. I don't even want to go in details about reference for structs,double reference or even typedefs… The whole is a mess and with automatic tools this was the best I could do. Now, about the Reference object, it is primary a "reference", not an "array". It just happens in C that arrays are actually references to the first item in a list. References are more common to the iOS API than tables, since NSArray is used usually instead (and is properly mapped to a "List"). These references (like NSError **) are used as "call by reference" arguments. So it's primary work is to provide a "pointer" and not a "table". What I actually did to automatically solve this issue, was a "trick", to support both references and arrays. The same object has behavior as a reference (with the get() and set() methods, without index) and as an array (the same methods with an index parameter). If you think that an ArrayList will be more appropriate than a fixed array, this is something that could change. I was also thinking of another solution - make everything as arrays. Then the callback variables will be a one-dimension, size-one arrays, but this will look a bit strange at first. If we want to do something more elegant that this (which I agree), I will be happy to provide an "XML API", in which will specifically define all those methods that need to be fine tuned. It won't be an easy job though. > ---------------------------------- > Constant values are missing, for example > CGContext.java: > /** > * void CGContextSetLineJoin(CGContextRef c, CGLineJoin join) ; > */ > public void setLineJoin(int join) > > The CGLineJoin constants (kCGLineJoinMiter, kCGLineJoinRound, > kCGLineJoinBevel) are not defined anywhere. These are defined in > CGContext.h as enum CGLineJoin. I don't know what all the > implications are, but if this could be made into a Java enum instead > of converting it to int, I think that would be great. (There are many > other such enums.) Yes you are right, they are not in the output, since it is still under discussion (not only enums but also external typedefs to constant values). The first thought (I though I said it myself to this list) was exactly what you said. Create Java enums that will do exactly that. The problem is that, many enums can be combined between them and represent different states. So they need to be left as int. In the sake of universality, I prefer to keep them as int, which will be much easier for the C backend too. > ---------------------------------- > CGContext.java: > /** > * void CGContextSelectFont(CGContextRef c, const char *name, CGFloat > size, CGTextEncoding textEncoding) ; > */ > public void selectFont(byte[] name, float size, int textEncoding) > > I'd really like to see (const char*) parameters changed to String > instead of byte[], although I wonder if it will be possible to do this > conversion automatically in the C wrappers in a way that will work in > all cases. (There are many instances of this.) I understand, and I absolutely disagree :) The reason is that char* is NOT a String, it is a byte[]. String is a unicode stream of (Java's) char, while char is 2-bytes variable instead of 1-byte C's char. In English language there is no real difference, but for every other language this is a serious problem (including my mother tongue). I wanted with the API to make clear that "char* is not a String - if you want to do something like this, do it with your own responsibility". Having said that, it would be indeed nice to have functions that the developer can call and convert between byte[] and String - oh wait, here they are! :) http://download.oracle.com/javase/1,5.0/docs/api/java/lang/String.html#String%28byte[]%29 http://download.oracle.com/javase/1,5.0/docs/api/java/lang/String.html#getBytes%28%29 > ---------------------------------- > UIKit.java: > /** > * CGContextRef UIGraphicsGetCurrentContext(void); > */ > public static CGContext UIGraphicsGetCurrentContext() > > I was expecting this to be UIGraphics.getCurrentContext(), rather than > UIKit.UIGraphicsGetCurrentContext(). But this was discussed > previously on the list, and the size of the UIKit grouping seems good > (not too large). So for this I would just want to make sure there is > some clear documentation about where to find methods like this. Yes, the idea is "if an object exists, put it there, if not, put it in the framework object:. Thank you for the constructive comments! |