Refactoring hatch to remove memory leaks (bad GTKish casting macros) and uninitialized value.

(bzr r13622)

5 files changed, 501 insertions, 377 deletions

diff --git a/src/extension/internal/cairo-render-context.cpp b/src/extension/internal/cairo-render-context.cpp
index 7e61cdbbb..cb414b3b2 100644
--- a/src/extension/internal/cairo-render-context.cpp
+++ b/src/extension/internal/cairo-render-context.cpp
@@ -1132,8 +1132,8 @@ CairoRenderContext::_createPatternPainter(SPPaintServer const *const paintserver
 
 cairo_pattern_t*
 CairoRenderContext::_createHatchPainter(SPPaintServer const *const paintserver, Geom::OptRect const &pbox) {
-    g_assert( SP_IS_HATCH(paintserver) );
-    SPHatch *hatch = SP_HATCH(paintserver);
+    SPHatch const *hatch = dynamic_cast<SPHatch const *>(paintserver);
+    g_assert( hatch );
 
     g_assert(hatch->pitch() > 0);
 
@@ -1141,7 +1141,9 @@ CairoRenderContext::_createHatchPainter(SPPaintServer const *const paintserver,
     Inkscape::Drawing drawing;
     unsigned dkey = SPItem::display_key_new(1);
 
-    hatch->show(drawing, dkey, pbox);
+    // TODO need to refactor 'evil' referenced code for const correctness.
+    SPHatch *evil = const_cast<SPHatch *>(hatch);
+    evil->show(drawing, dkey, pbox);
 
     SPHatch::RenderInfo render_info = hatch->calculateRenderInfo(dkey);
     Geom::Rect tile_rect = render_info.tile_rect;
@@ -1177,8 +1179,7 @@ CairoRenderContext::_createHatchPainter(SPPaintServer const *const paintserver,
     pattern_ctx->transform(Geom::Translate(-overflow_right_strip, 0.0));
     pattern_ctx->pushState();
 
-    std::vector<SPHatchPath *> children;
-    hatch->hatchPaths(children);
+    std::vector<SPHatchPath *> children(evil->hatchPaths());
 
     for (int i = 0; i < overflow_steps; i++) {
         for (std::vector<SPHatchPath *>::iterator iter = children.begin(); iter != children.end(); iter++) {
@@ -1200,7 +1201,7 @@ CairoRenderContext::_createHatchPainter(SPPaintServer const *const paintserver,
     pattern_transform = render_info.pattern_to_user_transform.inverse() * drawing_transform;
     ink_cairo_pattern_set_matrix(result, pattern_transform);
 
-    hatch->hide(dkey);
+    evil->hide(dkey);
 
     delete pattern_ctx;
     return result;
@@ -1260,7 +1261,7 @@ CairoRenderContext::_createPatternForPaintServer(SPPaintServer const *const pain
         }
     } else if (SP_IS_PATTERN (paintserver)) {
         pattern = _createPatternPainter(paintserver, pbox);
-    } else if (SP_IS_HATCH (paintserver)) {
+    } else if ( dynamic_cast<SPHatch const *>(paintserver) ) {
         pattern = _createHatchPainter(paintserver, pbox);
     } else {
         return NULL;
@@ -1331,8 +1332,8 @@ CairoRenderContext::_setFillStyle(SPStyle const *const style, Geom::OptRect cons
     if (paint_server && paint_server->isValid()) {
 
         g_assert(SP_IS_GRADIENT(SP_STYLE_FILL_SERVER(style))
-            || SP_IS_PATTERN(SP_STYLE_FILL_SERVER(style))
-            || SP_IS_HATCH(SP_STYLE_FILL_SERVER(style)));
+                 || SP_IS_PATTERN(SP_STYLE_FILL_SERVER(style))
+                 || dynamic_cast<SPHatch *>(SP_STYLE_FILL_SERVER(style)));
 
         cairo_pattern_t *pattern = _createPatternForPaintServer(paint_server, pbox, alpha);
         if (pattern) {
@@ -1369,7 +1370,7 @@ CairoRenderContext::_setStrokeStyle(SPStyle const *style, Geom::OptRect const &p
         g_assert( style->stroke.isPaintserver()
                   || SP_IS_GRADIENT(SP_STYLE_STROKE_SERVER(style))
                   || SP_IS_PATTERN(SP_STYLE_STROKE_SERVER(style))
-                  || SP_IS_HATCH(SP_STYLE_STROKE_SERVER(style)));
+                  || dynamic_cast<SPHatch *>(SP_STYLE_STROKE_SERVER(style)));
 
         cairo_pattern_t *pattern = _createPatternForPaintServer(SP_STYLE_STROKE_SERVER(style), pbox, alpha);
 
diff --git a/src/sp-hatch-path.cpp b/src/sp-hatch-path.cpp
index f7138fac2..8558b67f2 100644
--- a/src/sp-hatch-path.cpp
+++ b/src/sp-hatch-path.cpp
@@ -1,8 +1,11 @@
-/** @file
+/**
+ * @file
  * SVG <hatchPath> implementation
- *//*
+ */
+/*
  * Author:
  *   Tomasz Boczkowski <penginsbacon@gmail.com>
+ *   Jon A. Cruz <jon@joncruz.org>
  *
  * Copyright (C) 2014 Tomasz Boczkowski
  *
@@ -37,24 +40,31 @@
 #include "sp-factory.h"
 
 namespace {
-SPObject* createHatchPath() {
+
+SPObject* createHatchPath()
+{
     return new SPHatchPath();
 }
 
 bool hatchRegistered = SPFactory::instance().registerObject("svg:hatchPath", createHatchPath);
-}
+
+} // namespace
 
 SPHatchPath::SPHatchPath()
-    : _curve(NULL)
-    , _continuous(false)
+    : offset(),
+      _display(),
+      _curve(NULL),
+      _continuous(false)
 {
     offset.unset();
 }
 
-SPHatchPath::~SPHatchPath() {
+SPHatchPath::~SPHatchPath()
+{
 }
 
-void SPHatchPath::setCurve(SPCurve *new_curve, bool owner) {
+void SPHatchPath::setCurve(SPCurve *new_curve, bool owner)
+{
     if (_curve) {
         _curve = _curve->unref();
     }
@@ -67,21 +77,23 @@ void SPHatchPath::setCurve(SPCurve *new_curve, bool owner) {
         }
     }
 
-    this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+    requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
 }
 
-void SPHatchPath::build(SPDocument* doc, Inkscape::XML::Node* repr) {
+void SPHatchPath::build(SPDocument* doc, Inkscape::XML::Node* repr)
+{
     SPObject::build(doc, repr);
 
-    this->readAttr("d");
-    this->readAttr("offset");
-    this->readAttr( "style" );
+    readAttr("d");
+    readAttr("offset");
+    readAttr( "style" );
 
     style->fill.setNone();
 }
 
-void SPHatchPath::release() {
-    for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+void SPHatchPath::release()
+{
+    for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
         delete iter->arenaitem;
         iter->arenaitem = NULL;
     }
@@ -89,7 +101,8 @@ void SPHatchPath::release() {
     SPObject::release();
 }
 
-void SPHatchPath::set(unsigned int key, const gchar* value) {
+void SPHatchPath::set(unsigned int key, const gchar* value)
+{
     switch (key) {
     case SP_ATTR_D:
         if (value) {
@@ -98,24 +111,24 @@ void SPHatchPath::set(unsigned int key, const gchar* value) {
             SPCurve *curve = new SPCurve(pv);
 
             if (curve) {
-                this->setCurve(curve, true);
+                setCurve(curve, true);
                 curve->unref();
             }
         } else {
-            this->setCurve(NULL, true);
+            setCurve(NULL, true);
         }
 
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_OFFSET:
         offset.readOrUnset(value);
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     default:
         if (SP_ATTRIBUTE_IS_CSS(key)) {
-            sp_style_read_from_object(this->style, this);
+            sp_style_read_from_object(style, this);
             requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_STYLE_MODIFIED_FLAG);
         } else {
             SPObject::set(key, value);
@@ -125,41 +138,44 @@ void SPHatchPath::set(unsigned int key, const gchar* value) {
 }
 
 
-void SPHatchPath::update(SPCtx* ctx, unsigned int flags) {
-
+void SPHatchPath::update(SPCtx* ctx, unsigned int flags)
+{
     if (flags & (SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_STYLE_MODIFIED_FLAG | SP_OBJECT_VIEWPORT_MODIFIED_FLAG)) {
         flags &= ~SP_OBJECT_USER_MODIFIED_FLAG_B;
     }
 
     if (flags & (SP_OBJECT_STYLE_MODIFIED_FLAG | SP_OBJECT_VIEWPORT_MODIFIED_FLAG)) {
-        if (this->style->stroke_width.unit == SP_CSS_UNIT_PERCENT) {
+        if (style->stroke_width.unit == SP_CSS_UNIT_PERCENT) {
             //TODO: Check specification
 
-            SPItemCtx *ictx = (SPItemCtx *) ctx;
-            double const aw = 1.0 / ictx->i2vp.descrim();
-            this->style->stroke_width.computed = this->style->stroke_width.value * aw;
+            SPItemCtx *ictx = static_cast<SPItemCtx *>(ctx);
+            double const aw = (ictx) ? 1.0 / ictx->i2vp.descrim() : 1.0;
+            style->stroke_width.computed = style->stroke_width.value * aw;
 
-            for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
-                iter->arenaitem->setStyle(this->style);
+            for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
+                iter->arenaitem->setStyle(style);
             }
         }
     }
 
     if (flags & (SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_PARENT_MODIFIED_FLAG)) {
-        for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+        for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
             _updateView(*iter);
         }
     }
 }
 
-bool SPHatchPath::isValid() const {
+bool SPHatchPath::isValid() const
+{
     if (_curve && (_repeatLength() <= 0)) {
         return false;
+    } else {
+        return true;
     }
-    return true;
 }
 
-Inkscape::DrawingItem *SPHatchPath::show(Inkscape::Drawing &drawing, unsigned int key, Geom::OptInterval extents) {
+Inkscape::DrawingItem *SPHatchPath::show(Inkscape::Drawing &drawing, unsigned int key, Geom::OptInterval extents)
+{
     Inkscape::DrawingShape *s = new Inkscape::DrawingShape(drawing);
     _display.push_front(View(s, key));
     _display.front().extents = extents;
@@ -169,8 +185,9 @@ Inkscape::DrawingItem *SPHatchPath::show(Inkscape::Drawing &drawing, unsigned in
     return s;
 }
 
-void SPHatchPath::hide(unsigned int key) {
-    for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+void SPHatchPath::hide(unsigned int key)
+{
+    for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
         if (iter->key == key) {
             delete iter->arenaitem;
             _display.erase(iter);
@@ -181,8 +198,9 @@ void SPHatchPath::hide(unsigned int key) {
     g_assert_not_reached();
 }
 
-void SPHatchPath::setStripExtents(unsigned int key, Geom::OptInterval const &extents) {
-    for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+void SPHatchPath::setStripExtents(unsigned int key, Geom::OptInterval const &extents)
+{
+    for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
         if (iter->key == key) {
             iter->extents = extents;
             break;
@@ -190,18 +208,19 @@ void SPHatchPath::setStripExtents(unsigned int key, Geom::OptInterval const &ext
     }
 }
 
-Geom::Interval SPHatchPath::bounds() const {
+Geom::Interval SPHatchPath::bounds() const
+{
     Geom::OptRect bbox;
     Geom::Interval result;
 
     Geom::Affine transform = Geom::Translate(offset.computed, 0);
-    if (!this->_curve) {
+    if (!_curve) {
         SPCurve test_curve;
         test_curve.moveto(Geom::Point(0, 0));
         test_curve.moveto(Geom::Point(0, 1));
         bbox = bounds_exact_transformed(test_curve.get_pathvector(), transform);
     } else {
-        bbox = bounds_exact_transformed(this->_curve->get_pathvector(), transform);
+        bbox = bounds_exact_transformed(_curve->get_pathvector(), transform);
     }
 
     gdouble stroke_width = style->stroke_width.computed;
@@ -210,8 +229,9 @@ Geom::Interval SPHatchPath::bounds() const {
     return result;
 }
 
-SPCurve *SPHatchPath::calculateRenderCurve(unsigned key) const {
-    for (ConstViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+SPCurve *SPHatchPath::calculateRenderCurve(unsigned key) const
+{
+    for (ConstViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
         if (iter->key == key) {
             return _calculateRenderCurve(*iter);
         }
@@ -220,31 +240,32 @@ SPCurve *SPHatchPath::calculateRenderCurve(unsigned key) const {
     return NULL;
 }
 
-gdouble SPHatchPath::_repeatLength() const {
-    if (!_curve) {
-        return 0;
-    }
+gdouble SPHatchPath::_repeatLength() const
+{
+    gdouble val = 0;
 
-    if (!_curve->last_point()) {
-        return 0;
+    if (_curve && _curve->last_point()) {
+        val = _curve->last_point()->y();
     }
 
-    return _curve->last_point()->y();
+    return val;
 }
 
-void SPHatchPath::_updateView(View &view) {
+void SPHatchPath::_updateView(View &view)
+{
     SPCurve *calculated_curve = _calculateRenderCurve(view);
 
     Geom::Affine offset_transform = Geom::Translate(offset.computed, 0);
     view.arenaitem->setTransform(offset_transform);
     style->fill.setNone();
-    view.arenaitem->setStyle(this->style);
+    view.arenaitem->setStyle(style);
     view.arenaitem->setPath(calculated_curve);
 
     calculated_curve->unref();
 }
 
-SPCurve *SPHatchPath::_calculateRenderCurve(View const &view) const {
+SPCurve *SPHatchPath::_calculateRenderCurve(View const &view) const
+{
     SPCurve *calculated_curve = new SPCurve;
 
     if (!view.extents) {
@@ -265,7 +286,7 @@ SPCurve *SPHatchPath::_calculateRenderCurve(View const &view) const {
             segment->transform(Geom::Translate(0, initial_y));
 
             Geom::Affine step_transform = Geom::Translate(0, repeatLength);
-            for (int i = 0; i < segment_cnt; i++) {
+            for (int i = 0; i < segment_cnt; ++i) {
                 if (_continuous) {
                     calculated_curve->append_continuous(segment, 0.0625);
                 } else {
@@ -281,7 +302,8 @@ SPCurve *SPHatchPath::_calculateRenderCurve(View const &view) const {
 }
 
 
-void SPHatchPath::_readHatchPathVector(char const *str, Geom::PathVector &pathv, bool &continous_join) {
+void SPHatchPath::_readHatchPathVector(char const *str, Geom::PathVector &pathv, bool &continous_join)
+{
     if (!str) {
         return;
     }
@@ -314,8 +336,16 @@ void SPHatchPath::_readHatchPathVector(char const *str, Geom::PathVector &pathv,
 }
 
 SPHatchPath::View::View(Inkscape::DrawingShape *arenaitem, int key)
-        : arenaitem(arenaitem), key(key)
+    : arenaitem(arenaitem),
+      extents(),
+      key(key)
+{
+}
+
+SPHatchPath::View::~View()
 {
+    // remember, do not delete arenaitem here
+    arenaitem = NULL;
 }
 
 
diff --git a/src/sp-hatch-path.h b/src/sp-hatch-path.h
index 57b3a8237..11fc274ec 100644
--- a/src/sp-hatch-path.h
+++ b/src/sp-hatch-path.h
@@ -1,8 +1,11 @@
-/** @file
+/**
+ * @file
  * SVG <hatchPath> implementation
- *//*
+ */
+/*
  * Author:
  *   Tomasz Boczkowski <penginsbacon@gmail.com>
+ *   Jon A. Cruz <jon@joncruz.org>
  *
  * Copyright (C) 2014 Tomasz Boczkowski
  *
@@ -26,43 +29,44 @@ class DrawingShape;
 
 }
 
-#define SP_HATCH_PATH(obj) (dynamic_cast<SPHatchPath*>((SPObject*)obj))
-#define SP_IS_HATCH_PATH(obj) (dynamic_cast<const SPHatchPath*>((SPObject*)obj) != NULL)
-
 class SPHatchPath : public SPObject {
 public:
     SPHatchPath();
-	virtual ~SPHatchPath();
+    virtual ~SPHatchPath();
 
-	SVGLength offset;
+    SVGLength offset;
 
-	void setCurve(SPCurve *curve, bool owner);
+    void setCurve(SPCurve *curve, bool owner);
 
     bool isValid() const;
 
     Inkscape::DrawingItem *show(Inkscape::Drawing &drawing, unsigned int key, Geom::OptInterval extents);
     void hide(unsigned int key);
 
-	void setStripExtents(unsigned int key, Geom::OptInterval const &extents);
-	Geom::Interval bounds() const;
+    void setStripExtents(unsigned int key, Geom::OptInterval const &extents);
+    Geom::Interval bounds() const;
 
-	SPCurve *calculateRenderCurve(unsigned key) const;
+    SPCurve *calculateRenderCurve(unsigned key) const;
 
 protected:
-	virtual void build(SPDocument* doc, Inkscape::XML::Node* repr);
-	virtual void release();
-	virtual void set(unsigned int key, const gchar* value);
-	virtual void update(SPCtx* ctx, unsigned int flags);
+    virtual void build(SPDocument* doc, Inkscape::XML::Node* repr);
+    virtual void release();
+    virtual void set(unsigned int key, const gchar* value);
+    virtual void update(SPCtx* ctx, unsigned int flags);
 
 private:
-    struct View {
+    class View {
+    public:
         View(Inkscape::DrawingShape *arenaitem, int key);
         //Do not delete arenaitem in destructor.
 
+        ~View();
+
         Inkscape::DrawingShape *arenaitem;
         Geom::OptInterval extents;
         unsigned int key;
     };
+
     typedef std::list<SPHatchPath::View>::iterator ViewIterator;
     typedef std::list<SPHatchPath::View>::const_iterator ConstViewIterator;
     std::list<View> _display;
diff --git a/src/sp-hatch.cpp b/src/sp-hatch.cpp
index b007fc846..4a8707e08 100644
--- a/src/sp-hatch.cpp
+++ b/src/sp-hatch.cpp
@@ -1,8 +1,11 @@
-/** @file
+/**
+ * @file
  * SVG <hatch> implementation
- *//*
- * Author:
+ */
+/*
+ * Authors:
  *   Tomasz Boczkowski <penginsbacon@gmail.com>
+ *   Jon A. Cruz <jon@joncruz.org>
  *
  * Copyright (C) 2014 Tomasz Boczkowski
  *
@@ -35,65 +38,73 @@
 #include "sp-factory.h"
 
 namespace {
+
 SPObject* createHatch() {
     return new SPHatch();
 }
 
 bool hatchRegistered = SPFactory::instance().registerObject("svg:hatch", createHatch);
-}
+
+} // namespace
 
 SPHatch::SPHatch()
-        : SPPaintServer()
+    : SPPaintServer(),
+      href(),
+      ref(NULL), // avoiding 'this' in initializer list
+      _hatchUnits(UNITS_OBJECTBOUNDINGBOX),
+      _hatchUnits_set(false),
+      _hatchContentUnits(UNITS_USERSPACEONUSE),
+      _hatchContentUnits_set(false),
+      _hatchTransform(Geom::identity()),
+      _hatchTransform_set(false),
+      _x(),
+      _y(),
+      _pitch(),
+      _rotate(),
+      _modified_connection(),
+      _display()
 {
-    this->ref = new SPHatchReference(this);
-    this->ref->changedSignal().connect(sigc::mem_fun(this, &SPHatch::_onRefChanged));
-
-    this->_hatchUnits = UNITS_OBJECTBOUNDINGBOX;
-    this->_hatchUnits_set = false;
+    ref = new SPHatchReference(this);
+    ref->changedSignal().connect(sigc::mem_fun(this, &SPHatch::_onRefChanged));
 
-    this->_hatchContentUnits = UNITS_USERSPACEONUSE;
-    this->_hatchContentUnits_set = false;
-
-    this->_hatchTransform = Geom::identity();
-    this->_hatchTransform_set = false;
-
-    this->_x.unset();
-    this->_y.unset();
-    this->_pitch.unset();
-    this->_rotate.unset();
+    // TODO check that these should start already as unset:
+    _x.unset();
+    _y.unset();
+    _pitch.unset();
+    _rotate.unset();
 }
 
 SPHatch::~SPHatch() {
 }
 
-void SPHatch::build(SPDocument* doc, Inkscape::XML::Node* repr) {
+void SPHatch::build(SPDocument* doc, Inkscape::XML::Node* repr)
+{
     SPPaintServer::build(doc, repr);
 
-    this->readAttr("hatchUnits");
-    this->readAttr("hatchContentUnits");
-    this->readAttr("hatchTransform");
-    this->readAttr("x");
-    this->readAttr("y");
-    this->readAttr("pitch");
-    this->readAttr("rotate");
-    this->readAttr("xlink:href");
-    this->readAttr( "style" );
-
-    /* Register ourselves */
+    readAttr("hatchUnits");
+    readAttr("hatchContentUnits");
+    readAttr("hatchTransform");
+    readAttr("x");
+    readAttr("y");
+    readAttr("pitch");
+    readAttr("rotate");
+    readAttr("xlink:href");
+    readAttr( "style" );
+
+    // Register ourselves
     doc->addResource("hatch", this);
 }
 
-void SPHatch::release() {
-    if (this->document) {
+void SPHatch::release()
+{
+    if (document) {
         // Unregister ourselves
-        this->document->removeResource("hatch", this);
+        document->removeResource("hatch", this);
     }
 
-    std::vector<SPHatchPath *> children;
-    hatchPaths(children);
-    for (ViewIterator view_iter = _display.begin(); view_iter != _display.end(); view_iter++) {
-        for (ChildIterator child_iter = children.begin(); child_iter != children.end();
-                child_iter++) {
+    std::vector<SPHatchPath *> children(hatchPaths());
+    for (ViewIterator view_iter = _display.begin(); view_iter != _display.end(); ++view_iter) {
+        for (ChildIterator child_iter = children.begin(); child_iter != children.end(); ++child_iter) {
             SPHatchPath *child = *child_iter;
             child->hide(view_iter->key);
         }
@@ -101,20 +112,21 @@ void SPHatch::release() {
         view_iter->arenaitem = NULL;
     }
 
-    if (this->ref) {
-        this->_modified_connection.disconnect();
-        this->ref->detach();
-        delete this->ref;
-        this->ref = NULL;
+    if (ref) {
+        _modified_connection.disconnect();
+        ref->detach();
+        delete ref;
+        ref = NULL;
     }
 
     SPPaintServer::release();
 }
 
-void SPHatch::child_added(Inkscape::XML::Node* child, Inkscape::XML::Node* ref) {
+void SPHatch::child_added(Inkscape::XML::Node* child, Inkscape::XML::Node* ref)
+{
     SPObject::child_added(child, ref);
 
-    SPHatchPath *path_child = SP_HATCH_PATH(this->document->getObjectByRepr(child));
+    SPHatchPath *path_child = dynamic_cast<SPHatchPath *>(document->getObjectByRepr(child));
 
     if (path_child) {
         for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
@@ -130,102 +142,103 @@ void SPHatch::child_added(Inkscape::XML::Node* child, Inkscape::XML::Node* ref)
     //FIXME: notify all hatches that refer to this child set
 }
 
-void SPHatch::set(unsigned int key, const gchar* value) {
+void SPHatch::set(unsigned int key, const gchar* value)
+{
     switch (key) {
     case SP_ATTR_HATCHUNITS:
         if (value) {
             if (!strcmp(value, "userSpaceOnUse")) {
-                this->_hatchUnits = UNITS_USERSPACEONUSE;
+                _hatchUnits = UNITS_USERSPACEONUSE;
             } else {
-                this->_hatchUnits = UNITS_OBJECTBOUNDINGBOX;
+                _hatchUnits = UNITS_OBJECTBOUNDINGBOX;
             }
 
-            this->_hatchUnits_set = true;
+            _hatchUnits_set = true;
         } else {
-            this->_hatchUnits_set = false;
+            _hatchUnits_set = false;
         }
 
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_HATCHCONTENTUNITS:
         if (value) {
             if (!strcmp(value, "userSpaceOnUse")) {
-                this->_hatchContentUnits = UNITS_USERSPACEONUSE;
+                _hatchContentUnits = UNITS_USERSPACEONUSE;
             } else {
-                this->_hatchContentUnits = UNITS_OBJECTBOUNDINGBOX;
+                _hatchContentUnits = UNITS_OBJECTBOUNDINGBOX;
             }
 
-            this->_hatchContentUnits_set = true;
+            _hatchContentUnits_set = true;
         } else {
-            this->_hatchContentUnits_set = false;
+            _hatchContentUnits_set = false;
         }
 
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_HATCHTRANSFORM: {
         Geom::Affine t;
 
         if (value && sp_svg_transform_read(value, &t)) {
-            this->_hatchTransform = t;
-            this->_hatchTransform_set = true;
+            _hatchTransform = t;
+            _hatchTransform_set = true;
         } else {
-            this->_hatchTransform = Geom::identity();
-            this->_hatchTransform_set = false;
+            _hatchTransform = Geom::identity();
+            _hatchTransform_set = false;
         }
 
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
     }
     case SP_ATTR_X:
-        this->_x.readOrUnset(value);
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        _x.readOrUnset(value);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_Y:
-        this->_y.readOrUnset(value);
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        _y.readOrUnset(value);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_PITCH:
-        this->_pitch.readOrUnset(value);
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        _pitch.readOrUnset(value);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_ROTATE:
-        this->_rotate.readOrUnset(value);
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        _rotate.readOrUnset(value);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     case SP_ATTR_XLINK_HREF:
-        if (value && this->href == value) {
-            /* Href unchanged, do nothing. */
+        if (value && href == value) {
+            // Href unchanged, do nothing.
         } else {
-            this->href.clear();
+            href.clear();
 
             if (value) {
                 // First, set the href field; it's only used in the "unchanged" check above.
-                this->href = value;
+                href = value;
                 // Now do the attaching, which emits the changed signal.
                 if (value) {
                     try {
-                        this->ref->attach(Inkscape::URI(value));
+                        ref->attach(Inkscape::URI(value));
                     } catch (Inkscape::BadURIException &e) {
                         g_warning("%s", e.what());
-                        this->ref->detach();
+                        ref->detach();
                     }
                 } else {
-                    this->ref->detach();
+                    ref->detach();
                 }
             }
         }
-        this->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
+        requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
         break;
 
     default:
         if (SP_ATTRIBUTE_IS_CSS(key)) {
-            sp_style_read_from_object(this->style, this);
+            sp_style_read_from_object(style, this);
             requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG | SP_OBJECT_STYLE_MODIFIED_FLAG);
         } else {
             SPPaintServer::set(key, value);
@@ -234,43 +247,55 @@ void SPHatch::set(unsigned int key, const gchar* value) {
     }
 }
 
-bool SPHatch::_hasHatchPatchChildren(SPHatch const *hatch) {
-    for (SPObject const *child = hatch->firstChild(); child; child = child->getNext() ) {
-        if (SP_IS_HATCH_PATH(child)) {
-            return true;
+bool SPHatch::_hasHatchPatchChildren(SPHatch const *hatch)
+{
+    bool matched = false;
+    for (SPObject const *child = hatch->firstChild(); child && !matched; child = child->getNext() ) {
+        SPHatchPath const *hatchPath = dynamic_cast<SPHatchPath const *>(child);
+        if (hatchPath) {
+            matched = true;
         }
     }
-    return false;
+    return matched;
 }
 
-void SPHatch::hatchPaths(std::vector<SPHatchPath*>& l) {
+std::vector<SPHatchPath*> SPHatch::hatchPaths()
+{
+    std::vector<SPHatchPath*> list;
     SPHatch *src = chase_hrefs<SPHatch>(this, sigc::ptr_fun(&_hasHatchPatchChildren));
 
     if (src) {
         for (SPObject *child = src->firstChild(); child; child = child->getNext()) {
-            if (SP_IS_HATCH_PATH(child)) {
-                l.push_back(SP_HATCH_PATH(child));
+            SPHatchPath *hatchPath = dynamic_cast<SPHatchPath *>(child);
+            if (hatchPath) {
+                list.push_back(hatchPath);
             }
         }
     }
+    return list;
 }
 
-void SPHatch::hatchPaths(std::vector<SPHatchPath const*>& l) const {
+std::vector<SPHatchPath const*> SPHatch::hatchPaths() const
+{
+    std::vector<SPHatchPath const*> list;
     SPHatch const *src = chase_hrefs<SPHatch const>(this, sigc::ptr_fun(&_hasHatchPatchChildren));
 
     if (src) {
         for (SPObject const *child = src->firstChild(); child; child = child->getNext()) {
-            if (SP_IS_HATCH_PATH(child)) {
-                l.push_back(SP_HATCH_PATH(child));
+            SPHatchPath const *hatchPath = dynamic_cast<SPHatchPath const*>(child);
+            if (hatchPath) {
+                list.push_back(hatchPath);
             }
         }
     }
+    return list;
 }
 
-/* TODO: ::remove_child and ::order_changed handles - see SPPattern */
+// TODO: ::remove_child and ::order_changed handles - see SPPattern
 
 
-void SPHatch::update(SPCtx* ctx, unsigned int flags) {
+void SPHatch::update(SPCtx* ctx, unsigned int flags)
+{
     typedef std::list<SPHatch::View>::iterator ViewIterator;
 
     if (flags & SP_OBJECT_MODIFIED_FLAG) {
@@ -279,15 +304,14 @@ void SPHatch::update(SPCtx* ctx, unsigned int flags) {
 
     flags &= SP_OBJECT_MODIFIED_CASCADE;
 
-    std::vector<SPHatchPath *> children;
-    hatchPaths(children);
+    std::vector<SPHatchPath *> children(hatchPaths());
 
-    for (ChildIterator iter = children.begin(); iter != children.end(); iter++) {
+    for (ChildIterator iter = children.begin(); iter != children.end(); ++iter) {
         SPHatchPath* child = *iter;
 
         sp_object_ref(child, NULL);
 
-        for (ViewIterator view_iter = _display.begin(); view_iter != _display.end(); view_iter++) {
+        for (ViewIterator view_iter = _display.begin(); view_iter != _display.end(); ++view_iter) {
             Geom::OptInterval strip_extents = _calculateStripExtents(view_iter->bbox);
             child->setStripExtents(view_iter->key, strip_extents);
         }
@@ -300,22 +324,22 @@ void SPHatch::update(SPCtx* ctx, unsigned int flags) {
         sp_object_unref(child, NULL);
     }
 
-    for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+    for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
         _updateView(*iter);
     }
 }
 
-void SPHatch::modified(unsigned int flags) {
+void SPHatch::modified(unsigned int flags)
+{
     if (flags & SP_OBJECT_MODIFIED_FLAG) {
         flags |= SP_OBJECT_PARENT_MODIFIED_FLAG;
     }
 
     flags &= SP_OBJECT_MODIFIED_CASCADE;
 
-    std::vector<SPHatchPath *> children;
-    hatchPaths(children);
+    std::vector<SPHatchPath *> children(hatchPaths());
 
-    for (ChildIterator iter = children.begin(); iter != children.end(); iter++) {
+    for (ChildIterator iter = children.begin(); iter != children.end(); ++iter) {
         SPObject *child = *iter;
 
         sp_object_ref(child, NULL);
@@ -328,14 +352,16 @@ void SPHatch::modified(unsigned int flags) {
     }
 }
 
-void SPHatch::_onRefChanged(SPObject *old_ref, SPObject *ref) {
+void SPHatch::_onRefChanged(SPObject *old_ref, SPObject *ref)
+{
     typedef std::list<SPHatch::View>::iterator ViewIterator;
 
     if (old_ref) {
         _modified_connection.disconnect();
     }
 
-    if (SP_IS_HATCH(ref)) {
+    SPHatch *hatch = dynamic_cast<SPHatch *>(ref);
+    if (hatch) {
         _modified_connection = ref->connectModified(sigc::mem_fun(this, &SPHatch::_onRefModified));
     }
 
@@ -344,24 +370,26 @@ void SPHatch::_onRefChanged(SPObject *old_ref, SPObject *ref) {
         SPHatch *new_shown = NULL;
         std::vector<SPHatchPath *> oldhatchPaths;
         std::vector<SPHatchPath *> newhatchPaths;
-        if (SP_IS_HATCH(old_ref)) {
-            old_shown = SP_HATCH(old_ref)->rootHatch();
-            old_shown->hatchPaths(oldhatchPaths);
+
+        SPHatch *old_hatch = dynamic_cast<SPHatch *>(old_ref);
+        if (old_hatch) {
+            old_shown = old_hatch->rootHatch();
+            oldhatchPaths = old_shown->hatchPaths();
         }
-        if (SP_IS_HATCH(ref)) {
-            new_shown = SP_HATCH(ref)->rootHatch();
-            new_shown->hatchPaths(newhatchPaths);
+        if (hatch) {
+            new_shown = hatch->rootHatch();
+            newhatchPaths = new_shown->hatchPaths();
         }
         if (old_shown != new_shown) {
 
-            for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+            for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
                 Geom::OptInterval extents = _calculateStripExtents(iter->bbox);
 
-                for (ChildIterator child_iter = oldhatchPaths.begin(); child_iter != oldhatchPaths.end(); child_iter++) {
+                for (ChildIterator child_iter = oldhatchPaths.begin(); child_iter != oldhatchPaths.end(); ++child_iter) {
                     SPHatchPath *child = *child_iter;
                     child->hide(iter->key);
                 }
-                for (ChildIterator child_iter = newhatchPaths.begin(); child_iter != newhatchPaths.end(); child_iter++) {
+                for (ChildIterator child_iter = newhatchPaths.begin(); child_iter != newhatchPaths.end(); ++child_iter) {
                     SPHatchPath *child = *child_iter;
                     Inkscape::DrawingItem *cai = child->show(iter->arenaitem->drawing(), iter->key, extents);
                     child->requestDisplayUpdate(SP_OBJECT_MODIFIED_FLAG);
@@ -377,13 +405,15 @@ void SPHatch::_onRefChanged(SPObject *old_ref, SPObject *ref) {
     _onRefModified(ref, 0);
 }
 
-void SPHatch::_onRefModified(SPObject */*ref*/, guint /*flags*/) {
+void SPHatch::_onRefModified(SPObject */*ref*/, guint /*flags*/)
+{
     requestModified(SP_OBJECT_MODIFIED_FLAG);
     // Conditional to avoid causing infinite loop if there's a cycle in the href chain.
 }
 
 
-SPHatch *SPHatch::rootHatch() {
+SPHatch *SPHatch::rootHatch()
+{
     SPHatch *src = chase_hrefs<SPHatch>(this, sigc::ptr_fun(&_hasHatchPatchChildren));
     return src ? src : this; // document is broken, we can't get to root; but at least we can return pat which is supposedly a valid hatch
 }
@@ -391,101 +421,117 @@ SPHatch *SPHatch::rootHatch() {
 // Access functions that look up fields up the chain of referenced hatchs and return the first one which is set
 // FIXME: all of them must use chase_hrefs as children() and rootHatch()
 
-SPHatch::HatchUnits SPHatch::hatchUnits() const {
-    for (SPHatch const *pat_i = this; pat_i != NULL;
-            pat_i = pat_i->ref ? pat_i->ref->getObject() : NULL) {
-        if (pat_i->_hatchUnits_set)
-            return pat_i->_hatchUnits;
+SPHatch::HatchUnits SPHatch::hatchUnits() const
+{
+    HatchUnits units = _hatchUnits;
+    for (SPHatch const *pat_i = this; pat_i; pat_i = (pat_i->ref) ? pat_i->ref->getObject() : NULL) {
+        if (pat_i->_hatchUnits_set) {
+            units = pat_i->_hatchUnits;
+            break;
+        }
     }
-    return _hatchUnits;
+    return units;
 }
 
-SPHatch::HatchUnits SPHatch::hatchContentUnits() const {
-    for (SPHatch const *pat_i = this; pat_i != NULL;
-            pat_i = pat_i->ref ? pat_i->ref->getObject() : NULL) {
-        if (pat_i->_hatchContentUnits_set)
-            return pat_i->_hatchContentUnits;
+SPHatch::HatchUnits SPHatch::hatchContentUnits() const
+{
+    HatchUnits units = _hatchContentUnits;
+    for (SPHatch const *pat_i = this; pat_i; pat_i = (pat_i->ref) ? pat_i->ref->getObject() : NULL) {
+        if (pat_i->_hatchContentUnits_set) {
+            units = pat_i->_hatchContentUnits;
+            break;
+        }
     }
-    return _hatchContentUnits;
+    return units;
 }
 
-Geom::Affine const &SPHatch::hatchTransform() const {
-    for (SPHatch const *pat_i = this; pat_i != NULL;
-            pat_i = pat_i->ref ? pat_i->ref->getObject() : NULL) {
-        if (pat_i->_hatchTransform_set)
+Geom::Affine const &SPHatch::hatchTransform() const
+{
+    for (SPHatch const *pat_i = this; pat_i; pat_i = (pat_i->ref) ? pat_i->ref->getObject() : NULL) {
+        if (pat_i->_hatchTransform_set) {
             return pat_i->_hatchTransform;
+        }
     }
     return _hatchTransform;
 }
 
-gdouble SPHatch::x() const {
-    for (SPHatch const *pat_i = this; pat_i != NULL;
-            pat_i = pat_i->ref ? pat_i->ref->getObject() : NULL) {
-        if (pat_i->_x._set)
-            return pat_i->_x.computed;
+gdouble SPHatch::x() const
+{
+    gdouble val = 0;
+    for (SPHatch const *pat_i = this; pat_i; pat_i = (pat_i->ref) ? pat_i->ref->getObject() : NULL) {
+        if (pat_i->_x._set) {
+            val = pat_i->_x.computed;
+            break;
+        }
     }
-    return 0;
+    return val;
 }
 
-gdouble SPHatch::y() const {
-    for (SPHatch const *pat_i = this; pat_i != NULL;
-            pat_i = pat_i->ref ? pat_i->ref->getObject() : NULL) {
-        if (pat_i->_y._set)
-            return pat_i->_y.computed;
+gdouble SPHatch::y() const
+{
+    gdouble val = 0;
+    for (SPHatch const *pat_i = this; pat_i; pat_i = (pat_i->ref) ? pat_i->ref->getObject() : NULL) {
+        if (pat_i->_y._set) {
+            val = pat_i->_y.computed;
+            break;
+        }
     }
-    return 0;
+    return val;
 }
 
-gdouble SPHatch::pitch() const {
-    for (SPHatch const *pat_i = this; pat_i != NULL;
-            pat_i = pat_i->ref ? pat_i->ref->getObject() : NULL) {
-        if (pat_i->_pitch._set)
-            return pat_i->_pitch.computed;
+gdouble SPHatch::pitch() const
+{
+    gdouble val = 0;
+    for (SPHatch const *pat_i = this; pat_i; pat_i = (pat_i->ref) ? pat_i->ref->getObject() : NULL) {
+        if (pat_i->_pitch._set) {
+            val = pat_i->_pitch.computed;
+            break;
+        }
     }
-    return 0;
+    return val;
 }
 
-gdouble SPHatch::rotate() const {
-    for (SPHatch const *pat_i = this; pat_i != NULL;
-            pat_i = pat_i->ref ? pat_i->ref->getObject() : NULL) {
-        if (pat_i->_rotate._set)
-            return pat_i->_rotate.computed;
+gdouble SPHatch::rotate() const
+{
+    gdouble val = 0;
+    for (SPHatch const *pat_i = this; pat_i; pat_i = (pat_i->ref) ? pat_i->ref->getObject() : NULL) {
+        if (pat_i->_rotate._set) {
+            val = pat_i->_rotate.computed;
+            break;
+        }
     }
-    return 0;
+    return val;
 }
 
-bool SPHatch::isValid() const {
-    double strip_pitch = pitch();
-    if (strip_pitch <= 0) {
-        return false;
-    }
-
-    std::vector<SPHatchPath const *> children;
-    hatchPaths(children);
-    if (children.empty()) {
-        return false;
-    }
-    for (ConstChildIterator iter = children.begin(); iter != children.end(); iter++) {
-        SPHatchPath const *child = *iter;
-        if (!child->isValid()) {
-            return false;
+bool SPHatch::isValid() const
+{
+    bool valid = false;
+
+    if (pitch() > 0) {
+        std::vector<SPHatchPath const *> children(hatchPaths());
+        if (!children.empty()) {
+            valid = true;
+            for (ConstChildIterator iter = children.begin(); (iter != children.end()) && valid; ++iter) {
+                SPHatchPath const *child = *iter;
+                valid = child->isValid();
+            }
         }
     }
 
-    return true;
+    return valid;
 }
 
-Inkscape::DrawingPattern *SPHatch::show(Inkscape::Drawing &drawing, unsigned int key, Geom::OptRect bbox) {
+Inkscape::DrawingPattern *SPHatch::show(Inkscape::Drawing &drawing, unsigned int key, Geom::OptRect bbox)
+{
     Inkscape::DrawingPattern *ai = new Inkscape::DrawingPattern(drawing);
     //TODO: set some debug flag to see DrawingPattern
     _display.push_front(View(ai, key));
     _display.front().bbox = bbox;
 
-    std::vector<SPHatchPath *> children;
-    hatchPaths(children);
+    std::vector<SPHatchPath *> children(hatchPaths());
 
     Geom::OptInterval extents = _calculateStripExtents(bbox);
-    for (ChildIterator iter = children.begin(); iter != children.end(); iter++) {
+    for (ChildIterator iter = children.begin(); iter != children.end(); ++iter) {
         SPHatchPath *child = *iter;
         Inkscape::DrawingItem *cai = child->show(drawing, key, extents);
         if (cai) {
@@ -499,16 +545,16 @@ Inkscape::DrawingPattern *SPHatch::show(Inkscape::Drawing &drawing, unsigned int
     return ai;
 }
 
-void SPHatch::hide(unsigned int key) {
-    std::vector<SPHatchPath *> children;
-    hatchPaths(children);
+void SPHatch::hide(unsigned int key)
+{
+    std::vector<SPHatchPath *> children(hatchPaths());
 
-    for (ChildIterator iter = children.begin(); iter != children.end(); iter++) {
+    for (ChildIterator iter = children.begin(); iter != children.end(); ++iter) {
         SPHatchPath *child = *iter;
         child->hide(key);
     }
 
-    for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+    for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
         if (iter->key == key) {
             delete iter->arenaitem;
             _display.erase(iter);
@@ -520,12 +566,12 @@ void SPHatch::hide(unsigned int key) {
 }
 
 
-Geom::Interval SPHatch::bounds() const {
+Geom::Interval SPHatch::bounds() const
+{
     Geom::Interval result;
-    std::vector<SPHatchPath const *> children;
-    hatchPaths(children);
+    std::vector<SPHatchPath const *> children(hatchPaths());
 
-    for (ConstChildIterator iter = children.begin(); iter != children.end(); iter++) {
+    for (ConstChildIterator iter = children.begin(); iter != children.end(); ++iter) {
         SPHatchPath const *child = *iter;
         if (result.extent() == 0) {
             result = child->bounds();
@@ -536,9 +582,10 @@ Geom::Interval SPHatch::bounds() const {
     return result;
 }
 
-SPHatch::RenderInfo SPHatch::calculateRenderInfo(unsigned key) const {
+SPHatch::RenderInfo SPHatch::calculateRenderInfo(unsigned key) const
+{
     RenderInfo info;
-    for (ConstViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+    for (ConstViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
         if (iter->key == key) {
             return _calculateRenderInfo(*iter);
         }
@@ -547,7 +594,8 @@ SPHatch::RenderInfo SPHatch::calculateRenderInfo(unsigned key) const {
     return info;
 }
 
-void SPHatch::_updateView(View &view) {
+void SPHatch::_updateView(View &view)
+{
     RenderInfo info = _calculateRenderInfo(view);
     //The rendering of hatch overflow is implemented by repeated drawing
     //of hatch paths over one strip. Within each iteration paths are moved by pitch value.
@@ -558,93 +606,94 @@ void SPHatch::_updateView(View &view) {
     view.arenaitem->setChildTransform(info.child_transform);
     view.arenaitem->setPatternToUserTransform(info.pattern_to_user_transform);
     view.arenaitem->setTileRect(info.tile_rect);
-    view.arenaitem->setStyle(this->style);
+    view.arenaitem->setStyle(style);
     view.arenaitem->setOverflow(info.overflow_initial_transform, info.overflow_steps,
-            info.overflow_step_transform);
+                                info.overflow_step_transform);
 }
 
-SPHatch::RenderInfo SPHatch::_calculateRenderInfo(View const &view) const {
+SPHatch::RenderInfo SPHatch::_calculateRenderInfo(View const &view) const
+{
     RenderInfo info;
 
     Geom::OptInterval extents = _calculateStripExtents(view.bbox);
-    if (!extents) {
-        return info;
-    }
-
-    double tile_x = x();
-    double tile_y = y();
-    double tile_width = pitch();
-    double tile_height = extents->max() - extents->min();
-    double tile_rotate = rotate();
-    double tile_render_y = extents->min();
-
-    if (view.bbox && (hatchUnits() == UNITS_OBJECTBOUNDINGBOX)) {
-        tile_x *= view.bbox->width();
-        tile_y *= view.bbox->height();
-        tile_width *= view.bbox->width();
-        tile_height *= view.bbox->height();
-        tile_render_y *= view.bbox->height();
-    }
-
-    // Pattern size in hatch space
-    Geom::Rect hatch_tile = Geom::Rect::from_xywh(0, tile_render_y, tile_width, tile_height);
-    // Content to bbox
-    Geom::Affine content2ps;
-    if (view.bbox && (hatchContentUnits() == UNITS_OBJECTBOUNDINGBOX)) {
-        content2ps = Geom::Affine(view.bbox->width(), 0.0, 0.0, view.bbox->height(), 0, 0);
-    }
-
-    // Tile (hatch space) to user.
-    Geom::Affine ps2user = Geom::Translate(tile_x, tile_y) * Geom::Rotate::from_degrees(tile_rotate) * hatchTransform();
-
-    info.child_transform = content2ps;
-    info.pattern_to_user_transform = ps2user;
-    info.tile_rect = hatch_tile;
-
-    if (style->overflow.computed == SP_CSS_OVERFLOW_VISIBLE) {
-        Geom::Interval bounds = this->bounds();
-        gdouble pitch = this->pitch();
-        gdouble overflow_right_strip = floor(bounds.max() / pitch) * pitch;
-        info.overflow_steps = ceil((overflow_right_strip - bounds.min()) / pitch) + 1;
-        info.overflow_step_transform = Geom::Translate(pitch, 0.0);
-        info.overflow_initial_transform = Geom::Translate(-overflow_right_strip, 0.0);
-    } else {
-        info.overflow_steps = 1;
+    if (extents) {
+        double tile_x = x();
+        double tile_y = y();
+        double tile_width = pitch();
+        double tile_height = extents->max() - extents->min();
+        double tile_rotate = rotate();
+        double tile_render_y = extents->min();
+
+        if (view.bbox && (hatchUnits() == UNITS_OBJECTBOUNDINGBOX)) {
+            tile_x *= view.bbox->width();
+            tile_y *= view.bbox->height();
+            tile_width *= view.bbox->width();
+            tile_height *= view.bbox->height();
+            tile_render_y *= view.bbox->height();
+        }
+
+        // Pattern size in hatch space
+        Geom::Rect hatch_tile = Geom::Rect::from_xywh(0, tile_render_y, tile_width, tile_height);
+
+        // Content to bbox
+        Geom::Affine content2ps;
+        if (view.bbox && (hatchContentUnits() == UNITS_OBJECTBOUNDINGBOX)) {
+            content2ps = Geom::Affine(view.bbox->width(), 0.0, 0.0, view.bbox->height(), 0, 0);
+        }
+
+        // Tile (hatch space) to user.
+        Geom::Affine ps2user = Geom::Translate(tile_x, tile_y) * Geom::Rotate::from_degrees(tile_rotate) * hatchTransform();
+
+        info.child_transform = content2ps;
+        info.pattern_to_user_transform = ps2user;
+        info.tile_rect = hatch_tile;
+
+        if (style->overflow.computed == SP_CSS_OVERFLOW_VISIBLE) {
+            Geom::Interval bounds = this->bounds();
+            gdouble pitch = this->pitch();
+            gdouble overflow_right_strip = floor(bounds.max() / pitch) * pitch;
+            info.overflow_steps = ceil((overflow_right_strip - bounds.min()) / pitch) + 1;
+            info.overflow_step_transform = Geom::Translate(pitch, 0.0);
+            info.overflow_initial_transform = Geom::Translate(-overflow_right_strip, 0.0);
+        } else {
+            info.overflow_steps = 1;
+        }
     }
 
     return info;
 }
 
 //calculates strip extents in content space
-Geom::OptInterval SPHatch::_calculateStripExtents(Geom::OptRect bbox) const {
+Geom::OptInterval SPHatch::_calculateStripExtents(Geom::OptRect const &bbox) const
+{
     if (!bbox || (bbox->area() == 0)) {
         return Geom::OptInterval();
-    }
-
-    double tile_x = x();
-    double tile_y = y();
-    double tile_rotate = rotate();
-
-    Geom::Affine ps2user = Geom::Translate(tile_x, tile_y) * Geom::Rotate::from_degrees(tile_rotate) * hatchTransform();
-    Geom::Affine user2ps = ps2user.inverse();
-
-    Geom::Interval extents;
-    for (int i = 0; i < 4; i++) {
-        Geom::Point corner = bbox->corner(i);
-        Geom::Point corner_ps  =  corner * user2ps;
-        if (i == 0 || corner_ps.y() < extents.min()) {
-            extents.setMin(corner_ps.y());
+    } else {
+        double tile_x = x();
+        double tile_y = y();
+        double tile_rotate = rotate();
+
+        Geom::Affine ps2user = Geom::Translate(tile_x, tile_y) * Geom::Rotate::from_degrees(tile_rotate) * hatchTransform();
+        Geom::Affine user2ps = ps2user.inverse();
+
+        Geom::Interval extents;
+        for (int i = 0; i < 4; ++i) {
+            Geom::Point corner = bbox->corner(i);
+            Geom::Point corner_ps  =  corner * user2ps;
+            if (i == 0 || corner_ps.y() < extents.min()) {
+                extents.setMin(corner_ps.y());
+            }
+            if (i == 0 || corner_ps.y() > extents.max()) {
+                extents.setMax(corner_ps.y());
+            }
         }
-        if (i == 0 || corner_ps.y() > extents.max()) {
-            extents.setMax(corner_ps.y());
+
+        if (hatchUnits() == UNITS_OBJECTBOUNDINGBOX) {
+            extents /= bbox->height();
         }
-    }
 
-    if (hatchUnits() == UNITS_OBJECTBOUNDINGBOX) {
-        extents /= bbox->height();
+        return extents;
     }
-
-    return extents;
 }
 
 cairo_pattern_t* SPHatch::pattern_new(cairo_t * /*base_ct*/, Geom::OptRect const &/*bbox*/, double /*opacity*/)
@@ -654,8 +703,9 @@ cairo_pattern_t* SPHatch::pattern_new(cairo_t * /*base_ct*/, Geom::OptRect const
     return cairo_pattern_create_rgb(0.5, 0.5, 1.0);
 }
 
-void SPHatch::setBBox(unsigned int key, Geom::OptRect const &bbox) {
-    for (ViewIterator iter = _display.begin(); iter != _display.end(); iter++) {
+void SPHatch::setBBox(unsigned int key, Geom::OptRect const &bbox)
+{
+    for (ViewIterator iter = _display.begin(); iter != _display.end(); ++iter) {
         if (iter->key == key) {
             iter->bbox = bbox;
             break;
@@ -663,10 +713,37 @@ void SPHatch::setBBox(unsigned int key, Geom::OptRect const &bbox) {
     }
 }
 
+//
+
+SPHatch::RenderInfo::RenderInfo()
+    : child_transform(),
+      pattern_to_user_transform(),
+      tile_rect(),
+      overflow_steps(0),
+      overflow_step_transform(),
+      overflow_initial_transform()
+{
+}
+
+SPHatch::RenderInfo::~RenderInfo()
+{
+}
+
+//
+
 SPHatch::View::View(Inkscape::DrawingPattern *arenaitem, int key)
-        : arenaitem(arenaitem), key(key)
+    : arenaitem(arenaitem),
+      bbox(),
+      key(key)
 {
 }
+
+SPHatch::View::~View()
+{
+    // remember, do not delete arenaitem here
+    arenaitem = NULL;
+}
+
 /*
  Local Variables:
  mode:c++
diff --git a/src/sp-hatch.h b/src/sp-hatch.h
index dc6ee0add..5004a611f 100644
--- a/src/sp-hatch.h
+++ b/src/sp-hatch.h
@@ -1,8 +1,11 @@
-/** @file
+/**
+ * @file
  * SVG <hatch> implementation
- *//*
- * Author:
+ */
+/*
+ * Authors:
  *   Tomasz Boczkowski <penginsbacon@gmail.com>
+ *   Jon A. Cruz <jon@joncruz.org>
  *
  * Copyright (C) 2014 Tomasz Boczkowski
  *
@@ -38,9 +41,6 @@ class Node;
 }
 }
 
-#define SP_HATCH(obj) (dynamic_cast<SPHatch*>((SPObject*)obj))
-#define SP_IS_HATCH(obj) (dynamic_cast<const SPHatch*>((SPObject*)obj) != NULL)
-
 class SPHatch : public SPPaintServer {
 public:
     enum HatchUnits {
@@ -48,7 +48,11 @@ public:
         UNITS_OBJECTBOUNDINGBOX
     };
 
-    struct RenderInfo {
+    class RenderInfo {
+    public:
+        RenderInfo();
+        ~RenderInfo();
+
         Geom::Affine child_transform;
         Geom::Affine pattern_to_user_transform;
         Geom::Rect tile_rect;
@@ -59,10 +63,10 @@ public:
     };
 
     SPHatch();
-	virtual ~SPHatch();
+    virtual ~SPHatch();
 
-    /* Reference (href) */
-	Glib::ustring href;
+    // Reference (href)
+    Glib::ustring href;
     SPHatchReference *ref;
 
     gdouble x() const;
@@ -74,32 +78,35 @@ public:
     Geom::Affine const &hatchTransform() const;
     SPHatch *rootHatch(); //TODO: const
 
-    void hatchPaths(std::vector<SPHatchPath*>& l);
-    void hatchPaths(std::vector<SPHatchPath const *>& l) const;
+    std::vector<SPHatchPath *> hatchPaths();
+    std::vector<SPHatchPath const *> hatchPaths() const;
 
     bool isValid() const;
 
     Inkscape::DrawingPattern *show(Inkscape::Drawing &drawing, unsigned int key, Geom::OptRect bbox);
     void hide(unsigned int key);
-	virtual cairo_pattern_t* pattern_new(cairo_t *ct, Geom::OptRect const &bbox, double opacity);
+    virtual cairo_pattern_t* pattern_new(cairo_t *ct, Geom::OptRect const &bbox, double opacity);
 
-	RenderInfo calculateRenderInfo(unsigned key) const;
-	Geom::Interval bounds() const;
-	void setBBox(unsigned int key, Geom::OptRect const &bbox);
+    RenderInfo calculateRenderInfo(unsigned key) const;
+    Geom::Interval bounds() const;
+    void setBBox(unsigned int key, Geom::OptRect const &bbox);
 
 protected:
-	virtual void build(SPDocument* doc, Inkscape::XML::Node* repr);
-	virtual void release();
-	virtual void child_added(Inkscape::XML::Node* child, Inkscape::XML::Node* ref);
-	virtual void set(unsigned int key, const gchar* value);
-	virtual void update(SPCtx* ctx, unsigned int flags);
-	virtual void modified(unsigned int flags);
+    virtual void build(SPDocument* doc, Inkscape::XML::Node* repr);
+    virtual void release();
+    virtual void child_added(Inkscape::XML::Node* child, Inkscape::XML::Node* ref);
+    virtual void set(unsigned int key, const gchar* value);
+    virtual void update(SPCtx* ctx, unsigned int flags);
+    virtual void modified(unsigned int flags);
 
 private:
-    struct View {
+    class View {
+    public:
         View(Inkscape::DrawingPattern *arenaitem, int key);
         //Do not delete arenaitem in destructor.
 
+        ~View();
+
         Inkscape::DrawingPattern *arenaitem;
         Geom::OptRect bbox;
         unsigned int key;
@@ -114,28 +121,30 @@ private:
 
     void _updateView(View &view);
     RenderInfo _calculateRenderInfo(View const &view) const;
-	Geom::OptInterval _calculateStripExtents(Geom::OptRect bbox) const;
+    Geom::OptInterval _calculateStripExtents(Geom::OptRect const &bbox) const;
 
 
-	/**
-	Gets called when the hatch is reattached to another <hatch>
-	*/
-	void _onRefChanged(SPObject *old_ref, SPObject *ref);
+    /**
+     * Gets called when the hatch is reattached to another <hatch>
+     */
+    void _onRefChanged(SPObject *old_ref, SPObject *ref);
 
-	/**
-	Gets called when the referenced <hatch> is changed
-	*/
-	void _onRefModified(SPObject *ref, guint flags);
+    /**
+     * Gets called when the referenced <hatch> is changed
+     */
+    void _onRefModified(SPObject *ref, guint flags);
 
-    /* patternUnits and patternContentUnits attribute */
+    // patternUnits and patternContentUnits attribute
     HatchUnits _hatchUnits : 1;
     bool _hatchUnits_set : 1;
     HatchUnits _hatchContentUnits : 1;
     bool _hatchContentUnits_set : 1;
-    /* hatchTransform attribute */
+
+    // hatchTransform attribute
     Geom::Affine _hatchTransform;
     bool _hatchTransform_set : 1;
-    /* Strip */
+
+    // Strip
     SVGLength _x;
     SVGLength _y;
     SVGLength _pitch;
@@ -149,14 +158,17 @@ private:
 
 class SPHatchReference : public Inkscape::URIReference {
 public:
-    SPHatchReference (SPObject *obj) : URIReference(obj) {}
+    SPHatchReference (SPObject *obj)
+        : URIReference(obj)
+    {}
+
     SPHatch *getObject() const {
         return reinterpret_cast<SPHatch *>(URIReference::getObject());
     }
 
 protected:
     virtual bool _acceptObject(SPObject *obj) const {
-        return SP_IS_HATCH (obj);
+        return dynamic_cast<SPHatch *>(obj) != NULL;
     }
 };