Fixed a bug in BRegion copy constructor. If one constructed a BRegion with

BRegion region(region), the region would have been put into an unitialized state. Took the chance to cleanup a bit the code, using some methods of clipping.h. Changed the way offset_rect works, and changed its name too. Added a note to Region.cpp


git-svn-id: file:///srv/svn/repos/haiku/trunk/current@4512 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/interface/clipping.h

@@ -42,6 +42,15 @@ sect_rect(clipping_rect r1, clipping_rect r2)
 	return rect;
 }
 
+static inline void
+offset_rect(clipping_rect &rect, int32 x, int32 y)
+{
+	rect.left += x;
+	rect.top += y;
+	rect.right += x;
+	rect.bottom += y;
+}
+
 
 static inline BRect
 to_BRect(clipping_rect rect)
@@ -113,18 +122,4 @@ rect_height(clipping_rect rect)
 	return rect.bottom - rect.top;
 }
 
-
-static inline clipping_rect
-rect_offset(clipping_rect rect, int32 x, int32 y)
-{
-	clipping_rect offRect;
-
-	offRect.left = rect.left + x;
-	offRect.top = rect.top + y;
-	offRect.right = rect.right + x;
-	offRect.bottom = rect.bottom + y;
-
-	return offRect;
-}
-
 #endif // __CLIPPING_H

src/kits/interface/Region.cpp

@@ -25,10 +25,22 @@
 //
 //------------------------------------------------------------------------------
 
+//	Notes: As now, memory is always allocated and never freed (except on destruction).
+//  This let us be a bit faster since we don't do many reallocations.
+//	But that means that even an empty region could "waste" much space, if it contained
+//	many rects before being emptied.
+//	I.E: a region which contains 24 rects allocates more than 24 * 4 * sizeof(int32)
+//  = 96 * sizeof(int32) bytes. If we call MakeEmpty(), that region will contain no rects,
+//  but it will still keep the allocated memory.
+//	This shouldnt' be an issue, since usually BRegions are just used for calculations, 
+//	and don't last so long.
+//	Anyway, we can change that behaviour if we want, but BeOS's BRegion seems to behave exactly
+//	like this.
+
 
 // Standard Includes -----------------------------------------------------------
 #include <cstdlib>
-#include <string.h>
+#include <cstring>
 
 // System Includes -------------------------------------------------------------
 #include <Debug.h>
@@ -38,11 +50,13 @@
 
 #include "region_helpers.h"
 
+
 /*! \brief Initializes a region. The region will have no rects,
 	and its bound will be invalid.
 */
 BRegion::BRegion()
