[PATCH v3 0/2] MR4824: d2d1: Use 24-bit FP precision for triangulate.

[Brendan McGrath (＠redmcg)]

This fixes a rendering issue (and ultimately a crash) in PowerPoint when
compiling with GCC 8.

GCC8 doesn't support the `excess-precision=standard` option under the
`#pragma GCC optimize` directive.

This results in unpredictable floating point rounding leading to errors
when inserting segments (with missing edges and/or triangles).

Using 24-bit precision ensures we don't have any excess precision.

--
  v3: d2d1: Fix double free bug when d2d_geometry_sink_Close fails
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4824

[PATCH v3 1/2] d2d1: Use 24-bit FP precision for triangulate.

[Brendan McGrath]

From: Brendan McGrath <bmcgrath(a)codeweavers.com>

This fixes a rendering issue (and ultimately a crash) in PowerPoint when
compiling with GCC 8.

GCC8 doesn't support the `excess-precision=standard` option under the
`#pragma GCC optimize` directive.

This results in unpredictable floating point rounding leading to errors
when inserting segments (with missing edges and/or triangles).

Using 24-bit precision ensures we don't have any excess precision.
---
 dlls/d2d1/geometry.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/dlls/d2d1/geometry.c b/dlls/d2d1/geometry.c
index d6ee91de005..ffc75d1080a 100644
--- a/dlls/d2d1/geometry.c
+++ b/dlls/d2d1/geometry.c
@@ -16,10 +16,6 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 
-#if defined(__GNUC__) && !defined(__clang__)
-#pragma GCC optimize "O2,no-strict-aliasing,excess-precision=standard"
-#endif
-
 #include "d2d1_private.h"
 #include <float.h>
 
@@ -1539,7 +1535,7 @@ static BOOL d2d_cdt_triangulate(struct d2d_cdt *cdt, size_t start_vertex, size_t
         return TRUE;
     }
 
-    /* More than tree vertices, divide. */
+    /* More than three vertices, divide. */
     cut = vertex_count / 2;
     if (!d2d_cdt_triangulate(cdt, start_vertex, cut, &left_outer, &left_inner))
         return FALSE;
@@ -2289,6 +2285,7 @@ static HRESULT d2d_path_geometry_triangulate(struct d2d_geometry *geometry)
     size_t vertex_count, i, j;
     struct d2d_cdt cdt = {0};
     D2D1_POINT_2F *vertices;
+    unsigned int control_word_x87, mask = 0;
 
     for (i = 0, vertex_count = 0; i < geometry->u.path.figure_count; ++i)
     {
@@ -2339,10 +2336,16 @@ static HRESULT d2d_path_geometry_triangulate(struct d2d_geometry *geometry)
 
     cdt.free_edge = ~0u;
     cdt.vertices = vertices;
+
+    control_word_x87 = _controlfp(0, 0);
+    _controlfp(_PC_24, mask = _MCW_PC);
     if (!d2d_cdt_triangulate(&cdt, 0, vertex_count, &left_edge, &right_edge))
         goto fail;
     if (!d2d_cdt_insert_segments(&cdt, geometry))
         goto fail;
+    _controlfp(control_word_x87, _MCW_PC);
+    mask = 0;
+
     if (!d2d_cdt_generate_faces(&cdt, geometry))
         goto fail;
 
@@ -2354,6 +2357,7 @@ fail:
     geometry->fill.vertex_count = 0;
     free(vertices);
     free(cdt.edges);
+    if (mask) _controlfp(control_word_x87, mask);
     return E_FAIL;
 }
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/4824

[PATCH v3 2/2] d2d1: Fix double free bug when d2d_geometry_sink_Close fails

[Brendan McGrath]

From: Brendan McGrath <bmcgrath(a)codeweavers.com>

geometry->fill.bezier_vertices was being freed on the failed path in
d2d_geometry_sink_Close and then again when the path geometry was
released (in d2d_geometry_cleanup).

By setting it to NULL after freeing it initially, all other calls
to free it are a no-op.
---
 dlls/d2d1/geometry.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/d2d1/geometry.c b/dlls/d2d1/geometry.c
index ffc75d1080a..4cb9c5aff1c 100644
--- a/dlls/d2d1/geometry.c
+++ b/dlls/d2d1/geometry.c
@@ -3239,6 +3239,7 @@ done:
     if (FAILED(hr))
     {
         free(geometry->fill.bezier_vertices);
+        geometry->fill.bezier_vertices = NULL;
         geometry->fill.bezier_vertex_count = 0;
         d2d_path_geometry_free_figures(geometry);
         geometry->u.path.state = D2D_GEOMETRY_STATE_ERROR;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4824

[Brendan Shanks (＠bshanks)]

Should the `_controlfp()` calls be ifdef'd to i386 only? The [MS docs](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/control87-...) say that "On the ARM, ARM64, and x64 platforms, changing the infinity mode or the floating-point precision isn't supported. If the precision control mask is used on the x64 platform, the function raises an assertion, and the invalid parameter handler is invoked, as described in Parameter validation."

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4824#note_57133