Comment 4 for bug 166720

Revision history for this message
Jaspervdg (jaspervdg) wrote :

Wish I found this earlier :) I encountered it while trying to fix another problem (see bug #216152).

To "solve" this I decided to simply remove sp_bpath_good, sp_bpath_clean and sp_bpath_check_subpath (in SVN). The latter is only used by the other two and is the one performing the actual checks. The other two are only used by sp_curve_new_from_foreign_bpath. This function is only used with pre-defined paths, from sp_curve_copy and from sp_curve_new_from_bpath. The first two uses shouldn't need the checks, the latter is used in the following cases:
 - Directly following sp_svg_read_path. As far as I know this (now) produces valid paths, and if it doesn't it should be fixed.
 - In path-chemistry.cpp it is used on a path that is a transformed version of an existing curve. As far as sp_bpath_check_subpath is concerned this curve/path should already be okay.
 - In sp-ellipse.cpp it is used on path data that was generated. This should also not need these kinds of sanity checks.
 - In libnrtype/Layout-TNG-Output.cpp it is used on path data that was generated based on a glyph from a font (which is ultimately generated by Path::MakeArtBPath in livarot/PathCutting.cpp as far as I can see). I expect that this should not need any sanity checks (if it does generate invalid paths something should probably be fixed).

Furthermore, all the sp_curve_moveto/lineto/etc. functions also seem pretty water-tight, so as far as I can see there is no point in performing these checks (anymore?). I also tried loading a few files, tested path combine and tried drawing some text. As far as I can see everything is indeed still functioning just fine.

I did find however that sp_curve_closepath uses the same dubious equality comparison (and perhaps other functions make the same mistake). It would be good to find more of these cases and make sure they use more suitable comparisons.