[Bf-blender-cvs] [f6b4b12] master: Better fix for T41464: Material Boundary bug in Freestyle.

Tamito Kajiyama noreply at git.blender.org
Tue Sep 2 14:59:29 CEST 2014


Commit: f6b4b1296125d43e94968709bc52d5f7a062734e
Author: Tamito Kajiyama
Date:   Tue Sep 2 21:24:41 2014 +0900
Branches: master
https://developer.blender.org/rBf6b4b1296125d43e94968709bc52d5f7a062734e

Better fix for T41464: Material Boundary bug in Freestyle.

The problem addressed here is that there was no mean to check if an iterator
points the last of the elements being iterated over.  Such checking is necessary
to reliably dereference the iterator (i.e., calling the operator*() method of the
underlying C++ iterator object).

Now Interface0DIterator and StrokeVertexIterator have an .at_last property
to check if an iterator points the last element.  Using this new API feature,
the present commit partly reverts the previous commit rBeb8964fb7f19 to
better address T41464.

Differential revision: https://developer.blender.org/D752

Author: flokkievids (Folkert de Vries)

Reviewed by: kjym3 (Tamito Kajiyama)

===================================================================

M	release/scripts/freestyle/modules/parameter_editor.py
M	source/blender/freestyle/intern/python/Iterator/BPy_Interface0DIterator.cpp
M	source/blender/freestyle/intern/python/Iterator/BPy_StrokeVertexIterator.cpp
M	source/blender/freestyle/intern/stroke/StrokeIterators.h
M	source/blender/freestyle/intern/view_map/Interface0D.h

===================================================================

diff --git a/release/scripts/freestyle/modules/parameter_editor.py b/release/scripts/freestyle/modules/parameter_editor.py
index 6ab4184..ebd09bd 100644
--- a/release/scripts/freestyle/modules/parameter_editor.py
+++ b/release/scripts/freestyle/modules/parameter_editor.py
@@ -814,17 +814,14 @@ class FaceMarkOneUP1D(UnaryPredicate1D):
 
 class MaterialBoundaryUP0D(UnaryPredicate0D):
     def __call__(self, it):
-        if it.is_begin:
+        # can't use only it.is_end here, see commit rBeb8964fb7f19
+        if it.is_begin or it.at_last or it.is_end:
             return False
-        it_prev = Interface0DIterator(it)
-        it_prev.decrement()
-        v = it.object
-        it.increment()
-        if it.is_end:
-            return False
-        fe = v.get_fedge(it_prev.object)
+        it.decrement()
+        prev, v, succ = next(it), next(it), next(it)
+        fe = v.get_fedge(prev)
         idx1 = fe.material_index if fe.is_smooth else fe.material_index_left
-        fe = v.get_fedge(it.object)
+        fe = v.get_fedge(succ)
         idx2 = fe.material_index if fe.is_smooth else fe.material_index_left
         return idx1 != idx2
 
