vcl/inc/win/salgdi.h | 8 ++++++++
vcl/win/gdi/salprn.cxx | 3 +++
2 files changed, 11 insertions(+)
New commits:
commit a2adb7fa926c2b9ed053c65e130a9da336cc98b0
Author: Jan-Marek Glogowski <glo...@fbihome.de>
AuthorDate: Tue Oct 12 10:33:36 2021 +0200
Commit: Mark Hung <mark...@gmail.com>
CommitDate: Sun Oct 17 15:33:45 2021 +0200
WIN release SalPrinter's SalGraphics borrowed HDC
Regression from commit d27187b158d7e3f92180b1f2ab79b048dc5318a5
("vcl:use unique_ptr<SalWinGraphics> in WinSalPrinter").
Would need something like Rust's Borrowing semantics to prevent.
Ideas on IRC to make this bug more unlikely were some extra HDC +
SalGraphics struct to pass around or something like unique_ptr
reference passing and moving the value into a local copy.
For now just add some additional comments.
Change-Id: I472ee9acb4a4c02177c27ecd1c1277dfc812cadf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/123440
Tested-by: Jenkins
Reviewed-by: Mark Hung <mark...@gmail.com>
diff --git a/vcl/inc/win/salgdi.h b/vcl/inc/win/salgdi.h
index 9555cc158b75..974c58ae81ad 100644
--- a/vcl/inc/win/salgdi.h
+++ b/vcl/inc/win/salgdi.h
@@ -141,6 +141,13 @@ public:
void fill(sal_uInt32 color);
};
+/**
+ * WinSalGraphics never owns the HDC it uses to draw, because the HDC can have
+ * various origins with different ways to correctly free it. And WinSalGraphics
+ * stores all default values (mhDef*) of the HDC, which must be restored when
+ * the HDC changes (setHDC) or the SalGraphics is destructed. So think of the
+ * HDC in terms of Rust's Borrowing semantics.
+ */
class WinSalGraphics : public SalGraphics
{
friend class WinSalGraphicsImpl;
@@ -177,6 +184,7 @@ public:
HFONT ImplDoSetFont(vcl::font::FontSelectPattern const & i_rFont, const
vcl::font::PhysicalFontFace * i_pFontFace, HFONT& o_rOldFont);
HDC getHDC() const { return mhLocalDC; }
+ // NOTE: this doesn't transfer ownership! See class comment.
void setHDC(HDC aNew);
HPALETTE getDefPal() const;
diff --git a/vcl/win/gdi/salprn.cxx b/vcl/win/gdi/salprn.cxx
index 86e1e616a57f..332c7a8dfaae 100644
--- a/vcl/win/gdi/salprn.cxx
+++ b/vcl/win/gdi/salprn.cxx
@@ -1373,6 +1373,9 @@ WinSalPrinter::~WinSalPrinter()
HDC hDC = mhDC;
if ( hDC )
{
+ // explicitly reset(), so the mxGraphics's borrowed HDC defaults are
+ // restored and WinSalGraphics's destructor won't work on a deleted
HDC.
+ mxGraphics.reset();
DeleteDC( hDC );
}
