enable
enable copied to clipboard
Retire SWIG-based Agg backend in favour of Cython-based Celiagg
The SWIG-based Agg wrapper is showing its age: it is using an old version of Agg; it doesn't work with modern GCC or SWIG versions, the build process is idiosyncratic, and it is overall hard to maintain and modify. Celiagg is a Cython-based alternative wrapper of the Agg backend with better overall architecture and, importantly, is a stand-along package which is independent of Kiva. There is already a Celiagg backend for Kiva.
We should switch the default image backend to use Celiagg, and deprecate and retire the SWIG-based Agg wrapper.
Most of the pieces are in place for this, but it will likely be a multi-phase process:
- Audit Celiagg for feature compatibility (does it do everything right) and performance (we can take some performance degradation in exchange for reducing maintenance overhead, but 10x slower is too much).
- If there are discrepancies, fix them.
- Expose the current Agg backend as a separate-but-equivalent Kiva backend to the image backend ('oldagg' or 'swigagg' or something, not just 'agg').
- Switch the image backend to use Celiagg (while keeping the current celiagg backend)
- Deprecate the agg backend
- Remove the agg backend
#251 is connected here.
I believe the root issue is that GraphicsContext.draw_image is expecting a GraphicsContext instance as a possible argument. Unfortunately this is done in such a way which causes a hard dependency on the current agg backend in kiva.
Here's all the serious occurrences of context objects being blitted directly, as far as I can tell:
- https://github.com/enthought/enable/blob/4.8.1/kiva/agg/src/graphics_context.i#L719-L737
- https://github.com/enthought/enable/blob/4.8.1/kiva/quartz/ABCGI.pyx#L736-L737
- https://github.com/enthought/enable/blob/4.8.1/enable/component.py#L738-L775 (
self._backbufferis a context instance) Scrollbarhas lots ofdraw_imagecalls, but appears to be otherwise broken since theimage_formethod which can create those images is nowhere to be found.
Some refactoring (and probably deprecations) will need to be done to make changes here possible.
Can you clarify why you think there is a "hard dependency" on the current agg backend?
-
The first link just goes away if we blow away the current
aggbackend, so that's no problem. -
The second is just a branch that handles an
aggGraphicsContextif you happen to pass one in; you are never required or encouraged to. Now, I did have to do that at one point because there were places where someone explicitly used thekiva.agg.GraphicsContextArrayto draw something off-screen that they wanted blitted in. I think it would be more fruitful to hunt down those usages and determine whether they really care aboutkiva.agg; they can probably all just usekiva.celiaggas a drop-in. -
The third creates a
GraphicsContextof the same type as the passed-inGraphicsContext, so if we're drawing into akiva.celiagg.GraphicsContext, the backbuffer would be akiva.celiagg.GraphicsContext, which is a case that seems to be well-handled in thedraw_image()implementation. -
Scrollbaris indeed broken, but I'm pretty sure that the return value ofimage_for()was an array, not aGraphicsContext.
One of those places where kiva.agg.GraphicsContextArray is explicitly used is in the Chaco ImagePlot:
https://github.com/enthought/chaco/blob/4a6683521cce012b5df4830d64f41518798fc817/chaco/image_plot.py#L73-L74
There's no real reason that that must explicitly depend on kiva.agg. Instead, _compute_cached_image() could/should take the destination gc as an argument and do the same type dance as Component.
Perhaps the situation has improved in four years. The chaco code you linked to is somewhat concerning though. It makes any refactoring somewhat difficult.
I suspect the main thing to make the celiagg backend a drop-in replacement for these explicit uses of GraphicsContextArray is to expose self.gc.array as self.bmp_array. We do use GraphicsContextArray elsewhere (for example, in tests) for the specific purpose of accessing this array. This should be supported in the kiva.celiagg implementation, and we might as well use the kiva.agg interface for this.
Step 1 is now covered by the benchmark suite introduced in #647. Step 2 will follow shortly. Step 3 is covered by #669.
Steps 4-6 come later. I'm not sure how we should schedule those.