-	:data_size(8),
+	:
+	data_size(8),
 	data(NULL)	
 {
 	data = (clipping_rect *)malloc(data_size * sizeof(clipping_rect));
@@ -55,7 +69,9 @@ BRegion::BRegion()
 	\param region The region to copy.
 */
 BRegion::BRegion(const BRegion &region)
-	:data(NULL)
+	:
+	data_size(8),
+	data(NULL)
 {
 	if (&region != this) {
 		bound = region.bound;
@@ -68,6 +84,10 @@ BRegion::BRegion(const BRegion &region)
 		data = (clipping_rect *)malloc(data_size * sizeof(clipping_rect));
 		
 		memcpy(data, region.data, count * sizeof(clipping_rect));
+	} else {
+
+		data = (clipping_rect *)malloc(data_size * sizeof(clipping_rect));
+		zero_region(this);
 	}
 }
 
@@ -76,7 +96,8 @@ BRegion::BRegion(const BRegion &region)
 	\param rect The BRect to set the region to.
 */
 BRegion::BRegion(const BRect rect)
-	:data_size(8),
+	:
+	data_size(8),
 	data(NULL)	
 {
 	data = (clipping_rect *)malloc(data_size * sizeof(clipping_rect));
@@ -201,11 +222,11 @@ BRegion::Intersects(BRect rect) const
 bool
 BRegion::Intersects(clipping_rect rect) const
 {
-	if (!valid_rect(sect_rect(rect, bound)))
+	if (!rects_intersect(rect, bound))
 		return false;
 
 	for (long c = 0; c < count; c++) {
-		if (valid_rect(sect_rect(data[c], rect)))
+		if (rects_intersect(data[c], rect))
 			return true;
 	}
 	
@@ -276,16 +297,10 @@ void
 BRegion::OffsetBy(int32 dh, int32 dv)
 {
 	if (count > 0) {
-		for (long c = 0; c < count; c++) {
+		for (long c = 0; c < count; c++)
-			data[c].left += dh;
+			offset_rect(data[c], dh, dv);
-			data[c].right += dh;
+
-			data[c].top += dv;
+		offset_rect(bound, dh, dv);	
-			data[c].bottom += dv;
-		}
-		bound.left += dh;
-		bound.right += dh;
-		bound.top += dv;
-		bound.bottom += dv;		
 	}
 }
 
@@ -358,6 +373,7 @@ BRegion::Exclude(clipping_rect rect)
 	BRegion newRegion;
 	
 	region.Set(rect);
+
 	sub_region(this, &region, &newRegion);
 	copy_region(&newRegion, this);
 }
@@ -423,46 +439,43 @@ BRegion::operator=(const BRegion &region)
 void
 BRegion::_AddRect(clipping_rect rect)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(count >= 0);
 	ASSERT(data_size >= 0);
 	
 	// Should we just reallocate the memory and
 	// copy the rect ?
-	bool add = false; 
+	bool addRect = true; 
 		
-	if (count <= 0)
+	if (count > 0) {
-		// Yeah, we don't have any rect here..	
+		// Wait! We could merge the rect with one of the
-	 	add = true;
+		// existing rectangles, if it's adiacent.
-	else {
+		// We just check it against the last rectangle, since
-		// Wait! We could merge "rect" with one of the
+		// we are keeping them sorted by their "top" coordinates.
-		// existing rectangles...
 		long last = count - 1;
-		if (rect.top == data[last].bottom + 1
+		if (rect.left == data[last].left && rect.right == data[last].right
-				&& rect.left == data[last].left
+				&& rect.top == data[last].bottom + 1) {
-				&& rect.right == data[last].right) {
 		 
 			data[last].bottom = rect.bottom;
+			addRect = false;
 		
 		} else if (rect.top == data[last].top && rect.bottom == data[last].bottom) {			
+			if (rect.left == data[last].right + 1) {
 
-			if (rect.left == data[last].right + 1)
 				data[last].right = rect.right;
+				addRect = false;
+
+			} else if (rect.right == data[last].left - 1) {
 
-			else if (rect.right == data[last].left - 1)
 				data[last].left = rect.left;
-				
+				addRect = false;
-			else 
+			}	
-				add = true; //no luck... just add the rect
+		}
-		
-		} else
-			add = true; //just add the rect
 	}		
 	
 	// We weren't lucky.... just add the rect as a new one
-	if (add) {
+	if (addRect) {
 		if (data_size <= count)
-			set_size(data_size + 16);
+			set_size(count + 16);
 			
 		data[count] = rect;
 		
@@ -470,16 +483,16 @@ BRegion::_AddRect(clipping_rect rect)
 	}
 	
 	// Recalculate bounds
-	if (rect.top <= bound.top)
+	if (rect.top < bound.top)
 		bound.top = rect.top;
 
-	if (rect.left <= bound.left)
+	if (rect.left < bound.left)
 		bound.left = rect.left;
  
-	if (rect.right >= bound.right)
+	if (rect.right > bound.right)
 		bound.right = rect.right;
  
-	if (rect.bottom >= bound.bottom)
+	if (rect.bottom > bound.bottom)
 		bound.bottom = rect.bottom;
 }
 

src/kits/interface/region_helpers.cpp

@@ -1,8 +1,10 @@
-#include <string.h>
+#include <cstring>
+
 #include <Debug.h>
 #include <Region.h>
 
 #include <clipping.h>
+
 #include "region_helpers.h"
 
 static const int32 kMaxPoints = 1024;
@@ -17,8 +19,6 @@ static const int32 kMaxVerticalExtent = 0x10000000;
 void
 zero_region(BRegion *region)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
-	
 	region->count = 0;
 	region->bound.left = 0x7ffffffd;
 	region->bound.top = 0x7ffffffd;
@@ -30,7 +30,6 @@ zero_region(BRegion *region)
 void
 clear_region(BRegion *region)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	region->count = 0;
 	region->bound.left = 0xfffffff;
 	region->bound.top = 0xfffffff;
@@ -45,8 +44,6 @@ clear_region(BRegion *region)
 void
 cleanup_region_1(BRegion *region)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
-
 	clipping_rect testRect =
 		{
 			1, 1,
@@ -85,7 +82,6 @@ cleanup_region_1(BRegion *region)
 void
 cleanup_region(BRegion *region)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	long oldCount;
 	
 	do {
@@ -102,7 +98,6 @@ cleanup_region(BRegion *region)
 void
 sort_rects(clipping_rect *rects, long count)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	bool again; //flag that tells we changed rects positions
 			
 	if (count == 2) {
@@ -130,7 +125,6 @@ sort_rects(clipping_rect *rects, long count)
 void
 sort_trans(long *lptr1, long *lptr2, long count)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	bool again; //flag that tells we changed trans positions
 			
 	if (count == 2) {
@@ -169,8 +163,6 @@ sort_trans(long *lptr1, long *lptr2, long count)
 void
 cleanup_region_horizontal(BRegion *region)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
-
 	clipping_rect testRect =
 		{
 			1, 1,
@@ -204,7 +196,6 @@ cleanup_region_horizontal(BRegion *region)
 void
 copy_region(BRegion *source, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(source);
 	ASSERT(dest);
 	ASSERT(source != dest);
@@ -232,7 +223,6 @@ copy_region(BRegion *source, BRegion *dest)
 void
 copy_region_n(BRegion *source, BRegion *dest, long count)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(source);
 	ASSERT(dest);
 	ASSERT(source != dest);
@@ -262,7 +252,6 @@ copy_region_n(BRegion *source, BRegion *dest, long count)
 void
 and_region_complex(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(first);
 	ASSERT(second);
 	ASSERT(dest);
@@ -292,7 +281,6 @@ and_region_complex(BRegion *first, BRegion *second, BRegion *dest)
 void
 and_region_1_to_n(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(first);
 	ASSERT(second);
 	ASSERT(dest);
@@ -329,7 +317,6 @@ and_region_1_to_n(BRegion *first, BRegion *second, BRegion *dest)
 void
 and_region(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(first);
 	ASSERT(second);
 	ASSERT(dest);
@@ -367,7 +354,6 @@ and_region(BRegion *first, BRegion *second, BRegion *dest)
 void
 append_region(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(first);
 	ASSERT(second);
 	ASSERT(dest);
@@ -382,8 +368,6 @@ append_region(BRegion *first, BRegion *second, BRegion *dest)
 void
 r_or(long top, long bottom, BRegion *first, BRegion *second, BRegion *dest, long *indexA, long *indexB)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
-	
 	int32 lefts[kMaxPoints];
 	int32 rights[kMaxPoints];
 	
@@ -469,7 +453,6 @@ r_or(long top, long bottom, BRegion *first, BRegion *second, BRegion *dest, long
 void
 or_region_complex(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	long a = 0, b = 0;
 	
 	int32 top;
@@ -518,7 +501,6 @@ or_region_complex(BRegion *first, BRegion *second, BRegion *dest)
 void
 or_region_1_to_n(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(first);
 	ASSERT(second);
 	ASSERT(dest);
@@ -546,7 +528,6 @@ or_region_1_to_n(BRegion *first, BRegion *second, BRegion *dest)
 void
 or_region_no_x(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(first);
 	ASSERT(second);
 	ASSERT(dest);
@@ -600,7 +581,6 @@ or_region_no_x(BRegion *first, BRegion *second, BRegion *dest)
 void
 or_region(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(first);
 	ASSERT(second);
 	ASSERT(dest);
@@ -652,7 +632,6 @@ or_region(BRegion *first, BRegion *second, BRegion *dest)
 void
 sub_region_complex(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	long a = 0, b = 0;
 	
 	int32 top;
@@ -693,8 +672,6 @@ sub_region_complex(BRegion *first, BRegion *second, BRegion *dest)
 void
 r_sub(long top, long bottom, BRegion *first, BRegion *second, BRegion *dest, long *indexA, long *indexB)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
-	
 	int32 leftsA[kMaxPoints / 2];
 	int32 rightsA[kMaxPoints / 2];
 	int32 leftsB[kMaxPoints / 2];
@@ -837,7 +814,6 @@ r_sub(long top, long bottom, BRegion *first, BRegion *second, BRegion *dest, lon
 void
 sub_region(BRegion *first, BRegion *second, BRegion *dest)
 {
-	PRINT(("%s\n", __PRETTY_FUNCTION__));
 	ASSERT(first);
 	ASSERT(second);
 	ASSERT(dest);
@@ -845,7 +821,7 @@ sub_region(BRegion *first, BRegion *second, BRegion *dest)
 	if (first->count == 0)
 		zero_region(dest);
 		
-	else if (second->count == 0	|| !valid_rect(sect_rect(first->bound, second->bound)))
+	else if (second->count == 0	|| !rects_intersect(first->bound, second->bound))
 		copy_region(first, dest);
 		
 	else

src/kits/interface/region_helpers.h

@@ -19,7 +19,6 @@ void or_region_complex(BRegion*, BRegion*, BRegion*);
 void or_region_1_to_n(BRegion*, BRegion*, BRegion*);
 void or_region_no_x(BRegion*, BRegion*, BRegion*);
 void or_region(BRegion*, BRegion*, BRegion*);
-//void sub(long, long, BRegion*, BRegion*, BRegion*, long*, long*);
 void sub_region_complex(BRegion*, BRegion*, BRegion*);
 void r_sub(long , long, BRegion*, BRegion*, BRegion*, long*, long*);
 void sub_region(BRegion*, BRegion*, BRegion*);