From dd4eefcb295ebf35917a8e3d11fa3082019cf86f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 18 Oct 2011 21:56:48 +0000 Subject: [PATCH] Document and test behaviour of wxRegion methods when it is invalid. Document which wxRegion methods can and can't be used when the region itself is invalid. Apply the minor changes to wxGTK (Xor() didn't do the right thing, Offset() didn't assert) and wxOSX (Offset() crashed) to make their behaviour consistent with wxMSW. Add a (trivial, so far, but to be extended later) wxRegion unit test checking that the methods do indeed behave as documented. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@69461 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- interface/wx/region.h | 70 +++++++++++++++++++- src/gtk/region.cpp | 21 +++--- src/osx/carbon/region.cpp | 2 +- tests/Makefile.in | 4 ++ tests/geometry/region.cpp | 114 +++++++++++++++++++++++++++++++++ tests/makefile.bcc | 4 ++ tests/makefile.gcc | 4 ++ tests/makefile.vc | 4 ++ tests/makefile.wat | 4 ++ tests/test.bkl | 1 + tests/test_test_gui.dsp | 4 ++ tests/test_vc7_test_gui.vcproj | 3 + tests/test_vc8_test_gui.vcproj | 4 ++ tests/test_vc9_test_gui.vcproj | 4 ++ 14 files changed, 230 insertions(+), 13 deletions(-) create mode 100644 tests/geometry/region.cpp diff --git a/interface/wx/region.h b/interface/wx/region.h index 7430c592e5..9685c82f24 100644 --- a/interface/wx/region.h +++ b/interface/wx/region.h @@ -157,6 +157,9 @@ class wxRegion : public wxGDIObject public: /** Default constructor. + + This constructor creates an invalid, or null, object, i.e. calling + IsOk() on it returns @false and IsNull() returns @true. */ wxRegion(); /** @@ -202,18 +205,26 @@ public: /** Clears the current region. + + The object becomes invalid, or null, after being cleared. */ virtual void Clear(); /** Returns a value indicating whether the given point is contained within the region. + This method always returns @c wxOutRegion for an invalid region but + may, nevertheless, be safely called in this case. + @return The return value is one of @c wxOutRegion and @c wxInRegion. */ wxRegionContain Contains(wxCoord x, wxCoord y) const; /** Returns a value indicating whether the given point is contained within the region. + This method always returns @c wxOutRegion for an invalid region but + may, nevertheless, be safely called in this case. + @return The return value is one of @c wxOutRegion and @c wxInRegion. */ wxRegionContain Contains(const wxPoint& pt) const; @@ -221,6 +232,9 @@ public: Returns a value indicating whether the given rectangle is contained within the region. + This method always returns @c wxOutRegion for an invalid region but + may, nevertheless, be safely called in this case. + @return One of ::wxOutRegion, ::wxPartRegion or ::wxInRegion. @note On Windows, only ::wxOutRegion and ::wxInRegion are returned; a value @@ -232,6 +246,9 @@ public: Returns a value indicating whether the given rectangle is contained within the region. + This method always returns @c wxOutRegion for an invalid region but + may, nevertheless, be safely called in this case. + @return One of ::wxOutRegion, ::wxPartRegion or ::wxInRegion. @note On Windows, only ::wxOutRegion and ::wxInRegion are returned; a value @@ -243,12 +260,16 @@ public: /** Convert the region to a black and white bitmap with the white pixels being inside the region. + + This method can't be used for invalid region. */ wxBitmap ConvertToBitmap() const; //@{ /** Returns the outer bounds of the region. + + This method returns 0-sized bounding box for invalid regions. */ void GetBox(wxCoord& x, wxCoord& y, wxCoord& width, wxCoord& height) const; @@ -259,6 +280,9 @@ public: Finds the intersection of this region and another, rectangular region, specified using position and size. + This method always fails, i.e. returns @false, if this region is + invalid but may nevertheless be safely used even in this case. + @return @true if successful, @false otherwise. @remarks Creates the intersection of the two regions, that is, the parts @@ -270,6 +294,9 @@ public: /** Finds the intersection of this region and another, rectangular region. + This method always fails, i.e. returns @false, if this region is + invalid but may nevertheless be safely used even in this case. + @return @true if successful, @false otherwise. @remarks Creates the intersection of the two regions, that is, the parts @@ -280,6 +307,9 @@ public: /** Finds the intersection of this region and another region. + This method always fails, i.e. returns @false, if this region is + invalid but may nevertheless be safely used even in this case. + @return @true if successful, @false otherwise. @remarks Creates the intersection of the two regions, that is, the parts @@ -290,6 +320,8 @@ public: /** Returns @true if the region is empty, @false otherwise. + + Always returns @true if the region is invalid. */ virtual bool IsEmpty() const; @@ -297,8 +329,8 @@ public: Returns @true if the region is equal to, i.e. covers the same area as, another one. - @note If both this region and @a region are invalid, they are - considered to be equal. + If both this region and @a region are both invalid, they are considered + to be equal. */ bool IsEqual(const wxRegion& region) const; @@ -307,6 +339,10 @@ public: Moves the region by the specified offsets in horizontal and vertical directions. + This method can't be called if the region is invalid as it doesn't make + sense to offset it then. Attempts to do it will result in assert + failure. + @return @true if successful, @false otherwise (the region is unchanged then). */ @@ -317,6 +353,9 @@ public: /** Subtracts a rectangular region from this region. + This method always fails, i.e. returns @false, if this region is + invalid but may nevertheless be safely used even in this case. + @return @true if successful, @false otherwise. @remarks This operation combines the parts of 'this' region that are not @@ -327,6 +366,9 @@ public: /** Subtracts a region from this region. + This method always fails, i.e. returns @false, if this region is + invalid but may nevertheless be safely used even in this case. + @return @true if successful, @false otherwise. @remarks This operation combines the parts of 'this' region that are not @@ -339,6 +381,10 @@ public: Finds the union of this region and another, rectangular region, specified using position and size. + This method can be used even if this region is invalid and has the + natural behaviour in this case, i.e. makes this region equal to the + given rectangle. + @return @true if successful, @false otherwise. @remarks This operation creates a region that combines all of this region @@ -349,6 +395,10 @@ public: /** Finds the union of this region and another, rectangular region. + This method can be used even if this region is invalid and has the + natural behaviour in this case, i.e. makes this region equal to the + given rectangle. + @return @true if successful, @false otherwise. @remarks This operation creates a region that combines all of this region @@ -359,6 +409,10 @@ public: /** Finds the union of this region and another region. + This method can be used even if this region is invalid and has the + natural behaviour in this case, i.e. makes this region equal to the + given @a region. + @return @true if successful, @false otherwise. @remarks This operation creates a region that combines all of this region @@ -396,6 +450,10 @@ public: Finds the Xor of this region and another, rectangular region, specified using position and size. + This method can be used even if this region is invalid and has the + natural behaviour in this case, i.e. makes this region equal to the + given rectangle. + @return @true if successful, @false otherwise. @remarks This operation creates a region that combines all of this region @@ -406,6 +464,10 @@ public: /** Finds the Xor of this region and another, rectangular region. + This method can be used even if this region is invalid and has the + natural behaviour in this case, i.e. makes this region equal to the + given rectangle. + @return @true if successful, @false otherwise. @remarks This operation creates a region that combines all of this region @@ -416,6 +478,10 @@ public: /** Finds the Xor of this region and another region. + This method can be used even if this region is invalid and has the + natural behaviour in this case, i.e. makes this region equal to the + given @a region. + @return @true if successful, @false otherwise. @remarks This operation creates a region that combines all of this region diff --git a/src/gtk/region.cpp b/src/gtk/region.cpp index 669dc09424..cf1d5d363b 100644 --- a/src/gtk/region.cpp +++ b/src/gtk/region.cpp @@ -183,16 +183,14 @@ bool wxRegion::DoUnionWithRegion( const wxRegion& region ) if (!m_refData) { - m_refData = new wxRegionRefData(); - M_REGIONDATA->m_region = gdk_region_new(); + m_refData = new wxRegionRefData(*M_REGIONDATA_OF(region)); } else { AllocExclusive(); + gdk_region_union( M_REGIONDATA->m_region, region.GetRegion() ); } - gdk_region_union( M_REGIONDATA->m_region, region.GetRegion() ); - return true; } @@ -236,20 +234,23 @@ bool wxRegion::DoXor( const wxRegion& region ) if (!m_refData) { - return false; + // XOR-ing with an invalid region is the same as XOR-ing with an empty + // one, i.e. it is simply a copy. + m_refData = new wxRegionRefData(*M_REGIONDATA_OF(region)); } + else + { + AllocExclusive(); - AllocExclusive(); - - gdk_region_xor( M_REGIONDATA->m_region, region.GetRegion() ); + gdk_region_xor( M_REGIONDATA->m_region, region.GetRegion() ); + } return true; } bool wxRegion::DoOffset( wxCoord x, wxCoord y ) { - if (!m_refData) - return false; + wxCHECK_MSG( m_refData, false, wxS("invalid region") ); AllocExclusive(); diff --git a/src/osx/carbon/region.cpp b/src/osx/carbon/region.cpp index ff12238509..83a2e15e12 100644 --- a/src/osx/carbon/region.cpp +++ b/src/osx/carbon/region.cpp @@ -178,7 +178,7 @@ void wxRegion::Clear() // Move the region bool wxRegion::DoOffset(wxCoord x, wxCoord y) { - wxCHECK_MSG( M_REGION, false, wxT("invalid wxRegion") ); + wxCHECK_MSG( m_refData, false, wxT("invalid wxRegion") ); if ( !x && !y ) // nothing to do diff --git a/tests/Makefile.in b/tests/Makefile.in index 1fda4594c7..83e0719b6d 100644 --- a/tests/Makefile.in +++ b/tests/Makefile.in @@ -151,6 +151,7 @@ TEST_GUI_OBJECTS = \ test_gui_rect.o \ test_gui_size.o \ test_gui_point.o \ + test_gui_region.o \ test_gui_bitmap.o \ test_gui_colour.o \ test_gui_ellipsization.o \ @@ -705,6 +706,9 @@ test_gui_size.o: $(srcdir)/geometry/size.cpp $(TEST_GUI_ODEP) test_gui_point.o: $(srcdir)/geometry/point.cpp $(TEST_GUI_ODEP) $(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/geometry/point.cpp +test_gui_region.o: $(srcdir)/geometry/region.cpp $(TEST_GUI_ODEP) + $(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/geometry/region.cpp + test_gui_bitmap.o: $(srcdir)/graphics/bitmap.cpp $(TEST_GUI_ODEP) $(CXXC) -c -o $@ $(TEST_GUI_CXXFLAGS) $(srcdir)/graphics/bitmap.cpp diff --git a/tests/geometry/region.cpp b/tests/geometry/region.cpp new file mode 100644 index 0000000000..f6ae795e17 --- /dev/null +++ b/tests/geometry/region.cpp @@ -0,0 +1,114 @@ +/////////////////////////////////////////////////////////////////////////////// +// Name: tests/geometry/region.cpp +// Purpose: wxRegion unit test +// Author: Vadim Zeitlin +// Created: 2011-10-12 +// RCS-ID: $Id$ +// Copyright: (c) 2011 Vadim Zeitlin +/////////////////////////////////////////////////////////////////////////////// + +// ---------------------------------------------------------------------------- +// headers +// ---------------------------------------------------------------------------- + +#include "testprec.h" + +#ifdef __BORLANDC__ + #pragma hdrstop +#endif + +#ifndef WX_PRECOMP + #include "wx/region.h" +#endif // WX_PRECOMP + +#include "wx/iosfwrap.h" + +// ---------------------------------------------------------------------------- +// helper functions +// ---------------------------------------------------------------------------- + +namespace +{ + +// This function could be easily added to wxRegionIterator itself, where it +// could be implemented far more efficiently as all major platforms store the +// number of rectangles anyhow, but as we only use it for debugging purposes, +// just keep it here for now. +unsigned GetRectsCount(const wxRegion& rgn) +{ + unsigned count = 0; + for ( wxRegionIterator iter(rgn); iter.HaveRects(); ++iter ) + count++; + return count; +} + +} // anonymous namespace + +// this operator is needed to use CPPUNIT_ASSERT_EQUAL with wxRegions +std::ostream& operator<<(std::ostream& os, const wxRegion& rgn) +{ + wxRect r = rgn.GetBox(); + os << "# rects = " << GetRectsCount(rgn) + << "; bounding box {" + << r.x << ", " << r.y << ", " << r.width << ", " << r.height + << "}"; + return os; +} + +// ---------------------------------------------------------------------------- +// test class +// ---------------------------------------------------------------------------- + +class RegionTestCase : public CppUnit::TestCase +{ +public: + RegionTestCase() { } + +private: + CPPUNIT_TEST_SUITE( RegionTestCase ); + CPPUNIT_TEST( Validity ); + CPPUNIT_TEST_SUITE_END(); + + void Validity(); + + wxDECLARE_NO_COPY_CLASS(RegionTestCase); +}; + +// register in the unnamed registry so that these tests are run by default +CPPUNIT_TEST_SUITE_REGISTRATION( RegionTestCase ); + +// also include in its own registry so that these tests can be run alone +CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( RegionTestCase, "RegionTestCase" ); + +void RegionTestCase::Validity() +{ + wxRegion r; + + CPPUNIT_ASSERT_MESSAGE + ( + "Default constructed region must be invalid", + !r.IsOk() + ); + + CPPUNIT_ASSERT_MESSAGE + ( + "Invalid region should be empty", + r.IsEmpty() + ); + + // Offsetting an invalid region doesn't make sense. + WX_ASSERT_FAILS_WITH_ASSERT( r.Offset(1, 1) ); + + CPPUNIT_ASSERT_MESSAGE + ( + "Combining with a valid region should create a valid region", + r.Union(0, 0, 10, 10) + ); + + CPPUNIT_ASSERT_EQUAL_MESSAGE + ( + "Union() with invalid region should give the same region", + wxRegion(0, 0, 10, 10), + r + ); +} diff --git a/tests/makefile.bcc b/tests/makefile.bcc index 081c70a0cd..56bd7d3dd3 100644 --- a/tests/makefile.bcc +++ b/tests/makefile.bcc @@ -136,6 +136,7 @@ TEST_GUI_OBJECTS = \ $(OBJS)\test_gui_rect.obj \ $(OBJS)\test_gui_size.obj \ $(OBJS)\test_gui_point.obj \ + $(OBJS)\test_gui_region.obj \ $(OBJS)\test_gui_bitmap.obj \ $(OBJS)\test_gui_colour.obj \ $(OBJS)\test_gui_ellipsization.obj \ @@ -746,6 +747,9 @@ $(OBJS)\test_gui_size.obj: .\geometry\size.cpp $(OBJS)\test_gui_point.obj: .\geometry\point.cpp $(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\geometry\point.cpp +$(OBJS)\test_gui_region.obj: .\geometry\region.cpp + $(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\geometry\region.cpp + $(OBJS)\test_gui_bitmap.obj: .\graphics\bitmap.cpp $(CXX) -q -c -P -o$@ $(TEST_GUI_CXXFLAGS) .\graphics\bitmap.cpp diff --git a/tests/makefile.gcc b/tests/makefile.gcc index 1498b6a970..b7baaccd35 100644 --- a/tests/makefile.gcc +++ b/tests/makefile.gcc @@ -129,6 +129,7 @@ TEST_GUI_OBJECTS = \ $(OBJS)\test_gui_rect.o \ $(OBJS)\test_gui_size.o \ $(OBJS)\test_gui_point.o \ + $(OBJS)\test_gui_region.o \ $(OBJS)\test_gui_bitmap.o \ $(OBJS)\test_gui_colour.o \ $(OBJS)\test_gui_ellipsization.o \ @@ -727,6 +728,9 @@ $(OBJS)\test_gui_size.o: ./geometry/size.cpp $(OBJS)\test_gui_point.o: ./geometry/point.cpp $(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $< +$(OBJS)\test_gui_region.o: ./geometry/region.cpp + $(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $< + $(OBJS)\test_gui_bitmap.o: ./graphics/bitmap.cpp $(CXX) -c -o $@ $(TEST_GUI_CXXFLAGS) $(CPPDEPS) $< diff --git a/tests/makefile.vc b/tests/makefile.vc index 0442379cb7..198f67cf3a 100644 --- a/tests/makefile.vc +++ b/tests/makefile.vc @@ -131,6 +131,7 @@ TEST_GUI_OBJECTS = \ $(OBJS)\test_gui_rect.obj \ $(OBJS)\test_gui_size.obj \ $(OBJS)\test_gui_point.obj \ + $(OBJS)\test_gui_region.obj \ $(OBJS)\test_gui_bitmap.obj \ $(OBJS)\test_gui_colour.obj \ $(OBJS)\test_gui_ellipsization.obj \ @@ -872,6 +873,9 @@ $(OBJS)\test_gui_size.obj: .\geometry\size.cpp $(OBJS)\test_gui_point.obj: .\geometry\point.cpp $(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\geometry\point.cpp +$(OBJS)\test_gui_region.obj: .\geometry\region.cpp + $(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\geometry\region.cpp + $(OBJS)\test_gui_bitmap.obj: .\graphics\bitmap.cpp $(CXX) /c /nologo /TP /Fo$@ $(TEST_GUI_CXXFLAGS) .\graphics\bitmap.cpp diff --git a/tests/makefile.wat b/tests/makefile.wat index af16e37d8a..f5cebac92a 100644 --- a/tests/makefile.wat +++ b/tests/makefile.wat @@ -380,6 +380,7 @@ TEST_GUI_OBJECTS = & $(OBJS)\test_gui_rect.obj & $(OBJS)\test_gui_size.obj & $(OBJS)\test_gui_point.obj & + $(OBJS)\test_gui_region.obj & $(OBJS)\test_gui_bitmap.obj & $(OBJS)\test_gui_colour.obj & $(OBJS)\test_gui_ellipsization.obj & @@ -787,6 +788,9 @@ $(OBJS)\test_gui_size.obj : .AUTODEPEND .\geometry\size.cpp $(OBJS)\test_gui_point.obj : .AUTODEPEND .\geometry\point.cpp $(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $< +$(OBJS)\test_gui_region.obj : .AUTODEPEND .\geometry\region.cpp + $(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $< + $(OBJS)\test_gui_bitmap.obj : .AUTODEPEND .\graphics\bitmap.cpp $(CXX) -bt=nt -zq -fo=$^@ $(TEST_GUI_CXXFLAGS) $< diff --git a/tests/test.bkl b/tests/test.bkl index a35147fddd..bec842d7f1 100644 --- a/tests/test.bkl +++ b/tests/test.bkl @@ -132,6 +132,7 @@ geometry/rect.cpp geometry/size.cpp geometry/point.cpp + geometry/region.cpp graphics/bitmap.cpp graphics/colour.cpp graphics/ellipsization.cpp diff --git a/tests/test_test_gui.dsp b/tests/test_test_gui.dsp index baeb86696c..e8729ec1ce 100644 --- a/tests/test_test_gui.dsp +++ b/tests/test_test_gui.dsp @@ -461,6 +461,10 @@ SOURCE=.\geometry\rect.cpp # End Source File # Begin Source File +SOURCE=.\geometry\region.cpp +# End Source File +# Begin Source File + SOURCE=.\controls\richtextctrltest.cpp # End Source File # Begin Source File diff --git a/tests/test_vc7_test_gui.vcproj b/tests/test_vc7_test_gui.vcproj index e237709d40..2b7ea74c79 100644 --- a/tests/test_vc7_test_gui.vcproj +++ b/tests/test_vc7_test_gui.vcproj @@ -782,6 +782,9 @@ RelativePath=".\geometry\rect.cpp"> + + + + diff --git a/tests/test_vc9_test_gui.vcproj b/tests/test_vc9_test_gui.vcproj index 2871094105..caa1aa5c8e 100644 --- a/tests/test_vc9_test_gui.vcproj +++ b/tests/test_vc9_test_gui.vcproj @@ -1088,6 +1088,10 @@ > + + -- 2.45.2