[cairo] cairo-java API review

[Owen Taylor]

I spent some time going through the cairo-java API in detail.
Unlike my recent Java-GNOME foray, this is meant to be comprehensive.

Obviously the Cairo C API is still in flux and this is reflected
in the comments ... both in places where the Java bindings haven't
caught up with the C API, and in places where there are future 
planned changes in the C API.

I'd like to see eventually some sort of guidelines document for
creating a binding for Cairo. While some parts of the binding
will be forced by the language, and others by the correspondence
to the C API, there are other things which are more or less
arbitrary and it would be nice if the different languages make
the same choices.

It would unfortunate if the Java bindings, for example, returned
a Distance object from userToDeviceDistance and the C# bindings
returned a Point object. 

As always my personal preference is to sticking as close to the
C API as possible, but no closer. Language bindings should resist
the temptation to "improve" the basic API to match the tastes of
the binder.

Regards,
					Owen

Affine
======

  Should be removed. 

  Instead, Matrix should have getXX, getXY ... getX0, getY0
  and a 6 element constructor.

BoundingBox
===========

  Should be renamed to Box. (cairo_box_t is similar internally, 
  but is integer)

  Or possibly should be replaced with a Rectangle type ... 
  Rectangle could expose both a p1,p2 representation and 
  a x,y,width,height representation.

Cairo
=====

 - Should be renamed to Context to match other language bindings

 - currentPath object in Cairo should be removed. It corresponds to
   a deprecated C API. 

 - Cairo(Cairo copy) should be removed and replaced with a copy() method
   that does the same as cairo_copy. (Which will likely get removed 
   at some point, but until then...)

 - {Clip/Fill/Stroke}Preserve are missing

 - The overloading of userToDevice as

     public Point userToDevice(Point p);
     double[] userToDevice(double x, double y);

   is troublesome ... the assocation between input and return values
   doesn't really make sense. I'd suggest only providing the former.
  
 - quadTo. hLineTo, vLineTo, ellipse(), rounderRectangle() should be removed. 
   (and rel versions) These could conflict with future additions to Cairo.
   If they are useful, they should be added in the C version.

 - setFont should be setFontFace

 - getTextExtents, getGlyphExtents should be textExtents, glyphExtents.
   They were not called get_<blah> in Cairo for a reason ... they
   don't retrieve a property, they compute a value.

 - I'm a little dubious of the rectangle(Point p, double width, double height),
   (Point p1, Point p2) would make more sense to me. (Could also add a
   Box version)

 - copyPath(), appendPath() are missing [see suggested API for Path, PathIterator, 
   PathElement below]

Distance
========

 - I'm a bit dubious of the existance of this class ... compare Java2D which
   uses Point2D for both AffineTransform.transform and 
   AffineTransform.deltaTransform

 - If kept, I'd suggested dx,dy rather than xDistance,yDistance for the
   field names.

FillRule
========

  Shouldn't have and/or/xor ... it's an enumeration not a bitfield
  (Same for Filter, Operator)

FreetypeFont
============

  Should be removed. The API in it has no relationship to the real api 
  for cairo_ft_font_face_t, cairo_ft_scaled_font_t. Unless fontconfig 
  and/or FreeType were bound there is no point in exposing cairo-ft.h

Glyph
=====

  Needs setters. The only use of Glyph in the API requires construction
  by the caller.

ImageSurface
============

  Missing:

    getWidth, getHeight, createForPNG(String filename), 
    createForPNG(InputStream stream)

    [or maybe java.nio for the latter instead?]

Matrix
======

 - rotate(double radians, double cx, double cy) should be removed. 
    (See comments on quadTo, etc.)

 - cairo_matrix_init_translate/scale/rotate should be mapped into
   static methods

    Matrix.createTranslate()/createScale()/createRotate()

- Matrix.scale(double ScaleFactor) is a *little* dubious.

Path
====

 Suggested addition. API:
  
   public PathIterator iterator();

PathIterator
============

 Suggested addition. API:

   public boolean hasNext();
   public PathElement next();

PathElement
===========

 Suggested addition. API:

   public PathElementType getType();
   public Point getPoint(int index);

PathElementType
===============

  Added enumeration, values MOVE_TO, LINE_TO, CURVE_TO, CLOSE_PATH

Pattern
=======

 Should be a heirarchy of classes

   Pattern
    [SolidPattern - doesn't exist in C]
    SurfacePattern
    Gradient
      LinearGradient
      RadialGradient

  To avoid having Pattern(4 doubles) return a linear gradient, Pattern(6 doubles)
  return a radial gradient, and to avoid having methods like addColorStop()
  which can be called on inappropriate pattern types.

  LinearGradient/RadialGradient constructors probably should have overloaded Point
  versions.

PdfSurface
=========

 - The capitalization of the name is strange to me ... I'd expect PDFSurface.

   (There doens't seem to be much of a standard in Java ... there are multiple 
   mixed things like SslRMIClientSocketFactory, HttpURLConnection ... it seems 
   that caps are often avoided for > 3 letter acroynm or two acronyms next 
   to each other)

 - The testing in PdfSurface.initialize() should be removed... it's inefficient
   and racy. The way to check if you can open a file is to *open* one. The
   cairo_write_func_t version can be used to use Java methods to open the
   file.

   (cairo_pdf_surface_create_for_callback() should map to 
    PdfSurface(OutputStream stream). Or maybe 
    PdfSurface(OutputStream stream, boolean close), since whether 
    you want cairo_destroy_func_t to close() or flush()
    is not always clear)

PngSurface
==========

 Should be removed. The C API is gone

PsSurface
=========

 I'd suggest removing this for now, since the C API is broken 
 (only a FILE * version) and the C implementation isn't useful.

ScaledFont
==========

 ScaledFont needs to call cairo_scaled_font_free() when finalized.

RGBColor
========

  Should be removed, since cairo_current_rgb_color() is gone, which
  is the only legitimate use. It may conflict with future exposed color
  objects in the Cairo API.

  (But it does provide an argument for PDFSurface, since it isn't
  RgbColor :-)

Surface
=======

 - binding cairo_surface_create_similar() to an overloaded constructor
   seems odd to me. I'd expect a createSimilar() method.

   (One way to think about it is that createSimilar() is a virtual
   operation implemented by subclasses of Surface(), so if Cairo was a pure 
   Java API, there would have to be a createSimilar() method even if 
   there was a convenience constructor, But the convenience of the constructor 
   seems small)

 - makeTarget should be removed. The ImageSurface implementation
   of it is very odd, but that doesn't really matter since it's not
   the way the Cairo API works or will work.

 - Surface.writeToPNG(String filename);  
   Surface.writeToPNG (OutputStream stream) are missing.

 - Surface.setRepeat/getMatrix/setMatrix/getMatrix/setFilter/getFilter
   are going to be removed from the public C API

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/cairo/attachments/20050430/b2356646/attachment.pgp