[PATCH v3 0/2] MR4824: d2d1: Use 24-bit FP precision for triangulate.
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
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
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
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
participants (3)
-
Brendan McGrath -
Brendan McGrath (@redmcg) -
Brendan Shanks (@bshanks)