From fe0f9fefe4e77916f09175e1e67b5860a519a83e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 00:26:32 +0200 Subject: [PATCH 1/8] Refactor wxDCClipper ctors to use a common Init() method No changes, this is just a pure refactoring to facilitate the upcoming change which won't have to be done in all these ctors any more. --- include/wx/dc.h | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/include/wx/dc.h b/include/wx/dc.h index 38edf9da5f..cfc57cd1bd 100644 --- a/include/wx/dc.h +++ b/include/wx/dc.h @@ -1437,18 +1437,15 @@ class WXDLLIMPEXP_CORE wxDCClipper public: wxDCClipper(wxDC& dc, const wxRegion& r) : m_dc(dc) { - dc.GetClippingBox(m_oldClipRect); - dc.SetClippingRegion(r.GetBox()); + Init(r.GetBox()); } wxDCClipper(wxDC& dc, const wxRect& r) : m_dc(dc) { - dc.GetClippingBox(m_oldClipRect); - dc.SetClippingRegion(r.x, r.y, r.width, r.height); + Init(r); } wxDCClipper(wxDC& dc, wxCoord x, wxCoord y, wxCoord w, wxCoord h) : m_dc(dc) { - dc.GetClippingBox(m_oldClipRect); - dc.SetClippingRegion(x, y, w, h); + Init(wxRect(x, y, w, h)); } ~wxDCClipper() @@ -1459,6 +1456,13 @@ public: } private: + // Common part of all ctors. + void Init(const wxRect& r) + { + m_dc.GetClippingBox(m_oldClipRect); + m_dc.SetClippingRegion(r); + } + wxDC& m_dc; wxRect m_oldClipRect; From 119dce5eb8db37b60624a7b76e06e42b2b3e555c Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 10:21:59 +0200 Subject: [PATCH 2/8] Fix check for existing clipping region in wxDCClipper wxDC::GetClippingBox() is actually supposed to return a rectangle equal to the total wxDC area and not an empty rectangle if there is no clipping box at all, so avoid restoring the old clipping region unnecessarily in this case too: even if it should be harmless, it's still unnecessarily inefficient and, in practice, this is not really harmless neither as wxPdfDC (from the third party wxPdfDocument library) doesn't handle having a clipping region set when adding a new page correctly and so using wxDCClipper broke PDF generation. This fixes another fallout from 2a8c290e0de5e657f22a0c13817df65688b01345 See #13834. --- include/wx/dc.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/include/wx/dc.h b/include/wx/dc.h index cfc57cd1bd..76b5df8058 100644 --- a/include/wx/dc.h +++ b/include/wx/dc.h @@ -1459,7 +1459,14 @@ 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_dc.SetClippingRegion(r); } From 350867939a2560e8614edb5fe797cc9fbb12f1f6 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 14:39:11 +0200 Subject: [PATCH 3/8] 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 // ---------------------------------------------------------------------------- From 6a442d272347c647062ebd3cb1dcf4cae04248f8 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 16:53:52 +0200 Subject: [PATCH 4/8] Remove unnecessary clipping virtual methods from wxSVGFileDCImpl These methods seem to be useless as they're never called, only the corresponding DoXXX() methods are used by wxDC. --- include/wx/dcsvg.h | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/include/wx/dcsvg.h b/include/wx/dcsvg.h index 66d1cb42ea..488d0c3057 100644 --- a/include/wx/dcsvg.h +++ b/include/wx/dcsvg.h @@ -86,23 +86,11 @@ public: virtual wxCoord GetCharHeight() const wxOVERRIDE; virtual wxCoord GetCharWidth() const wxOVERRIDE; - virtual void SetClippingRegion(wxCoord x, wxCoord y, - wxCoord w, wxCoord h) - { - DoSetClippingRegion(x, y, w, h); - } - virtual void SetPalette(const wxPalette& WXUNUSED(palette)) wxOVERRIDE { wxFAIL_MSG(wxT("wxSVGFILEDC::SetPalette not implemented")); } - virtual void GetClippingBox(wxCoord *x, wxCoord *y, - wxCoord *w, wxCoord *h) - { - DoGetClippingBox(x, y, w, h); - } - virtual void SetLogicalFunction(wxRasterOperationMode WXUNUSED(function)) wxOVERRIDE { wxFAIL_MSG(wxT("wxSVGFILEDC::SetLogicalFunction Call not implemented")); From 070336470f27e046bc29f00688cea4833a6b218b Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 16:54:50 +0200 Subject: [PATCH 5/8] Make wxSVGFileDC::GetClippingBox() actually work wxSVGFileDCImpl class uses the default, i.e. inherited from wxDCImpl, implementation of this method, but for it to work, the clipping box coordinates stored in wxDCImpl need to be updated when the clipping region changes or is destroyed and this wasn't done before. Fix this now and add a unit test verifying that this indeed works. --- src/common/dcsvg.cpp | 6 ++++++ tests/graphics/clippingbox.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/common/dcsvg.cpp b/src/common/dcsvg.cpp index ca44afcac3..9eb2f4f0fb 100644 --- a/src/common/dcsvg.cpp +++ b/src/common/dcsvg.cpp @@ -934,6 +934,9 @@ void wxSVGFileDCImpl::DoSetClippingRegion(int x, int y, int width, int height) m_clipUniqueId++; m_clipNestingLevel++; + + // Update the base class m_clip[XY][12] fields too. + wxDCImpl::DoSetClippingRegion(x, y, width, height); } void wxSVGFileDCImpl::DestroyClippingRegion() @@ -958,6 +961,9 @@ void wxSVGFileDCImpl::DestroyClippingRegion() DoStartNewGraphics(); m_clipUniqueId = 0; + + // Also update the base class clipping region information. + wxDCImpl::DestroyClippingRegion(); } void wxSVGFileDCImpl::DoGetTextExtent(const wxString& string, wxCoord *w, wxCoord *h, wxCoord *descent, wxCoord *externalLeading, const wxFont *font) const diff --git a/tests/graphics/clippingbox.cpp b/tests/graphics/clippingbox.cpp index 6068660f91..42eed74701 100644 --- a/tests/graphics/clippingbox.cpp +++ b/tests/graphics/clippingbox.cpp @@ -21,7 +21,9 @@ #if wxUSE_GRAPHICS_CONTEXT #include "wx/dcgraph.h" #endif // wxUSE_GRAPHICS_CONTEXT +#include "wx/dcsvg.h" +#include "testfile.h" // ---------------------------------------------------------------------------- // test class @@ -2122,3 +2124,29 @@ void ClippingBoxTestCaseGCBase::RegionsAndPushPopState() } #endif // wxUSE_GRAPHICS_CONTEXT + +#if wxUSE_SVG + +// We can't reuse the existing tests for wxSVGFileDC as we can't check its +// output, but we can still at least check the behaviour of GetClippingBox(). +TEST_CASE("ClippingBoxTestCaseSVGDC", "[clip][svgdc]") +{ + TestFile tf; + wxSVGFileDC dc(tf.GetName(), s_dcSize.x, s_dcSize.y); + + wxRect rect; + dc.GetClippingBox(rect); + CHECK( rect == wxRect(s_dcSize) ); + + const wxRect rectClip(10, 20, 80, 75); + dc.SetClippingRegion(rectClip); + + dc.GetClippingBox(rect); + CHECK( rect == rectClip ); + + dc.DestroyClippingRegion(); + dc.GetClippingBox(rect); + CHECK( rect == wxRect(s_dcSize) ); +} + +#endif // wxUSE_SVG From 110ace680be6cc91c40366391873658d63fd7481 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 22:53:53 +0200 Subject: [PATCH 6/8] Remove unnecessary calls from wxDCImpl ctor There is no need to call neither ResetBoundingBox() nor ResetClipping() when the variables they reset had just been initialized to the same values in the ctor initializer list. --- src/common/dcbase.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/common/dcbase.cpp b/src/common/dcbase.cpp index a458696c1a..0a0e2d2f1d 100644 --- a/src/common/dcbase.cpp +++ b/src/common/dcbase.cpp @@ -353,9 +353,6 @@ wxDCImpl::wxDCImpl( wxDC *owner ) (double)wxGetDisplaySizeMM().GetWidth(); m_mm_to_pix_y = (double)wxGetDisplaySize().GetHeight() / (double)wxGetDisplaySizeMM().GetHeight(); - - ResetBoundingBox(); - ResetClipping(); } wxDCImpl::~wxDCImpl() From 43ce00b5bd2ba645eb0788e35abc8858d9e156e7 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 22:52:23 +0200 Subject: [PATCH 7/8] Call base class version from overridden DestroyClippingRegion() No real changes, just call wxDCImpl::DestroyClippingRegion() from the overridden versions in the derived classes instead of calling ResetClipping(): this makes the code more clear as it follows the usual pattern of the derived class doing something first and then forwarding to the base class. Also, as ResetClipping() is not really useful, add a comment documenting that it shouldn't be used in the new code. --- include/wx/dc.h | 5 ++++- src/common/dcgraph.cpp | 2 +- src/dfb/dc.cpp | 2 +- src/qt/dc.cpp | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/include/wx/dc.h b/include/wx/dc.h index 5749d578b3..55b1129218 100644 --- a/include/wx/dc.h +++ b/include/wx/dc.h @@ -666,7 +666,10 @@ private: wxDC *m_owner; protected: - // unset clipping variables (after clipping region was destroyed) + // This method exists for backwards compatibility only (while it's not + // documented, there are derived classes using it outside wxWidgets + // itself), don't use it in any new code and just call wxDCImpl version of + // DestroyClippingRegion() to reset the clipping information instead. void ResetClipping() { m_clipping = false; diff --git a/src/common/dcgraph.cpp b/src/common/dcgraph.cpp index 6eafed2622..8a1441697d 100644 --- a/src/common/dcgraph.cpp +++ b/src/common/dcgraph.cpp @@ -376,7 +376,7 @@ void wxGCDCImpl::DestroyClippingRegion() m_graphicContext->SetPen( m_pen ); m_graphicContext->SetBrush( m_brush ); - ResetClipping(); + wxDCImpl::DestroyClippingRegion(); m_isClipBoxValid = false; } diff --git a/src/dfb/dc.cpp b/src/dfb/dc.cpp index a8458e58ea..d6141a7602 100644 --- a/src/dfb/dc.cpp +++ b/src/dfb/dc.cpp @@ -115,7 +115,7 @@ void wxDFBDCImpl::DestroyClippingRegion() m_surface->SetClip(NULL); - ResetClipping(); + wxDCImpl::DestroyClippingRegion(); } // --------------------------------------------------------------------------- diff --git a/src/qt/dc.cpp b/src/qt/dc.cpp index 98c21fa739..38e4e0af21 100644 --- a/src/qt/dc.cpp +++ b/src/qt/dc.cpp @@ -454,7 +454,7 @@ void wxQtDCImpl::DoSetDeviceClippingRegion(const wxRegion& region) void wxQtDCImpl::DestroyClippingRegion() { - ResetClipping(); + wxDCImpl::DestroyClippingRegion(); m_clippingRegion->Clear(); if (m_qtPainter->isActive()) From c5530b1abf36c652c29611c2ccb5a9d01305a3c1 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Mon, 18 Jun 2018 23:57:05 +0200 Subject: [PATCH 8/8] Switch to using DoGetClippingRect() instead of DoGetClippingBox() The new method returns a boolean flag which indicates whether there is any clipping region or not and so is preferable to using the old one and checking its return value to determine this, which can't be done reliably. --- include/wx/dc.h | 8 ++++++-- include/wx/dcgraph.h | 3 +-- include/wx/gtk/dcclient.h | 2 +- include/wx/msw/dc.h | 3 +-- src/common/dcbase.cpp | 16 ++++++++++------ src/common/dcgraph.cpp | 24 ++++++++++++++---------- src/gtk/dcclient.cpp | 13 +++---------- src/msw/dc.cpp | 27 +++++++++++++++++---------- src/msw/dcclient.cpp | 2 +- 9 files changed, 54 insertions(+), 44 deletions(-) diff --git a/include/wx/dc.h b/include/wx/dc.h index 55b1129218..a34a73b698 100644 --- a/include/wx/dc.h +++ b/include/wx/dc.h @@ -453,11 +453,15 @@ public: // up to date. virtual bool DoGetClippingRect(wxRect& rect) const; +#if WXWIN_COMPATIBILITY_3_0 // 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; + wxDEPRECATED_BUT_USED_INTERNALLY( + virtual void DoGetClippingBox(wxCoord *x, wxCoord *y, + wxCoord *w, wxCoord *h) const + ); +#endif // WXWIN_COMPATIBILITY_3_0 virtual void DestroyClippingRegion() { ResetClipping(); } diff --git a/include/wx/dcgraph.h b/include/wx/dcgraph.h index f9ac7461dd..9a11a19ce4 100644 --- a/include/wx/dcgraph.h +++ b/include/wx/dcgraph.h @@ -195,8 +195,7 @@ public: virtual void DoSetDeviceClippingRegion(const wxRegion& region) wxOVERRIDE; virtual void DoSetClippingRegion(wxCoord x, wxCoord y, wxCoord width, wxCoord height) wxOVERRIDE; - virtual void DoGetClippingBox(wxCoord *x, wxCoord *y, - wxCoord *w, wxCoord *h) const wxOVERRIDE; + virtual bool DoGetClippingRect(wxRect& rect) const wxOVERRIDE; virtual void DoGetTextExtent(const wxString& string, wxCoord *x, wxCoord *y, diff --git a/include/wx/gtk/dcclient.h b/include/wx/gtk/dcclient.h index 7911b74274..a633faccda 100644 --- a/include/wx/gtk/dcclient.h +++ b/include/wx/gtk/dcclient.h @@ -71,7 +71,7 @@ public: virtual bool DoGetPartialTextExtents(const wxString& text, wxArrayInt& widths) const wxOVERRIDE; virtual void DoSetClippingRegion( wxCoord x, wxCoord y, wxCoord width, wxCoord height ) wxOVERRIDE; virtual void DoSetDeviceClippingRegion( const wxRegion ®ion ) wxOVERRIDE; - virtual void DoGetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h) const wxOVERRIDE; + virtual bool DoGetClippingRect(wxRect& rect) const wxOVERRIDE; virtual wxCoord GetCharWidth() const wxOVERRIDE; virtual wxCoord GetCharHeight() const wxOVERRIDE; diff --git a/include/wx/msw/dc.h b/include/wx/msw/dc.h index 269b81cff2..7e16a138b2 100644 --- a/include/wx/msw/dc.h +++ b/include/wx/msw/dc.h @@ -243,8 +243,7 @@ public: virtual void DoSetClippingRegion(wxCoord x, wxCoord y, wxCoord width, wxCoord height) wxOVERRIDE; virtual void DoSetDeviceClippingRegion(const wxRegion& region) wxOVERRIDE; - virtual void DoGetClippingBox(wxCoord *x, wxCoord *y, - wxCoord *w, wxCoord *h) const wxOVERRIDE; + virtual bool DoGetClippingRect(wxRect& rect) const wxOVERRIDE; virtual void DoGetSizeMM(int* width, int* height) const wxOVERRIDE; diff --git a/src/common/dcbase.cpp b/src/common/dcbase.cpp index 0a0e2d2f1d..1363984b7e 100644 --- a/src/common/dcbase.cpp +++ b/src/common/dcbase.cpp @@ -414,6 +414,7 @@ wxRect wxDCImpl::GetLogicalArea() const bool wxDCImpl::DoGetClippingRect(wxRect& rect) const { +#if WXWIN_COMPATIBILITY_3_0 // Call the old function for compatibility. DoGetClippingBox(&rect.x, &rect.y, &rect.width, &rect.height); if ( rect != wxRect(-1, -1, 0, 0) ) @@ -425,6 +426,7 @@ bool wxDCImpl::DoGetClippingRect(wxRect& rect) const // is not empty because some implementations seem to do this instead. return !rect.IsEmpty() && rect != GetLogicalArea(); } +#endif // WXWIN_COMPATIBILITY_3_0 if ( m_clipping ) { @@ -443,6 +445,7 @@ bool wxDCImpl::DoGetClippingRect(wxRect& rect) const } } +#if WXWIN_COMPATIBILITY_3_0 void wxDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h) const { @@ -457,6 +460,7 @@ void wxDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, if ( h ) *h = 0; } +#endif // WXWIN_COMPATIBILITY_3_0 // ---------------------------------------------------------------------------- // coordinate conversions and transforms @@ -1370,12 +1374,12 @@ void wxDC::GetDeviceOrigin(long *x, long *y) const void wxDC::GetClippingBox(long *x, long *y, long *w, long *h) const { - wxCoord xx,yy,ww,hh; - m_pimpl->DoGetClippingBox(&xx, &yy, &ww, &hh); - if (x) *x = xx; - if (y) *y = yy; - if (w) *w = ww; - if (h) *h = hh; + wxRect r; + m_pimpl->DoGetClippingRect(r); + if (x) *x = r.x; + if (y) *y = r.y; + if (w) *w = r.width; + if (h) *h = r.height; } void wxDC::DrawObject(wxDrawObject* drawobject) diff --git a/src/common/dcgraph.cpp b/src/common/dcgraph.cpp index 8a1441697d..d70113dfcc 100644 --- a/src/common/dcgraph.cpp +++ b/src/common/dcgraph.cpp @@ -282,6 +282,17 @@ void wxGCDCImpl::UpdateClipBox() double x, y, w, h; m_graphicContext->GetClipBox(&x, &y, &w, &h); + // We shouldn't reset m_clipping if the clipping region that we set happens + // to be empty (e.g. because its intersection with the previous clipping + // region was empty), but we should set it to true if we do have a valid + // clipping region and it was false which may happen if the clipping region + // set from the outside of wxWidgets code. + if ( !m_clipping ) + { + if ( w != 0. && h != 0. ) + m_clipping = true; + } + m_clipX1 = wxRound(x); m_clipY1 = wxRound(y); m_clipX2 = wxRound(x+w); @@ -289,9 +300,9 @@ void wxGCDCImpl::UpdateClipBox() m_isClipBoxValid = true; } -void wxGCDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h) const +bool wxGCDCImpl::DoGetClippingRect(wxRect& rect) const { - wxCHECK_RET( IsOk(), wxS("wxGCDC::DoGetClippingRegion - invalid GC") ); + wxCHECK_MSG( IsOk(), false, wxS("wxGCDC::DoGetClippingRegion - invalid GC") ); // Check if we should retrieve the clipping region possibly not set // by SetClippingRegion() but modified by application: this can // happen when we're associated with an existing graphics context using @@ -303,14 +314,7 @@ void wxGCDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h self->UpdateClipBox(); } - if ( x ) - *x = m_clipX1; - if ( y ) - *y = m_clipY1; - if ( w ) - *w = m_clipX2 - m_clipX1; - if ( h ) - *h = m_clipY2 - m_clipY1; + return wxDCImpl::DoGetClippingRect(rect); } void wxGCDCImpl::DoSetClippingRegion( wxCoord x, wxCoord y, wxCoord w, wxCoord h ) diff --git a/src/gtk/dcclient.cpp b/src/gtk/dcclient.cpp index 0f29c219f5..9e98f62b4d 100644 --- a/src/gtk/dcclient.cpp +++ b/src/gtk/dcclient.cpp @@ -1914,9 +1914,9 @@ void wxWindowDCImpl::UpdateClipBox() m_isClipBoxValid = true; } -void wxWindowDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h) const +bool wxWindowDCImpl::DoGetClippingRect(wxRect& rect) const { - wxCHECK_RET( IsOk(), wxS("invalid window dc") ); + wxCHECK_MSG( IsOk(), false, wxS("invalid window dc") ); // Check if we should try to retrieve the clipping region possibly not set // by our SetClippingRegion() but preset or modified by application: this @@ -1928,14 +1928,7 @@ void wxWindowDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoor self->UpdateClipBox(); } - if ( x ) - *x = m_clipX1; - if ( y ) - *y = m_clipY1; - if ( w ) - *w = m_clipX2 - m_clipX1; - if ( h ) - *h = m_clipY2 - m_clipY1; + return wxGTKDCImpl::DoGetClippingRect(rect); } void wxWindowDCImpl::DoSetClippingRegion( wxCoord x, wxCoord y, wxCoord width, wxCoord height ) diff --git a/src/msw/dc.cpp b/src/msw/dc.cpp index 404c86735b..41ada622d9 100644 --- a/src/msw/dc.cpp +++ b/src/msw/dc.cpp @@ -574,8 +574,7 @@ void wxMSWDCImpl::UpdateClipBox() m_isClipBoxValid = true; } -void -wxMSWDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h) const +bool wxMSWDCImpl::DoGetClippingRect(wxRect& rect) const { // check if we should try to retrieve the clipping region possibly not set // by our SetClippingRegion() but preset or modified by Windows: this @@ -589,14 +588,22 @@ wxMSWDCImpl::DoGetClippingBox(wxCoord *x, wxCoord *y, wxCoord *w, wxCoord *h) co self->UpdateClipBox(); } - if ( x ) - *x = m_clipX1; - if ( y ) - *y = m_clipY1; - if ( w ) - *w = m_clipX2 - m_clipX1; - if ( h ) - *h = m_clipY2 - m_clipY1; + // Unfortunately we can't just call wxDCImpl::DoGetClippingRect() here + // because it wouldn't return the correct result if there is no clipping + // region and this DC has a world transform applied to it, as the base + // class version doesn't know anything about world transforms and wouldn't + // apply it to GetLogicalArea() that it returns in this case. + // + // We could solve this by overriding GetLogicalArea() in wxMSW and using + // DPtoLP() to perform the conversion correctly ourselves, but it's even + // simpler to just use the value returned by our UpdateClipBox() which is + // already correct in any case, so just do this instead. + rect = wxRect(m_clipX1, + m_clipY1, + m_clipX2 - m_clipX1, + m_clipY2 - m_clipY1); + + return m_clipping; } // common part of DoSetClippingRegion() and DoSetDeviceClippingRegion() diff --git a/src/msw/dcclient.cpp b/src/msw/dcclient.cpp index 511256f4a5..ae8cbeaec0 100644 --- a/src/msw/dcclient.cpp +++ b/src/msw/dcclient.cpp @@ -302,7 +302,7 @@ wxPaintDCImpl::wxPaintDCImpl( wxDC *owner, wxWindow *window ) : InitDC(); // the HDC can have a clipping box (which we didn't set), make sure our - // DoGetClippingBox() checks for it + // DoGetClippingRect() checks for it m_clipping = true; }