From 56b8933ede912a7a679157811009f87d1fac6ce7 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Wed, 8 Jan 2025 11:05:01 +0100 Subject: [PATCH 1/6] DWrite: Remove unused variables --- src/win32/cairo-dwrite-font.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/win32/cairo-dwrite-font.cpp b/src/win32/cairo-dwrite-font.cpp index f7d1c4767..ba1695259 100644 --- a/src/win32/cairo-dwrite-font.cpp +++ b/src/win32/cairo-dwrite-font.cpp @@ -1689,7 +1689,6 @@ _dwrite_draw_glyphs_to_gdi_surface_d2d(cairo_win32_surface_t *surface, if (FAILED(hr)) return CAIRO_INT_STATUS_UNSUPPORTED; - float x = 0, y = 0; if (transform) { rt->SetTransform(D2D1::Matrix3x2F(transform->m11, transform->m12, @@ -1722,7 +1721,6 @@ _cairo_dwrite_show_glyphs_on_surface(void *surface, { // TODO: Check font & surface for types. cairo_dwrite_scaled_font_t *dwritesf = reinterpret_cast(scaled_font); - cairo_dwrite_font_face_t *dwriteff = reinterpret_cast(scaled_font->font_face); cairo_win32_surface_t *dst = reinterpret_cast(surface); cairo_int_status_t status; /* We can only handle dwrite fonts */ @@ -1747,10 +1745,6 @@ _cairo_dwrite_show_glyphs_on_surface(void *surface, AutoDWriteGlyphRun run; run.allocate(num_glyphs); - UINT16 *indices = const_cast(run.glyphIndices); - FLOAT *advances = const_cast(run.glyphAdvances); - DWRITE_GLYPH_OFFSET *offsets = const_cast(run.glyphOffsets); - BOOL transform = FALSE; _cairo_dwrite_glyph_run_from_glyphs(glyphs, num_glyphs, dwritesf, &run, &transform); From bdac72997480b9e71e1f341d8e09388703f1bde3 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Wed, 8 Jan 2025 11:20:08 +0100 Subject: [PATCH 2/6] DWrite/GeometryRecorder: Use IFACEMETHOD consistently Fixes the following warnings on CLang: ../cairo/src/win32/cairo-dwrite-font.cpp:869:27: warning: exception specification of overriding function is more lax than base version [-Wmicrosoft-exception-spec] 869 | IFACEMETHODIMP_(void) SetFillMode(D2D1_FILL_MODE fillMode) | ^ D:/msys64/clang64/include/d2d1.h:1491:22: note: overridden virtual function is here 1491 | STDMETHOD_(void, SetFillMode)(D2D1_FILL_MODE fillMode) PURE; | COM objects are usually implemented like that: 1. The class is defined with only method declarations. For that, one should use IFACEMETHOD macros. 2. Then methods are implemented (defined), outside of the class definition. For that, one should use the IFACEMETHODIMP macros If one really wants to provide inline method definitions (that is, inside the class definition), then IFACEMETHOD macros should be used (and not IFACEMETHODIMP, though it's a definition / implementation). --- src/win32/cairo-dwrite-font.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/win32/cairo-dwrite-font.cpp b/src/win32/cairo-dwrite-font.cpp index ba1695259..bcff2d42a 100644 --- a/src/win32/cairo-dwrite-font.cpp +++ b/src/win32/cairo-dwrite-font.cpp @@ -846,7 +846,7 @@ public: , mMatrix(matrix) {} // IUnknown interface - IFACEMETHOD(QueryInterface)(IID const& iid, OUT void** ppObject) + IFACEMETHOD (QueryInterface)(IID const& iid, OUT void** ppObject) { if (iid != __uuidof(IDWriteGeometrySink)) return E_NOINTERFACE; @@ -866,22 +866,22 @@ public: return 1; } - IFACEMETHODIMP_(void) SetFillMode(D2D1_FILL_MODE fillMode) + IFACEMETHOD_(void, SetFillMode)(D2D1_FILL_MODE fillMode) { return; } - STDMETHODIMP Close() + IFACEMETHOD (Close)() { return S_OK; } - IFACEMETHODIMP_(void) SetSegmentFlags(D2D1_PATH_SEGMENT vertexFlags) + IFACEMETHOD_(void, SetSegmentFlags)(D2D1_PATH_SEGMENT vertexFlags) { return; } - IFACEMETHODIMP_(void) BeginFigure( + IFACEMETHOD_(void, BeginFigure)( D2D1_POINT_2F startPoint, D2D1_FIGURE_BEGIN figureBegin) { @@ -896,7 +896,7 @@ public: (void)status; /* squelch warning */ } - IFACEMETHODIMP_(void) EndFigure( + IFACEMETHOD_(void, EndFigure)( D2D1_FIGURE_END figureEnd) { if (figureEnd == D2D1_FIGURE_END_CLOSED) { @@ -907,7 +907,7 @@ public: } } - IFACEMETHODIMP_(void) AddBeziers( + IFACEMETHOD_(void, AddBeziers)( const D2D1_BEZIER_SEGMENT *beziers, UINT beziersCount) { @@ -932,7 +932,7 @@ public: } } - IFACEMETHODIMP_(void) AddLines( + IFACEMETHOD_(void, AddLines)( const D2D1_POINT_2F *points, UINT pointsCount) { From 8107086c6a8d52a2b86352e86b32799885a3b746 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Thu, 16 Jan 2025 11:37:39 +0100 Subject: [PATCH 3/6] DWrite/GeometryRecorder: Add override specifier IFACEMETHOD already adds the __override / __allowed(on_function) SAL annotation (only on Windows SDK, not mingw-w64), which is understood by some code analysis tools [1]. Since we're compiling in C++11 mode, we can add the override specifier, so that the compiler is informed as well. [1] https://devblogs.microsoft.com/oldnewthing/20200911-00/?p=104205 --- src/win32/cairo-dwrite-font.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/win32/cairo-dwrite-font.cpp b/src/win32/cairo-dwrite-font.cpp index bcff2d42a..f20df6de9 100644 --- a/src/win32/cairo-dwrite-font.cpp +++ b/src/win32/cairo-dwrite-font.cpp @@ -846,7 +846,7 @@ public: , mMatrix(matrix) {} // IUnknown interface - IFACEMETHOD (QueryInterface)(IID const& iid, OUT void** ppObject) + IFACEMETHOD (QueryInterface)(IID const& iid, OUT void** ppObject) override { if (iid != __uuidof(IDWriteGeometrySink)) return E_NOINTERFACE; @@ -856,34 +856,34 @@ public: return S_OK; } - IFACEMETHOD_(ULONG, AddRef)() + IFACEMETHOD_(ULONG, AddRef)() override { return 1; } - IFACEMETHOD_(ULONG, Release)() + IFACEMETHOD_(ULONG, Release)() override { return 1; } - IFACEMETHOD_(void, SetFillMode)(D2D1_FILL_MODE fillMode) + IFACEMETHOD_(void, SetFillMode)(D2D1_FILL_MODE fillMode) override { return; } - IFACEMETHOD (Close)() + IFACEMETHOD (Close)() override { return S_OK; } - IFACEMETHOD_(void, SetSegmentFlags)(D2D1_PATH_SEGMENT vertexFlags) + IFACEMETHOD_(void, SetSegmentFlags)(D2D1_PATH_SEGMENT vertexFlags) override { return; } IFACEMETHOD_(void, BeginFigure)( D2D1_POINT_2F startPoint, - D2D1_FIGURE_BEGIN figureBegin) + D2D1_FIGURE_BEGIN figureBegin) override { double x = startPoint.x; double y = startPoint.y; @@ -897,7 +897,7 @@ public: } IFACEMETHOD_(void, EndFigure)( - D2D1_FIGURE_END figureEnd) + D2D1_FIGURE_END figureEnd) override { if (figureEnd == D2D1_FIGURE_END_CLOSED) { cairo_status_t status = _cairo_path_fixed_line_to(mCairoPath, @@ -909,7 +909,7 @@ public: IFACEMETHOD_(void, AddBeziers)( const D2D1_BEZIER_SEGMENT *beziers, - UINT beziersCount) + UINT beziersCount) override { for (unsigned int i = 0; i < beziersCount; i++) { double x1 = beziers[i].point1.x; @@ -934,7 +934,7 @@ public: IFACEMETHOD_(void, AddLines)( const D2D1_POINT_2F *points, - UINT pointsCount) + UINT pointsCount) override { for (unsigned int i = 0; i < pointsCount; i++) { double x = points[i].x; From 59197e779106985fb14625159318b0d48afb9882 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Thu, 16 Jan 2025 11:52:52 +0100 Subject: [PATCH 4/6] DWrite/GeometryRecorder: Add noexcept specifier STDMETHOD / IFACEMETHOD macros already add __declspec(nothrow), but noexcept is better. From MSDN [1]: We recommend that all new code use the noexcept operator rather than __declspec(nothrow). This attribute tells the compiler that the declared function and the functions it calls never throw an exception. However, it does not enforce the directive. In other words, it never causes std::terminate to be invoked, unlike noexcept, or in std:c++17 mode (Visual Studio 2017 version 15.5 and later), throw(). See also [2]: Non-throwing functions are permitted to call potentially-throwing functions. Whenever an exception is thrown and the search for a handler encounters the outermost block of a non-throwing function, the function std::terminate is called: extern void f(); // potentially-throwing void g() noexcept { f(); // valid, even if f throws throw 42; // valid, effectively a call to std::terminate } References: [1] https://learn.microsoft.com/en-us/cpp/cpp/nothrow-cpp?view=msvc-170 [2] https://en.cppreference.com/w/cpp/language/noexcept_spec --- src/win32/cairo-dwrite-font.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/win32/cairo-dwrite-font.cpp b/src/win32/cairo-dwrite-font.cpp index f20df6de9..36d6b5c7b 100644 --- a/src/win32/cairo-dwrite-font.cpp +++ b/src/win32/cairo-dwrite-font.cpp @@ -846,7 +846,7 @@ public: , mMatrix(matrix) {} // IUnknown interface - IFACEMETHOD (QueryInterface)(IID const& iid, OUT void** ppObject) override + IFACEMETHOD (QueryInterface)(IID const& iid, OUT void** ppObject) noexcept override { if (iid != __uuidof(IDWriteGeometrySink)) return E_NOINTERFACE; @@ -856,34 +856,34 @@ public: return S_OK; } - IFACEMETHOD_(ULONG, AddRef)() override + IFACEMETHOD_(ULONG, AddRef)() noexcept override { return 1; } - IFACEMETHOD_(ULONG, Release)() override + IFACEMETHOD_(ULONG, Release)() noexcept override { return 1; } - IFACEMETHOD_(void, SetFillMode)(D2D1_FILL_MODE fillMode) override + IFACEMETHOD_(void, SetFillMode)(D2D1_FILL_MODE fillMode) noexcept override { return; } - IFACEMETHOD (Close)() override + IFACEMETHOD (Close)() noexcept override { return S_OK; } - IFACEMETHOD_(void, SetSegmentFlags)(D2D1_PATH_SEGMENT vertexFlags) override + IFACEMETHOD_(void, SetSegmentFlags)(D2D1_PATH_SEGMENT vertexFlags) noexcept override { return; } IFACEMETHOD_(void, BeginFigure)( D2D1_POINT_2F startPoint, - D2D1_FIGURE_BEGIN figureBegin) override + D2D1_FIGURE_BEGIN figureBegin) noexcept override { double x = startPoint.x; double y = startPoint.y; @@ -897,7 +897,7 @@ public: } IFACEMETHOD_(void, EndFigure)( - D2D1_FIGURE_END figureEnd) override + D2D1_FIGURE_END figureEnd) noexcept override { if (figureEnd == D2D1_FIGURE_END_CLOSED) { cairo_status_t status = _cairo_path_fixed_line_to(mCairoPath, @@ -909,7 +909,7 @@ public: IFACEMETHOD_(void, AddBeziers)( const D2D1_BEZIER_SEGMENT *beziers, - UINT beziersCount) override + UINT beziersCount) noexcept override { for (unsigned int i = 0; i < beziersCount; i++) { double x1 = beziers[i].point1.x; @@ -934,7 +934,7 @@ public: IFACEMETHOD_(void, AddLines)( const D2D1_POINT_2F *points, - UINT pointsCount) override + UINT pointsCount) noexcept override { for (unsigned int i = 0; i < pointsCount; i++) { double x = points[i].x; From 46153a04087d1bef8f2ef3da3cb14e1b97bf4a6c Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Thu, 16 Jan 2025 12:02:27 +0100 Subject: [PATCH 5/6] DWrite/GeometryRecorder: Complete implementation of QueryInterface ...by checking for IUnknown. This makes GeometryRecorder::QueryInterface compliant with the rules of COM. QueryInterface for IUnknown has a special meaning in COM: it's used to check whether two interface pointers refer to the same object. --- src/win32/cairo-dwrite-font.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/win32/cairo-dwrite-font.cpp b/src/win32/cairo-dwrite-font.cpp index 36d6b5c7b..a69adec0b 100644 --- a/src/win32/cairo-dwrite-font.cpp +++ b/src/win32/cairo-dwrite-font.cpp @@ -845,15 +845,18 @@ public: : mCairoPath(aCairoPath) , mMatrix(matrix) {} - // IUnknown interface IFACEMETHOD (QueryInterface)(IID const& iid, OUT void** ppObject) noexcept override { - if (iid != __uuidof(IDWriteGeometrySink)) - return E_NOINTERFACE; + if (iid == __uuidof (IUnknown) || + iid == __uuidof (IDWriteGeometrySink)) + { + AddRef(); + *ppObject = this; + return S_OK; + } - *ppObject = static_cast(this); - - return S_OK; + *ppObject = nullptr; + return E_NOINTERFACE; } IFACEMETHOD_(ULONG, AddRef)() noexcept override From e0287e09f41fc7d063d2e3d707a5e5a5521b6f85 Mon Sep 17 00:00:00 2001 From: Luca Bacci Date: Thu, 16 Jan 2025 12:13:39 +0100 Subject: [PATCH 6/6] DWrite/GeometryRecorder: Add final specifier --- src/win32/cairo-dwrite-font.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/win32/cairo-dwrite-font.cpp b/src/win32/cairo-dwrite-font.cpp index a69adec0b..92d2980cd 100644 --- a/src/win32/cairo-dwrite-font.cpp +++ b/src/win32/cairo-dwrite-font.cpp @@ -838,7 +838,8 @@ _cairo_dwrite_scaled_font_init_glyph_metrics(cairo_dwrite_scaled_font_t *scaled_ * Used to determine the path of the glyphs. */ -class GeometryRecorder : public IDWriteGeometrySink +class GeometryRecorder final + : public IDWriteGeometrySink { public: GeometryRecorder(cairo_path_fixed_t *aCairoPath, const cairo_matrix_t &matrix)