diff --git a/source/blender/freestyle/intern/python/Iterator/BPy_Interface0DIterator.cpp b/source/blender/freestyle/intern/python/Iterator/BPy_Interface0DIterator.cpp
index faf99e9..fca4c97 100644
--- a/source/blender/freestyle/intern/python/Iterator/BPy_Interface0DIterator.cpp
+++ b/source/blender/freestyle/intern/python/Iterator/BPy_Interface0DIterator.cpp
@@ -120,19 +120,14 @@ static PyObject *Interface0DIterator_iternext(BPy_Interface0DIterator *self)
 		self->if0D_it->decrement();
 	}
 	else {
-		if (self->if0D_it->isEnd()) {
+		if (self->if0D_it->atLast() || self->if0D_it->isEnd()) {
 			PyErr_SetNone(PyExc_StopIteration);
 			return NULL;
 		}
 		if (self->at_start)
 			self->at_start = false;
-		else {
+		else
 			self->if0D_it->increment();
-			if (self->if0D_it->isEnd()) {
-				PyErr_SetNone(PyExc_StopIteration);
-				return NULL;
-			}
-		}
 	}
 	Interface0D *if0D = self->if0D_it->operator->();
 	return Any_BPy_Interface0D_from_Interface0D(*if0D);
@@ -177,11 +172,24 @@ static PyObject *Interface0DIterator_u_get(BPy_Interface0DIterator *self, void *
 	return PyFloat_FromDouble(self->if0D_it->u());
 }
 
+PyDoc_STRVAR(Interface0DIterator_at_last_doc,
+"True if the interator points to the last valid element.\n"
+"For its counterpart (pointing to the first valid element), use it.is_begin.\n"
+"\n"
+":type: bool");
+
+static PyObject *Interface0DIterator_at_last_get(BPy_Interface0DIterator *self, void *UNUSED(closure))
+{
+	return PyBool_from_bool(self->if0D_it->atLast());
+}
+
 static PyGetSetDef BPy_Interface0DIterator_getseters[] = {
 	{(char *)"object", (getter)Interface0DIterator_object_get, (setter)NULL,
 	                   (char *)Interface0DIterator_object_doc, NULL},
 	{(char *)"t", (getter)Interface0DIterator_t_get, (setter)NULL, (char *)Interface0DIterator_t_doc, NULL},
 	{(char *)"u", (getter)Interface0DIterator_u_get, (setter)NULL, (char *)Interface0DIterator_u_doc, NULL},
+	{(char *)"at_last", (getter)Interface0DIterator_at_last_get, (setter)NULL,
+	                    (char *)Interface0DIterator_at_last_doc, NULL},
 	{NULL, NULL, NULL, NULL, NULL}  /* Sentinel */
 };
 
diff --git a/source/blender/freestyle/intern/python/Iterator/BPy_StrokeVertexIterator.cpp b/source/blender/freestyle/intern/python/Iterator/BPy_StrokeVertexIterator.cpp
index 2906f20..18d1b37 100644
--- a/source/blender/freestyle/intern/python/Iterator/BPy_StrokeVertexIterator.cpp
+++ b/source/blender/freestyle/intern/python/Iterator/BPy_StrokeVertexIterator.cpp
@@ -102,8 +102,8 @@ static PyObject *StrokeVertexIterator_iter(BPy_StrokeVertexIterator *self)
 static PyObject *StrokeVertexIterator_iternext(BPy_StrokeVertexIterator *self)
 {
 	/* Because Freestyle iterators for which it.isEnd() holds true have no valid object
-	 * (referencing it.object in this case leads to a crash), we must check if it.object
-	 * is valid after incrementing, to prevent crashes in Python.
+	 * (they point to the past-the-end element and can't be dereferenced), we have to check
+	 * iterators for validity.
 	 * Additionally, the at_start attribute is used to keep Freestyle iterator objects
 	 * and Python for loops in sync. */
 
@@ -115,7 +115,10 @@ static PyObject *StrokeVertexIterator_iternext(BPy_StrokeVertexIterator *self)
 		self->sv_it->decrement();
 	}
 	else {
-		if (self->sv_it->isEnd()) {
+		/* if sv_it.isEnd() is true, the iterator can't be incremented. if sv_it.isLast() is true,
+		 * the iterator is currently pointing to the final valid argument. Incrementing it further would
+		 * give a python object that can't be dereferenced. */
+		if (self->sv_it->atLast() || self->sv_it->isEnd()) {
 			PyErr_SetNone(PyExc_StopIteration);
 			return NULL;
 		}
@@ -123,15 +126,8 @@ static PyObject *StrokeVertexIterator_iternext(BPy_StrokeVertexIterator *self)
 		 * and don't increment, to keep for-loops in sync */
 		if (self->at_start)
 			self->at_start = false;
-		/* after incrementing, check if the iterator is at its end
-		 * exit the loop if it is. not doing so will result in a crash */
-		else {
+		else
 			self->sv_it->increment();
-			if (self->sv_it->isEnd()) {
-				PyErr_SetNone(PyExc_StopIteration);
-				return NULL;
-			}
-		}
 	}
 	StrokeVertex *sv = self->sv_it->operator->();
 	return BPy_StrokeVertex_from_StrokeVertex(*sv);
@@ -194,7 +190,7 @@ static PyObject *StrokeVertexIterator_reversed(BPy_StrokeVertexIterator *self)
 }
 
 static PyMethodDef BPy_StrokeVertexIterator_methods[] = {
-	{"incremented", (PyCFunction) StrokeVertexIterator_incremented, METH_NOARGS, StrokeVertexIterator_incremented_doc},
+	{"incremented", (PyCFunction)StrokeVertexIterator_incremented, METH_NOARGS, StrokeVertexIterator_incremented_doc},
 	{"decremented", (PyCFunction)StrokeVertexIterator_decremented, METH_NOARGS, StrokeVertexIterator_decremented_doc},
 	{"reversed", (PyCFunction)StrokeVertexIterator_reversed, METH_NOARGS, StrokeVertexIterator_reversed_doc},
 	{NULL, NULL, 0, NULL}
@@ -239,11 +235,25 @@ static PyObject *StrokeVertexIterator_u_get(BPy_StrokeVertexIterator *self, void
 	return PyFloat_FromDouble(self->sv_it->u());
 }
 
+PyDoc_STRVAR(StrokeVertexIterator_at_last_doc,
+"True if the interator points to the last valid element.\n"
+"For its counterpart (pointing to the first valid element), use it.is_begin.\n"
+"\n"
+":type: bool");
+
+static PyObject *StrokeVertexIterator_at_last_get(BPy_StrokeVertexIterator *self)
+{
+	return PyBool_from_bool(self->sv_it->atLast());
+
+}
+
 static PyGetSetDef BPy_StrokeVertexIterator_getseters[] = {
 	{(char *)"object", (getter)StrokeVertexIterator_object_get, (setter)NULL,
 	                   (char *)StrokeVertexIterator_object_doc, NULL},
 	{(char *)"t", (getter)StrokeVertexIterator_t_get, (setter)NULL, (char *)StrokeVertexIterator_t_doc, NULL},
 	{(char *)"u", (getter)StrokeVertexIterator_u_get, (setter)NULL, (char *)StrokeVertexIterator_u_doc, NULL},
+	{(char *)"at_last", (getter)StrokeVertexIterator_at_last_get, (setter)NULL,
+	                    (char *)StrokeVertexIterator_at_last_doc, NULL},
 	{NULL, NULL, NULL, NULL, NULL}  /* Sentinel */
 };
 
diff --git a/source/blender/freestyle/intern/stroke/StrokeIterators.h b/source/blender/freestyle/intern/stroke/StrokeIterators.h
index a8ec529..1df9aa2 100644
--- a/source/blender/freestyle/intern/stroke/StrokeIterators.h
+++ b/source/blender/freestyle/intern/stroke/StrokeIterators.h
@@ -171,6 +171,18 @@ public:
 		return _it == _begin;
 	}
 
+	/*! Returns true if the pointed StrokeVertex is the final valid StrokeVertex of the Stroke. */
+	bool atLast()
+	{
+		if (_it == _end)
+			return false;
+
+		++_it;
+		bool result = (_it == _end);
+		--_it;
+		return result;
+	}
+
 	/*! Returns true if the pointed StrokeVertex is after the last StrokeVertex of the Stroke. */
 	bool isEnd() const
 	{
diff --git a/source/blender/freestyle/intern/view_map/Interface0D.h b/source/blender/freestyle/intern/view_map/Interface0D.h
index 123253b..da71d7a 100644
--- a/source/blender/freestyle/intern/view_map/Interface0D.h
+++ b/source/blender/freestyle/intern/view_map/Interface0D.h
@@ -302,6 +302,18 @@ public:
 		return _iterator->isEnd();
 	}
 
+	/*! Returns true when the iterator is pointing to the final valid element. */
+	virtual bool atLast() const
+	{
+		if (_iterator->isEnd())
+			return false;
+
+		_iterator->increment();
+		bool result = _iterator->isEnd();
+		_iterator->decrement();
+		return result;
+	}
+
 	/*! operator == . */
 	bool operator==(const Interface0DIterator& it) const
 	{




More information about the Bf-blender-cvs mailing list