From ce5301e4e604150c8b49c1ffd40be3ee4a7b8416 Mon Sep 17 00:00:00 2001 From: Jay Nabonne Date: Mon, 21 Jan 2019 12:35:55 +0000 Subject: [PATCH 01/10] Fix off by 1 errors in wxQt wxDC::SetClippingRegion() --- src/qt/dc.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qt/dc.cpp b/src/qt/dc.cpp index 073a10d755..2d91673124 100644 --- a/src/qt/dc.cpp +++ b/src/qt/dc.cpp @@ -434,9 +434,9 @@ void wxQtDCImpl::DoSetClippingRegion(wxCoord x, wxCoord y, wxRect clipRect = m_clippingRegion->GetBox(); m_clipX1 = clipRect.GetLeft(); - m_clipX2 = clipRect.GetRight(); + m_clipX2 = clipRect.GetRight() + 1; m_clipY1 = clipRect.GetTop(); - m_clipY2 = clipRect.GetBottom(); + m_clipY2 = clipRect.GetBottom() + 1; m_clipping = true; } } @@ -472,9 +472,9 @@ void wxQtDCImpl::DoSetDeviceClippingRegion(const wxRegion& region) wxRect clipRect = m_clippingRegion->GetBox(); m_clipX1 = clipRect.GetLeft(); - m_clipX2 = clipRect.GetRight(); + m_clipX2 = clipRect.GetRight() + 1; m_clipY1 = clipRect.GetTop(); - m_clipY2 = clipRect.GetBottom(); + m_clipY2 = clipRect.GetBottom() + 1; m_clipping = true; } } From a130a59111353c5277abcb37605845ae7d5b143e Mon Sep 17 00:00:00 2001 From: Jay Nabonne Date: Mon, 21 Jan 2019 12:35:55 +0000 Subject: [PATCH 02/10] Handle clipping regions with negative width or height in wxQt --- src/qt/dc.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/qt/dc.cpp b/src/qt/dc.cpp index 2d91673124..35dec4b0ac 100644 --- a/src/qt/dc.cpp +++ b/src/qt/dc.cpp @@ -416,6 +416,17 @@ void wxQtDCImpl::DoSetClippingRegion(wxCoord x, wxCoord y, } else { + if ( width < 0 ) + { + width = -width; + x -= width - 1; + } + if ( height < 0 ) + { + height = -height; + y -= height - 1; + } + if (m_qtPainter->isActive()) { // Set QPainter clipping (intersection if not the first one) From 5b8b30d2436f0488ca3ebaaf6d8feae23a778b5f Mon Sep 17 00:00:00 2001 From: Jay Nabonne Date: Mon, 21 Jan 2019 12:35:55 +0000 Subject: [PATCH 03/10] Don't use pointer for wxDC::m_clippingRegion in wxQt An object can be used directly instead. --- include/wx/qt/dc.h | 2 +- src/qt/dc.cpp | 18 ++++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/include/wx/qt/dc.h b/include/wx/qt/dc.h index 43bd13e070..d1c632f714 100644 --- a/include/wx/qt/dc.h +++ b/include/wx/qt/dc.h @@ -116,7 +116,7 @@ protected: QPainter *m_qtPainter; QPixmap *m_qtPixmap; - wxRegion *m_clippingRegion; + wxRegion m_clippingRegion; private: enum wxQtRasterColourOp { diff --git a/src/qt/dc.cpp b/src/qt/dc.cpp index 35dec4b0ac..7ba806795f 100644 --- a/src/qt/dc.cpp +++ b/src/qt/dc.cpp @@ -48,7 +48,6 @@ wxIMPLEMENT_CLASS(wxQtDCImpl,wxDCImpl); wxQtDCImpl::wxQtDCImpl( wxDC *owner ) : wxDCImpl( owner ) { - m_clippingRegion = new wxRegion; m_qtPixmap = NULL; m_rasterColourOp = wxQtNONE; m_qtPenColor = new QColor; @@ -67,7 +66,6 @@ wxQtDCImpl::~wxQtDCImpl() delete m_qtPainter; } - delete m_clippingRegion; delete m_qtPenColor; delete m_qtBrushColor; } @@ -87,7 +85,7 @@ void wxQtDCImpl::QtPreparePainter( ) if (m_clipping) { - wxRegionIterator ri(*m_clippingRegion); + wxRegionIterator ri(m_clippingRegion); bool append = false; while (ri.HaveRects()) { @@ -438,11 +436,11 @@ void wxQtDCImpl::DoSetClippingRegion(wxCoord x, wxCoord y, /* Note: Qt states that QPainter::clipRegion() may be slow, so we * keep the region manually, which should be faster */ if ( m_clipping ) - m_clippingRegion->Union( wxRect( x, y, width, height ) ); + m_clippingRegion.Union( wxRect( x, y, width, height ) ); else - m_clippingRegion->Intersect( wxRect( x, y, width, height ) ); + m_clippingRegion.Intersect( wxRect( x, y, width, height ) ); - wxRect clipRect = m_clippingRegion->GetBox(); + wxRect clipRect = m_clippingRegion.GetBox(); m_clipX1 = clipRect.GetLeft(); m_clipX2 = clipRect.GetRight() + 1; @@ -476,11 +474,11 @@ void wxQtDCImpl::DoSetDeviceClippingRegion(const wxRegion& region) /* Note: Qt states that QPainter::clipRegion() may be slow, so we * keep the region manually, which should be faster */ if ( m_clipping ) - m_clippingRegion->Union( region ); + m_clippingRegion.Union( region ); else - m_clippingRegion->Intersect( region ); + m_clippingRegion.Intersect( region ); - wxRect clipRect = m_clippingRegion->GetBox(); + wxRect clipRect = m_clippingRegion.GetBox(); m_clipX1 = clipRect.GetLeft(); m_clipX2 = clipRect.GetRight() + 1; @@ -493,7 +491,7 @@ void wxQtDCImpl::DoSetDeviceClippingRegion(const wxRegion& region) void wxQtDCImpl::DestroyClippingRegion() { wxDCImpl::DestroyClippingRegion(); - m_clippingRegion->Clear(); + m_clippingRegion.Clear(); if (m_qtPainter->isActive()) m_qtPainter->setClipping( false ); From 5929c5831eb8959114839ec6b4ddeeb4b00e17c8 Mon Sep 17 00:00:00 2001 From: Jay Nabonne Date: Mon, 21 Jan 2019 12:35:55 +0000 Subject: [PATCH 04/10] Fix handling of empty clipping region in wxQt --- src/qt/dc.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/qt/dc.cpp b/src/qt/dc.cpp index 7ba806795f..dfde8af7d1 100644 --- a/src/qt/dc.cpp +++ b/src/qt/dc.cpp @@ -435,10 +435,14 @@ void wxQtDCImpl::DoSetClippingRegion(wxCoord x, wxCoord y, // Set internal state for getters /* Note: Qt states that QPainter::clipRegion() may be slow, so we * keep the region manually, which should be faster */ - if ( m_clipping ) - m_clippingRegion.Union( wxRect( x, y, width, height ) ); - else - m_clippingRegion.Intersect( wxRect( x, y, width, height ) ); + if ( !m_clipping || m_clippingRegion.IsEmpty() ) + { + int dcwidth, dcheight; + DoGetSize(&dcwidth, &dcheight); + + m_clippingRegion = wxRegion(0, 0, dcwidth, dcheight); + } + m_clippingRegion.Intersect( wxRect(x, y, width, height) ); wxRect clipRect = m_clippingRegion.GetBox(); From 0cd2f7b68791a51c9f67b71d8aade2d1d4fa841e Mon Sep 17 00:00:00 2001 From: Jay Nabonne Date: Mon, 21 Jan 2019 12:35:55 +0000 Subject: [PATCH 05/10] Remove unnecessary checks for null pointer before deleting it No real changes. --- src/qt/region.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qt/region.cpp b/src/qt/region.cpp index a0c3ea8698..7dfb58aa41 100644 --- a/src/qt/region.cpp +++ b/src/qt/region.cpp @@ -279,14 +279,12 @@ wxRegionIterator::wxRegionIterator(const wxRegionIterator& ri) wxRegionIterator::~wxRegionIterator() { - if ( m_qtRects != NULL ) - delete m_qtRects; + delete m_qtRects; } wxRegionIterator& wxRegionIterator::operator=(const wxRegionIterator& ri) { - if ( m_qtRects != NULL ) - delete m_qtRects; + delete m_qtRects; m_qtRects = new QVector< QRect >( *ri.m_qtRects ); m_pos = ri.m_pos; @@ -300,8 +298,7 @@ void wxRegionIterator::Reset() void wxRegionIterator::Reset(const wxRegion& region) { - if ( m_qtRects != NULL ) - delete m_qtRects; + delete m_qtRects; m_qtRects = new QVector< QRect >( region.GetHandle().rects() ); m_pos = 0; From 637b861eb1bcd9c4151c7d008b7e67e92183d913 Mon Sep 17 00:00:00 2001 From: Jay Nabonne Date: Mon, 21 Jan 2019 12:35:55 +0000 Subject: [PATCH 06/10] Don't allocate wxRegionRefData at all for empty region Empty regions don't really need any data. --- src/qt/region.cpp | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/qt/region.cpp b/src/qt/region.cpp index 7dfb58aa41..0dd73003b6 100644 --- a/src/qt/region.cpp +++ b/src/qt/region.cpp @@ -57,7 +57,7 @@ wxIMPLEMENT_DYNAMIC_CLASS(wxRegion,wxGDIObject); wxRegion::wxRegion() { - m_refData = new wxRegionRefData(); + m_refData = NULL; } wxRegion::wxRegion(wxCoord x, wxCoord y, wxCoord w, wxCoord h) @@ -133,14 +133,20 @@ wxRegion::wxRegion(const wxBitmap& bmp, const wxColour& transp, int tolerance) bool wxRegion::IsEmpty() const { - wxCHECK_MSG( IsOk(), true, "Invalid region" ); + if ( IsNull() ) + return true; + + wxCHECK_MSG(IsOk(), true, "Invalid region" ); return M_REGIONDATA.isEmpty(); } void wxRegion::Clear() { - wxCHECK_RET( IsOk(), "Invalid region" ); + if ( IsNull() ) + return; + + wxCHECK_RET(IsOk(), "Invalid region" ); AllocExclusive(); M_REGIONDATA = QRegion(); @@ -171,6 +177,15 @@ bool wxRegion::DoIsEqual(const wxRegion& region) const bool wxRegion::DoGetBox(wxCoord& x, wxCoord& y, wxCoord& w, wxCoord& h) const { + if ( m_refData == NULL ) + { + x = + y = + w = + h = 0; + return false; + } + wxCHECK_MSG( IsOk(), false, "Invalid region" ); QRect bounding = M_REGIONDATA.boundingRect(); From c28a8c9e7e3eb0b350d211eac84ae7760a8dbb34 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 21 Jan 2019 23:28:18 +0100 Subject: [PATCH 07/10] Implement wxRegion operations in terms of DoCombine() --- include/wx/qt/region.h | 2 + src/qt/region.cpp | 94 ++++++++++++++++++++++++++++++++---------- 2 files changed, 75 insertions(+), 21 deletions(-) diff --git a/include/wx/qt/region.h b/include/wx/qt/region.h index 7bb2a56d68..d49ad45964 100644 --- a/include/wx/qt/region.h +++ b/include/wx/qt/region.h @@ -48,6 +48,8 @@ protected: virtual bool DoSubtract(const wxRegion& region); virtual bool DoXor(const wxRegion& region); + virtual bool DoCombine(const wxRegion& rgn, wxRegionOp op); + private: wxDECLARE_DYNAMIC_CLASS(wxRegion); }; diff --git a/src/qt/region.cpp b/src/qt/region.cpp index 0dd73003b6..ec501ecdc3 100644 --- a/src/qt/region.cpp +++ b/src/qt/region.cpp @@ -154,6 +154,7 @@ void wxRegion::Clear() void wxRegion::QtSetRegion(QRegion region) { + AllocExclusive(); M_REGIONDATA = region; } @@ -219,47 +220,98 @@ bool wxRegion::DoOffset(wxCoord x, wxCoord y) return true; } -bool wxRegion::DoUnionWithRect(const wxRect& rect) +// combine another region with this one +bool wxRegion::DoCombine(const wxRegion& region, wxRegionOp op) { - wxCHECK_MSG( IsOk(), false, "Invalid region" ); + // we can't use the API functions if we don't have a valid region + if ( !m_refData ) + { + // combining with an empty/invalid region works differently depending + // on the operation + switch ( op ) + { + case wxRGN_COPY: + case wxRGN_OR: + case wxRGN_XOR: + *this = region; + break; - M_REGIONDATA = M_REGIONDATA.united( wxQtConvertRect( rect ) ); + default: + wxFAIL_MSG(wxT("unknown region operation")); + wxFALLTHROUGH; + + case wxRGN_AND: + case wxRGN_DIFF: + // leave empty/invalid + return false; + } + } + else // we have a valid region + { + AllocExclusive(); + + switch ( op ) + { + case wxRGN_AND: + M_REGIONDATA = M_REGIONDATA.intersected(region.GetHandle()); + break; + + case wxRGN_OR: + M_REGIONDATA = M_REGIONDATA.united(region.GetHandle()); + break; + + case wxRGN_XOR: + M_REGIONDATA = M_REGIONDATA.xored(region.GetHandle()); + break; + + case wxRGN_DIFF: + M_REGIONDATA = M_REGIONDATA.subtracted(region.GetHandle()); + break; + + default: + wxFAIL_MSG(wxT("unknown region operation")); + wxFALLTHROUGH; + + case wxRGN_COPY: + M_REGIONDATA = QRegion(region.GetHandle()); + break; + } + } return true; } bool wxRegion::DoUnionWithRegion(const wxRegion& region) { - wxCHECK_MSG( IsOk(), false, "Invalid region" ); - wxCHECK_MSG( region.IsOk(), false, "Invalid parameter region" ); - - M_REGIONDATA = M_REGIONDATA.united( region.GetHandle() ); - return true; + return DoCombine(region, wxRGN_OR); } bool wxRegion::DoIntersect(const wxRegion& region) { - wxCHECK_MSG( IsOk(), false, "Invalid region" ); - wxCHECK_MSG( region.IsOk(), false, "Invalid parameter region" ); - - M_REGIONDATA = M_REGIONDATA.intersected( region.GetHandle() ); - return true; + return DoCombine(region, wxRGN_AND); } bool wxRegion::DoSubtract(const wxRegion& region) { - wxCHECK_MSG( IsOk(), false, "Invalid region" ); - wxCHECK_MSG( region.IsOk(), false, "Invalid parameter region" ); - - M_REGIONDATA = M_REGIONDATA.subtracted( region.GetHandle() ); - return true; + return DoCombine(region, wxRGN_DIFF); } bool wxRegion::DoXor(const wxRegion& region) { + return DoCombine(region, wxRGN_XOR); +} + +bool wxRegion::DoUnionWithRect(const wxRect& rect) +{ + if ( m_refData == NULL ) + { + m_refData = new wxRegionRefData(wxQtConvertRect(rect)); + return true; + } + wxCHECK_MSG( IsOk(), false, "Invalid region" ); - wxCHECK_MSG( region.IsOk(), false, "Invalid parameter region" ); - - M_REGIONDATA = M_REGIONDATA.xored( region.GetHandle() ); + + AllocExclusive(); + M_REGIONDATA = M_REGIONDATA.united( wxQtConvertRect( rect ) ); return true; } From dddca2f5161614ee8247ea87e9d55f042465c9af Mon Sep 17 00:00:00 2001 From: Jay Nabonne Date: Mon, 21 Jan 2019 12:35:55 +0000 Subject: [PATCH 08/10] Unindent wxRegionRefData class body Only whitespace changes. --- src/qt/region.cpp | 48 +++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/qt/region.cpp b/src/qt/region.cpp index ec501ecdc3..47db60542f 100644 --- a/src/qt/region.cpp +++ b/src/qt/region.cpp @@ -20,35 +20,35 @@ class wxRegionRefData: public wxGDIRefData { - public: - wxRegionRefData() - { - } +public: + wxRegionRefData() + { + } - wxRegionRefData( QRect r ) : m_qtRegion( r ) - { - } + wxRegionRefData( QRect r ) : m_qtRegion( r ) + { + } - wxRegionRefData( QBitmap b ) : m_qtRegion ( b ) - { - } + wxRegionRefData( QBitmap b ) : m_qtRegion ( b ) + { + } - wxRegionRefData( QPolygon p, Qt::FillRule fr ) : m_qtRegion( p, fr ) - { - } + wxRegionRefData( QPolygon p, Qt::FillRule fr ) : m_qtRegion( p, fr ) + { + } - wxRegionRefData( const wxRegionRefData& data ) + wxRegionRefData( const wxRegionRefData& data ) : wxGDIRefData() - { - m_qtRegion = data.m_qtRegion; - } - - bool operator == (const wxRegionRefData& data) const - { - return m_qtRegion == data.m_qtRegion; - } - - QRegion m_qtRegion; + { + m_qtRegion = data.m_qtRegion; + } + + bool operator == (const wxRegionRefData& data) const + { + return m_qtRegion == data.m_qtRegion; + } + + QRegion m_qtRegion; }; #define M_REGIONDATA ((wxRegionRefData *)m_refData)->m_qtRegion From 8412a40b903ebc62325501bb7508b3fefe747fd8 Mon Sep 17 00:00:00 2001 From: Jay Nabonne Date: Mon, 21 Jan 2019 12:35:55 +0000 Subject: [PATCH 09/10] Make some local variables const No real changes. --- src/qt/region.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qt/region.cpp b/src/qt/region.cpp index 47db60542f..8a71b4ab3c 100644 --- a/src/qt/region.cpp +++ b/src/qt/region.cpp @@ -113,16 +113,16 @@ wxRegion::wxRegion(const wxBitmap& bmp, const wxColour& transp, int tolerance) memset(raw.get(), 0, bmp.GetWidth()*bmp.GetHeight()); QImage img(bmp.GetHandle()->toImage()); - int r = transp.Red(), g = transp.Green(), b = transp.Blue(); + const int r = transp.Red(), g = transp.Green(), b = transp.Blue(); for(int y=0; y tolerance || abs(c.green() - g) > tolerance || abs(c.blue() - b) > tolerance) { - int ind = y*img.width()+x; + const int ind = y*img.width()+x; raw[ind>>3] |= 1<<(ind&7); } } @@ -189,7 +189,7 @@ bool wxRegion::DoGetBox(wxCoord& x, wxCoord& y, wxCoord& w, wxCoord& h) const wxCHECK_MSG( IsOk(), false, "Invalid region" ); - QRect bounding = M_REGIONDATA.boundingRect(); + const QRect bounding = M_REGIONDATA.boundingRect(); x = bounding.x(); y = bounding.y(); w = bounding.width(); From 99b0efcdf27ce6e957fff1253655ed6963785be6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 21 Jan 2019 23:31:39 +0100 Subject: [PATCH 10/10] Reformat a couple of for loops Only whitespace changes. --- src/qt/region.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qt/region.cpp b/src/qt/region.cpp index 8a71b4ab3c..44672de82b 100644 --- a/src/qt/region.cpp +++ b/src/qt/region.cpp @@ -114,9 +114,9 @@ wxRegion::wxRegion(const wxBitmap& bmp, const wxColour& transp, int tolerance) QImage img(bmp.GetHandle()->toImage()); const int r = transp.Red(), g = transp.Green(), b = transp.Blue(); - for(int y=0; y tolerance ||