From 350867939a2560e8614edb5fe797cc9fbb12f1f6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 14:39:11 +0200 Subject: [PATCH] Add bool return value for wxDC::GetClippingBox() Determining whether there is an actual clipping region or not is not that simple, as shown by the recent problems in wxDCClipper code, so return a boolean value indicating this from GetClippingBox() directly, instead of requiring the caller to find it out on their own. This simplifies wxDCClipper code, as well as any other code calling GetClippingBox(), at the price of some extra complexity in wxDCImpl itself, which seems to be worth it. --- include/wx/dc.h | 60 ++++++++++++++++++++++++------------------- interface/wx/dc.h | 24 ++++++++++++++++- src/common/dcbase.cpp | 55 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 28 deletions(-) diff --git a/include/wx/dc.h b/include/wx/dc.h index 76b5df8058..5749d578b3 100644 --- a/include/wx/dc.h +++ b/include/wx/dc.h @@ -446,21 +446,18 @@ public: // NB: this function works with device coordinates, not the logical ones! virtual void DoSetDeviceClippingRegion(const wxRegion& region) = 0; - virtual void DoGetClippingBox(wxCoord *x, wxCoord *y, - wxCoord *w, wxCoord *h) const - { - int dcWidth, dcHeight; - DoGetSize(&dcWidth, &dcHeight); + // Method used to implement wxDC::GetClippingBox(). + // + // Default implementation returns values stored in m_clip[XY][12] member + // variables, so this method doesn't need to be overridden if they're kept + // up to date. + virtual bool DoGetClippingRect(wxRect& rect) const; - if ( x ) - *x = m_clipping ? m_clipX1 : DeviceToLogicalX(0); - if ( y ) - *y = m_clipping ? m_clipY1 : DeviceToLogicalY(0); - if ( w ) - *w = m_clipping ? m_clipX2 - m_clipX1 : DeviceToLogicalXRel(dcWidth); - if ( h ) - *h = m_clipping ? m_clipY2 - m_clipY1 : DeviceToLogicalYRel(dcHeight); - } + // This method is kept for backwards compatibility but shouldn't be used + // nor overridden in the new code, implement DoGetClippingRect() above + // instead. + virtual void DoGetClippingBox(wxCoord *x, wxCoord *y, + wxCoord *w, wxCoord *h) const; virtual void DestroyClippingRegion() { ResetClipping(); } @@ -736,6 +733,9 @@ protected: #endif // wxUSE_PALETTE private: + // Return the full DC area in logical coordinates. + wxRect GetLogicalArea() const; + wxDECLARE_ABSTRACT_CLASS(wxDCImpl); }; @@ -959,10 +959,22 @@ public: void DestroyClippingRegion() { m_pimpl->DestroyClippingRegion(); } - void GetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h) const - { m_pimpl->DoGetClippingBox(x, y, w, h); } - void GetClippingBox(wxRect& rect) const - { m_pimpl->DoGetClippingBox(&rect.x, &rect.y, &rect.width, &rect.height); } + bool GetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h) const + { + wxRect r; + const bool clipping = m_pimpl->DoGetClippingRect(r); + if ( x ) + *x = r.x; + if ( y ) + *y = r.y; + if ( w ) + *w = r.width; + if ( h ) + *h = r.height; + return clipping; + } + bool GetClippingBox(wxRect& rect) const + { return m_pimpl->DoGetClippingRect(rect); } // coordinates conversions and transforms @@ -1451,7 +1463,7 @@ public: ~wxDCClipper() { m_dc.DestroyClippingRegion(); - if ( !m_oldClipRect.IsEmpty() ) + if ( m_restoreOld ) m_dc.SetClippingRegion(m_oldClipRect); } @@ -1459,19 +1471,13 @@ private: // Common part of all ctors. void Init(const wxRect& r) { - // GetClippingBox() is supposed to return the rectangle corresponding - // to the full DC area and some implementations actually do it, while - // others return an empty rectangle instead. Check for both possible - // results here to avoid restoring the clipping region unnecessarily in - // the dtor. - m_dc.GetClippingBox(m_oldClipRect); - if ( m_oldClipRect == m_dc.GetSize() ) - m_oldClipRect = wxRect(); + m_restoreOld = m_dc.GetClippingBox(m_oldClipRect); m_dc.SetClippingRegion(r); } wxDC& m_dc; wxRect m_oldClipRect; + bool m_restoreOld; wxDECLARE_NO_COPY_CLASS(wxDCClipper); }; diff --git a/interface/wx/dc.h b/interface/wx/dc.h index 79490c9c9a..9e363dc90a 100644 --- a/interface/wx/dc.h +++ b/interface/wx/dc.h @@ -764,8 +764,30 @@ public: @remarks Clipping region is given in logical coordinates. + + @param x If non-@NULL, filled in with the logical horizontal coordinate + of the top left corner of the clipping region if the function + returns true or 0 otherwise. + @param y If non-@NULL, filled in with the logical vertical coordinate + of the top left corner of the clipping region if the function + returns true or 0 otherwise. + @param width If non-@NULL, filled in with the width of the clipping + region if the function returns true or the device context width + otherwise. + @param height If non-@NULL, filled in with the height of the clipping + region if the function returns true or the device context height + otherwise. + @return @true if there is a clipping region or @false if there is no + active clipping region (note that this return value is available + only since wxWidgets 3.1.2, this function didn't return anything in + the previous versions). */ - void GetClippingBox(wxCoord *x, wxCoord *y, wxCoord *width, wxCoord *height) const; + bool GetClippingBox(wxCoord *x, wxCoord *y, wxCoord *width, wxCoord *height) const; + + /** + @overload + */ + bool GetClippingBox(wxRect& rect) const; /** Sets the clipping region for this device context to the intersection of diff --git a/src/common/dcbase.cpp b/src/common/dcbase.cpp index 6747d70f6b..a458696c1a 100644 --- a/src/common/dcbase.cpp +++ b/src/common/dcbase.cpp @@ -406,6 +406,61 @@ void wxDCImpl::DoSetClippingRegion(wxCoord x, wxCoord y, wxCoord w, wxCoord h) } } +wxRect wxDCImpl::GetLogicalArea() const +{ + const wxSize size = GetSize(); + return wxRect(DeviceToLogicalX(0), + DeviceToLogicalY(0), + DeviceToLogicalXRel(size.x), + DeviceToLogicalYRel(size.y)); +} + +bool wxDCImpl::DoGetClippingRect(wxRect& rect) const +{ + // Call the old function for compatibility. + DoGetClippingBox(&rect.x, &rect.y, &rect.width, &rect.height); + if ( rect != wxRect(-1, -1, 0, 0) ) + { + // Custom overridden version of DoGetClippingBox() was called, we need + // to check if there is an actual clipping region or not. Normally the + // function is supposed to return the whole DC area (in logical + // coordinates) in this case, but also check that the clipping region + // is not empty because some implementations seem to do this instead. + return !rect.IsEmpty() && rect != GetLogicalArea(); + } + + if ( m_clipping ) + { + rect = wxRect(m_clipX1, + m_clipY1, + m_clipX2 - m_clipX1, + m_clipY2 - m_clipY1); + + return true; + } + else // No active clipping region. + { + rect = GetLogicalArea(); + + return false; + } +} + +void wxDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, + wxCoord *w, wxCoord *h) const +{ + // Dummy implementation just to allow DoGetClippingRect() above to + // determine if this version was called or not. + if ( x ) + *x = -1; + if ( y ) + *y = -1; + if ( w ) + *w = 0; + if ( h ) + *h = 0; +} + // ---------------------------------------------------------------------------- // coordinate conversions and transforms // ----------------------------------------------------------------------------