Monday, 21 May 2007

Refactoring work

First planned work for this summer was to refactor filter renderer initialization. The old initialization code was a monolithic piece of code copy-pasted in three places in codebase. (nr-arena-shape.cpp, nr-arena-group.cpp and nr-arena-image.cpp) While not bad in itself, each new filter primitive implemented would have grown that code by a dozen lines or so. It would soon have grown to an unmaintainable size.

So, the new approach: SPFilter has a single method sp_filter_build_renderer, which will initialize given renderer object (NR::Filter) to a correct state. Calling this method is all that needs to be done in those three nr-arena-* classes to set the correct filter renderer state.

The inside workings of sp_filter_build_renderer are as follows: each filter primitive (SPFilterPrimitive subclasses) has a build_renderer virtual function that will add the correct NR::FilterPrimitive object in the filter renderer. This function in turn calls sp_filter_primitive_renderer_common, which will do the part of initialization, which is common for all filter primitives.

Also, I looked into how modification messages are handles in document level filter objects. This actually took more time than the actual refactoring work, as it required me quite a few recompiles and plenty debugging to figure out, how these messages are propagated. Well, now these messages are propagated from SPGaussianBlur and SPFeBlend. This means that updating the underlying xml nodes for these two filters will cause the image on screen be updated. This will allow for further improvements, for example making the blur slider update the existing filter instead of creating a new filter every time the slider is changed.

Now that I've done this refactoring work, I will look into making in-parameter for filter primitives work. There is now the single method sp_filter_primitive_renderer_common where I should make this change to, so it will be somewhat simpler than it would have been before this refactoring work.

Also, I probably should write documentation, telling basically the same things about this refactored initialization routine, I've told in this blog post, just more detailed.

No comments: