> Actually there have been very few interface changes, more moving code > around and adding piles of new code. The libdevice internal API is new > in it's entirety. I do plan to add more doxygen documentation as I do > the merge, and you'll see the commits in the email list.
I can't currently compile this code because I don't have openvg on my machines. I took a look at random at OpenVGStyle.h ( http://git.savannah.gnu.org/cgit/gnash.git/tree/librender/openvg/OpenVGStyle.h?h=openvg ) and discovered that it hasn't really got the point of static visitors for a boost::variant. These are supposed to be designed to handle each type differently, much like virtual functions do. They aren't supposed to provide redundant getter-setters for each member of each variant type. The agg renderer can be used as a reference for how to do it properly. Functions with a return type should actually return something (see line 167-168). I would hope the compiler warns about this when it's compiled. I also noticed that, as a consequence of treating static_visitors like extremely complicated getter-setters, GradientFill ( http://git.savannah.gnu.org/cgit/gnash.git/tree/libcore/FillStyle.h?h=openvg )now has a new, undocumented function that duplicates existing functionality: > const GradientRecords &getRecords() const { > return _gradients; > } I don't have any real problem with replacing the existing accessors (which return the gradient stops by index), but I do have a problem with undocumented and unnecessary additions to a simple API: this makes code bases into a large mess (not to be confused with 'complex'). If the old functions are to be replaced, code in the existing three renderers needs to be adapted to the new accessor and the old accessors removed. The position of the reference operator is also inconsistent with the rest of Gnash. In this file: http://git.savannah.gnu.org/cgit/gnash.git/tree/librender/openvg/OpenVGBitmap.cpp?h=openvg the function from lines 128: > image::GnashImage& > OpenVGBitmap::image() > { > // GNASH_REPORT_FUNCTION; > if (_image) { > return *_image; > } > } lacks an appropriate return. I'd expect the compiler to say "Warning: control reaches end of non-void function" here. I'm not going to review the code extensively because, as Sandro said, it's currently not easy to see where the additions are. Before any further reading of this code, it would also be nice if the Real Programmer would fix at least the errors that his compiler is telling him about. bwy -- -- Free Flash, use Gnash http://www.gnu.org/software/gnash/ Benjamin Wolsey, Software Developer - http://benjaminwolsey.de C++ and Open-Source Flash blog - http://www.benjaminwolsey.de/bwysblog xmpp:[email protected] http://identi.ca/bwy
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Gnash-dev mailing list [email protected] http://lists.gnu.org/mailman/listinfo/gnash-dev
