Patch attached.
Created attachment 14661 [details] Implement PathCairo
Comment on attachment 14661 [details] Implement PathCairo are you sure strokeContains and strokeBoundingRect are meant to be svg only?
(In reply to comment #2) > (From update of attachment 14661 [details] [edit]) > are you sure strokeContains and strokeBoundingRect are meant to be svg only? > These functions are SVG-specific but it could be argued that they belong in graphics/svg/cairo (which we do not have yet). If you're more comfortable leaving the code out for now, that's fine, otherwise it'll be useful to have the code around for when we start to work on SVG support.
Created attachment 14663 [details] Implement PathCairo, take two
Comment on attachment 14663 [details] Implement PathCairo, take two I'm looking at this and i don't like the need to have different handling of the PlatformPath, and currently it looks like you're begging for memory leaks. i would suggest typedef struct cairopath { cairo_t* m_context; cairopath() { cairo_surface_t* pathSurface = cairo_image_surface_create(CAIRO_FORMAT_A8, 1, 1); m_cr = cairo_create(pathSurface); moveTo(FloatPoint(0, 0)); } ~cairopath() { //appriate delete stuff } } PlatformPath; then do everything in terms of that, that way you don't need ifdefs in the actual definition part of the header. This is an ugly ugly byproduct of cairo's borked path api :-/ It's far to similar to the canvas api for my liking :-(
Created attachment 14682 [details] Implement PathCairo, take three This fixes some leaks in the previous patch, and also takes the approach I think olliej was supporting -- though I'm not too sure about the verbosity it adds without a clear gain in readability.
Will attempt to review later The reason for having a CairoPath struct is not to enhance the readability of the gtk stuff -- although if you made the cairo path stuff have appropriate arcTo, etc methods on it it *might*, i'm not sure if svg uses the same path object -- if it did it might make life easier/cleaner later on The real reason is it reduces if defs in the platform headers -- which reduces the possiblity of requiring platform hacks elsewhere
Comment on attachment 14682 [details] Implement PathCairo, take three you could #define notImplemented() ASSERT(!"Not implemented"); for non gdk cairo... I believe all platforms have ASSERT I would be tempted to say that you shoud add CairoPath.h to define the CairoPath struct, and then include that file, that should minimise changes to the platform header. But otherwise r=me
Created attachment 14723 [details] Implement PathCairo, take four Generalises notImplemented() and splits out CairoPath into CairoPath.h, minimising modifications to Path.h as recommended in the review.
Landed, with formatting corrections, in r21828.