From e9c9d1d0a04acc566d93f91f50688ce2ff1ba6f3 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sun, 3 Nov 2024 02:44:02 +0100 Subject: [PATCH 01/17] filechooser: Use g_clear_object() on potentially NULL objects We can reach here in early exit paths when the shortcut_files have not been initialized yet. --- gtk/gtkfilechoosernativewin32.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/gtk/gtkfilechoosernativewin32.c b/gtk/gtkfilechoosernativewin32.c index bfa4dadcf0..01164d357b 100644 --- a/gtk/gtkfilechoosernativewin32.c +++ b/gtk/gtkfilechoosernativewin32.c @@ -333,10 +333,9 @@ filechooser_win32_thread_data_free (FilechooserWin32ThreadData *data) g_array_free (data->choices_selections, TRUE); data->choices_selections = NULL; } - g_object_unref (data->shortcut_files); + g_clear_object (&data->shortcut_files); g_slist_free_full (data->files, g_object_unref); - if (data->self) - g_object_unref (data->self); + g_clear_object (&data->self); g_free (data->accept_label); g_free (data->cancel_label); g_free (data->title); From 70c327993b782b51424fde33b8157c760ec2168c Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Thu, 31 Oct 2024 13:28:42 +0100 Subject: [PATCH 02/17] win32: Remove unused macro --- gdk/win32/gdkprivate-win32.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/gdk/win32/gdkprivate-win32.h b/gdk/win32/gdkprivate-win32.h index c99a05c2e9..4bc1a99612 100644 --- a/gdk/win32/gdkprivate-win32.h +++ b/gdk/win32/gdkprivate-win32.h @@ -53,8 +53,6 @@ #define GDK_DEBUG_EVENTS_OR_INPUT (GDK_DEBUG_EVENTS|GDK_DEBUG_INPUT) #define GDK_DEBUG_MISC_OR_EVENTS (GDK_DEBUG_MISC|GDK_DEBUG_EVENTS) -GdkWin32Screen *GDK_SURFACE_SCREEN(GObject *win); - /* Use this for hWndInsertAfter (2nd argument to SetWindowPos()) if * SWP_NOZORDER flag is used. Otherwise it's unobvious why a particular * argument is used. Using NULL is misleading, because From 5822989e82cae06ef557812e4372352031dbc65e Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 30 Oct 2024 01:59:36 +0100 Subject: [PATCH 03/17] gdk: Add missing G_END_DECLS --- gdk/gdkdebugprivate.h | 1 + 1 file changed, 1 insertion(+) diff --git a/gdk/gdkdebugprivate.h b/gdk/gdkdebugprivate.h index 5742d737a8..540c017145 100644 --- a/gdk/gdkdebugprivate.h +++ b/gdk/gdkdebugprivate.h @@ -120,3 +120,4 @@ guint gdk_parse_debug_var (const char *variable, const GdkDebugKey *keys, guint nkeys); +G_END_DECLS From 566c484317b54bec55b9a301101f5c2f6b147589 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 30 Oct 2024 01:59:53 +0100 Subject: [PATCH 04/17] cicpparams: Guard with BEGIN/END_DECLS I want to include this header from DirectX C++ code. --- gdk/gdkcicpparamsprivate.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gdk/gdkcicpparamsprivate.h b/gdk/gdkcicpparamsprivate.h index bd9bbb3817..e5381242e8 100644 --- a/gdk/gdkcicpparamsprivate.h +++ b/gdk/gdkcicpparamsprivate.h @@ -2,6 +2,8 @@ #include "gdkcicpparams.h" +G_BEGIN_DECLS + typedef struct _GdkCicp GdkCicp; struct _GdkCicp @@ -82,3 +84,5 @@ gdk_cicp_equivalent (const GdkCicp *p1, const GdkCicp * gdk_cicp_params_get_cicp (GdkCicpParams *self); GdkCicpParams * gdk_cicp_params_new_for_cicp (const GdkCicp *cicp); + +G_END_DECLS From 8b1dff5b94c4c029785e95eb74b6ea137333012e Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Fri, 1 Nov 2024 09:01:30 +0100 Subject: [PATCH 05/17] gstsink: Do not advertise dmabufs with no formats We return an empty format list when dmabufs aren't supported, not NULL. And the sink was treating the empty format list by setting no fourccs on the caps, which GStreamer conveniently interpreted as "any", not as "none". --- modules/media/gtkgstsink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/media/gtkgstsink.c b/modules/media/gtkgstsink.c index 4cd19d7127..7617aa0d93 100644 --- a/modules/media/gtkgstsink.c +++ b/modules/media/gtkgstsink.c @@ -219,7 +219,7 @@ gtk_gst_sink_get_caps (GstBaseSink *bsink, { GdkDmabufFormats *formats = gdk_display_get_dmabuf_formats (self->gdk_display); - if (formats) + if (formats && gdk_dmabuf_formats_get_n_formats (formats) > 0) { tmp = gst_caps_from_string (DMABUF_TEXTURE_CAPS); add_drm_formats_and_modifiers (tmp, formats); From 7085a58f01d4c5ec882069a804fc5fa221018518 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Thu, 31 Oct 2024 22:57:42 +0100 Subject: [PATCH 06/17] gstreamer: Identify GL memory by looking at the memory Instead of looking at availability of a GL context, check the memory's type. That's what we do everywhere else and GL is not special. --- modules/media/gtkgstsink.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/modules/media/gtkgstsink.c b/modules/media/gtkgstsink.c index 7617aa0d93..79533773c8 100644 --- a/modules/media/gtkgstsink.c +++ b/modules/media/gtkgstsink.c @@ -415,6 +415,7 @@ gtk_gst_sink_texture_from_buffer (GtkGstSink *self, graphene_rect_t *viewport) { GstVideoFrame *frame = g_new (GstVideoFrame, 1); + GstMemory *mem; GdkTexture *texture; viewport->origin.x = 0; @@ -422,7 +423,9 @@ gtk_gst_sink_texture_from_buffer (GtkGstSink *self, viewport->size.width = GST_VIDEO_INFO_WIDTH (&self->v_info); viewport->size.height = GST_VIDEO_INFO_HEIGHT (&self->v_info); - if (gst_is_dmabuf_memory (gst_buffer_peek_memory (buffer, 0))) + mem = gst_buffer_peek_memory (buffer, 0); + + if (gst_is_dmabuf_memory (mem)) { GdkDmabufTextureBuilder *builder = NULL; const GstVideoMeta *vmeta = gst_buffer_get_video_meta (buffer); @@ -446,7 +449,6 @@ gtk_gst_sink_texture_from_buffer (GtkGstSink *self, for (i = 0; i < vmeta->n_planes; i++) { - GstMemory *mem; guint mem_idx, length; gsize skip; @@ -480,7 +482,7 @@ gtk_gst_sink_texture_from_buffer (GtkGstSink *self, *pixel_aspect_ratio = ((double) GST_VIDEO_INFO_PAR_N (&self->v_info) / (double) GST_VIDEO_INFO_PAR_D (&self->v_info)); } - else if (self->gdk_context && + else if (gst_is_gl_memory (mem) && gst_video_frame_map (frame, &self->v_info, buffer, GST_MAP_READ | GST_MAP_GL)) { GstGLSyncMeta *sync_meta; From 0a6c4711bcef75b30e1af4301707191cbc10bd35 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sun, 3 Nov 2024 12:14:49 +0100 Subject: [PATCH 07/17] gstreamer: Fix small memleak --- modules/media/gtkgstsink.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/media/gtkgstsink.c b/modules/media/gtkgstsink.c index 79533773c8..293e858353 100644 --- a/modules/media/gtkgstsink.c +++ b/modules/media/gtkgstsink.c @@ -477,7 +477,10 @@ gtk_gst_sink_texture_from_buffer (GtkGstSink *self, g_object_unref (builder); if (!texture) - GST_ERROR_OBJECT (self, "Failed to create dmabuf texture: %s", error->message); + { + GST_ERROR_OBJECT (self, "Failed to create dmabuf texture: %s", error->message); + g_error_free (error); + } *pixel_aspect_ratio = ((double) GST_VIDEO_INFO_PAR_N (&self->v_info) / (double) GST_VIDEO_INFO_PAR_D (&self->v_info)); From 6cd98bae3beb76c107a3126cd0091f3b5b167b4a Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 4 Nov 2024 20:53:46 +0100 Subject: [PATCH 08/17] testsuite: Add --strip-trailing-cr to diff call On Windows, git defaults to maintaining line endings, which means it changed \n to \r\n on all files it identifies as text. And that includes our test output. Luckily diff(1) has an option to undo that. And since we do not care about line endings in those tests, we can just use it. --- testsuite/gsk/node-parser.c | 2 +- testsuite/testutils.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testsuite/gsk/node-parser.c b/testsuite/gsk/node-parser.c index bf97cca2ca..99928ff441 100644 --- a/testsuite/gsk/node-parser.c +++ b/testsuite/gsk/node-parser.c @@ -75,7 +75,7 @@ diff_with_file (const char *file1, process = g_subprocess_new (G_SUBPROCESS_FLAGS_STDIN_PIPE | G_SUBPROCESS_FLAGS_STDOUT_PIPE, error, - "diff", "-u", file1, "-", NULL); + "diff", "--strip-trailing-cr", "-u", file1, "-", NULL); if (process == NULL) return NULL; diff --git a/testsuite/testutils.c b/testsuite/testutils.c index 6e8ba8e940..ce9f474122 100644 --- a/testsuite/testutils.c +++ b/testsuite/testutils.c @@ -42,7 +42,7 @@ diff_with_file (const char *file1, diff_cmd = g_find_program_in_path ("diff"); if (diff_cmd) { - const char *command[] = { diff_cmd, "-u", file1, NULL, NULL }; + const char *command[] = { diff_cmd, "--strip-trailing-cr", "-u", file1, NULL, NULL }; if (len < 0) len = strlen (text); From dd93aa9f50811505724655c939b533a0f49e0c8c Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 4 Nov 2024 21:32:08 +0100 Subject: [PATCH 09/17] testsuite: Rewrite diff_with_file() use the modern version using GSubprocess that already exists in node-parser. Also change from one function to two - so tests can diff GBytes and strings, depending on which they prefer. --- testsuite/css/change/test-css-change.c | 2 +- testsuite/css/nodes/test-css-nodes.c | 2 +- testsuite/css/parser/test-css-parser.c | 4 +- testsuite/css/style/test-css-style.c | 2 +- testsuite/gtk/composetable.c | 2 +- testsuite/gtk/test-focus-chain.c | 2 +- testsuite/testutils.c | 123 +++++++++++++++++-------- testsuite/testutils.h | 11 ++- 8 files changed, 100 insertions(+), 48 deletions(-) diff --git a/testsuite/css/change/test-css-change.c b/testsuite/css/change/test-css-change.c index 93f4df2236..2c896d6817 100644 --- a/testsuite/css/change/test-css-change.c +++ b/testsuite/css/change/test-css-change.c @@ -111,7 +111,7 @@ load_ui_file (GFile *file, gboolean generate) reference_file = test_get_other_file (ui_file, ".nodes"); - diff = diff_with_file (reference_file, output, -1, &error); + diff = diff_string_with_file (reference_file, output, -1, &error); g_assert_no_error (error); if (diff && diff[0]) diff --git a/testsuite/css/nodes/test-css-nodes.c b/testsuite/css/nodes/test-css-nodes.c index 67bd2429aa..89a8fb41eb 100644 --- a/testsuite/css/nodes/test-css-nodes.c +++ b/testsuite/css/nodes/test-css-nodes.c @@ -99,7 +99,7 @@ load_ui_file (GFile *file, gboolean generate) reference_file = test_get_reference_file (ui_file); - diff = diff_with_file (reference_file, output, -1, &error); + diff = diff_string_with_file (reference_file, output, -1, &error); g_assert_no_error (error); if (diff && diff[0]) diff --git a/testsuite/css/parser/test-css-parser.c b/testsuite/css/parser/test-css-parser.c index 3f6e9ed04b..eb28accaed 100644 --- a/testsuite/css/parser/test-css-parser.c +++ b/testsuite/css/parser/test-css-parser.c @@ -146,7 +146,7 @@ parse_css_file (GFile *file, gboolean generate) reference_file = test_get_reference_file (css_file); - diff = diff_with_file (reference_file, css, -1, &error); + diff = diff_string_with_file (reference_file, css, -1, &error); g_assert_no_error (error); if (diff && diff[0]) @@ -161,7 +161,7 @@ parse_css_file (GFile *file, gboolean generate) if (errors_file) { - diff = diff_with_file (errors_file, errors->str, errors->len, &error); + diff = diff_string_with_file (errors_file, errors->str, errors->len, &error); g_assert_no_error (error); if (diff && diff[0]) diff --git a/testsuite/css/style/test-css-style.c b/testsuite/css/style/test-css-style.c index 20e234ce8f..fff7e6adb5 100644 --- a/testsuite/css/style/test-css-style.c +++ b/testsuite/css/style/test-css-style.c @@ -129,7 +129,7 @@ load_ui_file (GFile *file, gboolean generate) reference_file = test_get_other_file (ui_file, ".nodes"); g_assert_nonnull (reference_file); - diff = diff_with_file (reference_file, output, -1, &error); + diff = diff_string_with_file (reference_file, output, -1, &error); g_assert_no_error (error); if (diff && diff[0]) diff --git a/testsuite/gtk/composetable.c b/testsuite/gtk/composetable.c index 85a27b0fa7..8c9d282acd 100644 --- a/testsuite/gtk/composetable.c +++ b/testsuite/gtk/composetable.c @@ -107,7 +107,7 @@ compose_table_compare (gconstpointer data) table = gtk_compose_table_parse (file, NULL); output = gtk_compose_table_print (table); - diff = diff_with_file (expected, output, -1, &error); + diff = diff_string_with_file (expected, output, -1, &error); g_assert_no_error (error); if (diff && diff[0]) diff --git a/testsuite/gtk/test-focus-chain.c b/testsuite/gtk/test-focus-chain.c index e58912ae0d..8af9954e29 100644 --- a/testsuite/gtk/test-focus-chain.c +++ b/testsuite/gtk/test-focus-chain.c @@ -256,7 +256,7 @@ load_ui_file (GFile *ui_file, dir = get_dir_for_file (ref_path); output = generate_focus_chain (window, dir); - diff = diff_with_file (ref_path, output, -1, &error); + diff = diff_string_with_file (ref_path, output, -1, &error); g_assert_no_error (error); if (diff && diff[0]) diff --git a/testsuite/testutils.c b/testsuite/testutils.c index ce9f474122..a3ca396fba 100644 --- a/testsuite/testutils.c +++ b/testsuite/testutils.c @@ -19,6 +19,7 @@ #include #include +#include #ifdef G_OS_WIN32 #include @@ -28,66 +29,93 @@ #include "testsuite/testutils.h" +/* + * diff_bytes_with_file: + * @file1: The filename of the original. This is assumed + * to be the reference to compare against. + * @input: some text contained in a `GBytes` that is + * supposed to match the file's contents + * @error: A return location for an error if diffing could + * not happen + * + * Diffs generated text with a reference file. + * + * If the diffing runs into any error, NULL is returned and + * `error` is set. If diffing succeeds, the error is not set + * and NULL is returned if the file was identical to the + * contents of the file or the actual diff is returned if + * they were not. + * + * Returns: NULL on success or failure, the diff on failure + */ char * -diff_with_file (const char *file1, - const char *text, - gssize len, - GError **error) +diff_bytes_with_file (const char *file1, + GBytes *input, + GError **error) { - char *diff_cmd, *diff, *tmpfile; - int fd; + char *diff_cmd, *diff; diff = NULL; diff_cmd = g_find_program_in_path ("diff"); if (diff_cmd) { - const char *command[] = { diff_cmd, "--strip-trailing-cr", "-u", file1, NULL, NULL }; + GSubprocess *process; + GBytes *output; - if (len < 0) - len = strlen (text); - - /* write the text buffer to a temporary file */ - fd = g_file_open_tmp (NULL, &tmpfile, error); - if (fd < 0) + process = g_subprocess_new (G_SUBPROCESS_FLAGS_STDIN_PIPE + | G_SUBPROCESS_FLAGS_STDOUT_PIPE, + error, + diff_cmd, "--strip-trailing-cr", "-u", file1, "-", NULL); + if (process == NULL) return NULL; - if (write (fd, text, len) != (int) len) + if (!g_subprocess_communicate (process, + input, + NULL, + &output, + NULL, + error)) { - close (fd); - g_set_error (error, - G_FILE_ERROR, G_FILE_ERROR_FAILED, - "Could not write data to temporary file '%s'", tmpfile); - goto done; + g_object_unref (process); + return NULL; } - close (fd); - command[3] = tmpfile; - /* run diff command */ - g_spawn_sync (NULL, - (char **) command, - NULL, - 0, - NULL, NULL, - &diff, - NULL, NULL, - error); + if (g_subprocess_get_successful (process)) + { + g_clear_pointer (&output, g_bytes_unref); + } + else if (g_subprocess_get_if_exited (process) && g_subprocess_get_exit_status (process) == 1) + { + gsize size; -done: - g_unlink (tmpfile); - g_free (tmpfile); - g_free (diff_cmd); + /* this is the condition when the files differ */ + diff = g_bytes_unref_to_data (output, &size); + output = NULL; + } + else + { + g_clear_pointer (&output, g_bytes_unref); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "The `diff' process exited with error status %d", + g_subprocess_get_exit_status (process)); + } + + g_object_unref (process); } else { char *buf1; - gsize len1; + const char *buf2; + gsize len1, len2; + + buf2 = g_bytes_get_data (input, &len2); if (!g_file_get_contents (file1, &buf1, &len1, error)) return NULL; - if ((len != -1 && len != len1) || - strncmp (text, buf1, len1) != 0) + if ((len2 != len1) || + strncmp (buf2, buf1, len1) != 0) diff = g_strdup ("Files differ.\n"); g_free (buf1); @@ -95,3 +123,24 @@ done: return diff; } + +char * +diff_string_with_file (const char *file1, + const char *text, + gssize len, + GError **error) +{ + GBytes *bytes; + char *result; + + if (len < 0) + len = strlen (text); + + bytes = g_bytes_new_static (text, len); + + result = diff_bytes_with_file (file1, bytes, error); + + g_bytes_unref (bytes); + + return result; +} diff --git a/testsuite/testutils.h b/testsuite/testutils.h index bbb720d6ff..72471c10f7 100644 --- a/testsuite/testutils.h +++ b/testsuite/testutils.h @@ -2,8 +2,11 @@ #include -char * diff_with_file (const char *file1, - const char *text, - gssize len, - GError **error); +char * diff_string_with_file (const char *file1, + const char *text, + gssize len, + GError **error); +char * diff_bytes_with_file (const char *file1, + GBytes *bytes, + GError **error); From 2e8fac7789feae07eb3b75142e7daa067f00d11f Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 4 Nov 2024 21:47:37 +0100 Subject: [PATCH 10/17] testsuite: Make nodeparser use shared diffing code We have a testutils.c version, use that one. --- testsuite/gsk/meson.build | 3 +- testsuite/gsk/node-parser.c | 62 +++++++------------------------------ 2 files changed, 14 insertions(+), 51 deletions(-) diff --git a/testsuite/gsk/meson.build b/testsuite/gsk/meson.build index ee9387c149..24130a1fa0 100644 --- a/testsuite/gsk/meson.build +++ b/testsuite/gsk/meson.build @@ -4,7 +4,8 @@ compare_render = executable('compare-render', c_args: common_cflags + ['-DGTK_COMPILATION'], ) -node_parser = executable('node-parser', 'node-parser.c', +node_parser = executable('node-parser', + [ 'node-parser.c', '../testutils.c' ], dependencies: libgtk_dep, c_args: common_cflags, ) diff --git a/testsuite/gsk/node-parser.c b/testsuite/gsk/node-parser.c index 99928ff441..95d2dd2429 100644 --- a/testsuite/gsk/node-parser.c +++ b/testsuite/gsk/node-parser.c @@ -22,6 +22,8 @@ #include +#include "../testutils.h" + static char * test_get_reference_file (const char *node_file) { @@ -64,47 +66,6 @@ test_get_errors_file (const char *node_file) return g_string_free (file, FALSE); } -static GBytes * -diff_with_file (const char *file1, - GBytes *input, - GError **error) -{ - GSubprocess *process; - GBytes *output; - - process = g_subprocess_new (G_SUBPROCESS_FLAGS_STDIN_PIPE - | G_SUBPROCESS_FLAGS_STDOUT_PIPE, - error, - "diff", "--strip-trailing-cr", "-u", file1, "-", NULL); - if (process == NULL) - return NULL; - - if (!g_subprocess_communicate (process, - input, - NULL, - &output, - NULL, - error)) - { - g_object_unref (process); - return NULL; - } - - if (!g_subprocess_get_successful (process) && - /* this is the condition when the files differ */ - !(g_subprocess_get_if_exited (process) && g_subprocess_get_exit_status (process) == 1)) - { - g_clear_pointer (&output, g_bytes_unref); - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "The `diff' process exited with error status %d", - g_subprocess_get_exit_status (process)); - } - - g_object_unref (process); - - return output; -} - static void append_error_value (GString *string, GType enum_type, @@ -162,8 +123,9 @@ parse_node_file (GFile *file, gboolean generate) char *node_file, *reference_file, *errors_file; GskRenderNode *node; GString *errors; - GBytes *diff, *bytes; + GBytes *bytes; GError *error = NULL; + char *diff; gboolean result = TRUE; bytes = g_file_load_bytes (file, NULL, NULL, &error); @@ -193,33 +155,33 @@ parse_node_file (GFile *file, gboolean generate) node_file = g_file_get_path (file); reference_file = test_get_reference_file (node_file); - diff = diff_with_file (reference_file, bytes, &error); + diff = diff_bytes_with_file (reference_file, bytes, &error); g_assert_no_error (error); - if (diff && g_bytes_get_size (diff) > 0) + if (diff) { g_print ("Resulting file doesn't match reference:\n%s\n", - (const char *) g_bytes_get_data (diff, NULL)); + (const char *) diff); result = FALSE; } g_free (reference_file); - g_clear_pointer (&diff, g_bytes_unref); + g_clear_pointer (&diff, g_free); errors_file = test_get_errors_file (node_file); if (errors_file) { GBytes *error_bytes = g_string_free_to_bytes (errors); - diff = diff_with_file (errors_file, error_bytes, &error); + diff = diff_bytes_with_file (errors_file, error_bytes, &error); g_assert_no_error (error); - if (diff && g_bytes_get_size (diff) > 0) + if (diff) { g_print ("Errors don't match expected errors:\n%s\n", - (const char *) g_bytes_get_data (diff, NULL)); + (const char *) diff); result = FALSE; } - g_clear_pointer (&diff, g_bytes_unref); + g_clear_pointer (&diff, g_free); g_clear_pointer (&error_bytes, g_bytes_unref); } else if (errors->str[0]) From bfbc3e7484feb9b9aafc5f92efa39158c386ef71 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Tue, 5 Nov 2024 02:42:58 +0100 Subject: [PATCH 11/17] testsuite: Clarify error 200% is not a fractional scale, but it's still forbidden. --- testsuite/gsk/offload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testsuite/gsk/offload.c b/testsuite/gsk/offload.c index f559232b33..98a0b782f6 100644 --- a/testsuite/gsk/offload.c +++ b/testsuite/gsk/offload.c @@ -391,7 +391,7 @@ parse_node_file (GFile *file, const char *generate) if (gdk_surface_get_scale (surface) != 1.0) { - g_print ("Offload tests don't work with fractional scales\n"); + g_print ("Offload tests don't work with scale != 1.0\n"); exit (77); } From 2a7beb75e8afa89e1c983d59135d03d84dd10bc6 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 4 Nov 2024 21:48:14 +0100 Subject: [PATCH 12/17] testsuite: Make offload test do actual diffs We have a testutils.c version for that. --- testsuite/gsk/meson.build | 3 ++- testsuite/gsk/offload.c | 44 ++++++++++++--------------------------- 2 files changed, 15 insertions(+), 32 deletions(-) diff --git a/testsuite/gsk/meson.build b/testsuite/gsk/meson.build index 24130a1fa0..5eb636f1cf 100644 --- a/testsuite/gsk/meson.build +++ b/testsuite/gsk/meson.build @@ -489,7 +489,8 @@ endforeach # offload does not work outside of linux if os_linux - offload = executable('offload', 'offload.c', 'gskrendernodeattach.c', + offload = executable('offload', + [ 'offload.c', 'gskrendernodeattach.c', '../testutils.c' ], dependencies : libgtk_static_dep, c_args: common_cflags + [ '-DGTK_COMPILATION=1' ], ) diff --git a/testsuite/gsk/offload.c b/testsuite/gsk/offload.c index 98a0b782f6..daf725d9d5 100644 --- a/testsuite/gsk/offload.c +++ b/testsuite/gsk/offload.c @@ -29,6 +29,8 @@ #include #include "gskrendernodeattach.h" +#include "../testutils.h" + static char * test_get_sibling_file (const char *node_file, const char *old_ext, @@ -52,26 +54,6 @@ test_get_sibling_file (const char *node_file, return g_string_free (file, FALSE); } -static GBytes * -diff_with_file (const char *file1, - GBytes *input, - GError **error) -{ - char *buffer; - gsize len; - static const char msg[] = "The output is not as expected"; - - g_file_get_contents (file1, &buffer, &len, NULL); - if (strcmp (buffer, (char *) g_bytes_get_data (input, NULL)) == 0) - { - g_free (buffer); - return NULL; - } - - g_free (buffer); - return g_bytes_new_static (msg, strlen (msg) + 1); -} - static void append_error_value (GString *string, GType enum_type, @@ -225,7 +207,7 @@ collect_offload_info (GdkSurface *surface, info->was_offloaded ? "was offloaded, " : ""); } - bytes = g_bytes_new (s->str, s->len + 1); + bytes = g_bytes_new (s->str, s->len); g_string_free (s, TRUE); @@ -364,11 +346,11 @@ parse_node_file (GFile *file, const char *generate) GdkSubsurface *subsurface; GskOffload *offload; GskRenderNode *node, *tmp; - GBytes *offload_state, *diff; + GBytes *offload_state; GError *error = NULL; gboolean result = TRUE; cairo_region_t *clip, *region; - char *path; + char *path, *diff; GskRenderNode *node2; const char *generate_values[] = { "offload", "offload2", "diff", NULL }; @@ -425,20 +407,20 @@ parse_node_file (GFile *file, const char *generate) if (reference_file == NULL) return FALSE; - diff = diff_with_file (reference_file, offload_state, &error); + diff = diff_bytes_with_file (reference_file, offload_state, &error); g_assert_no_error (error); - if (diff && g_bytes_get_size (diff) > 0) + if (diff) { char *basename = g_path_get_basename (reference_file); g_print ("Resulting file doesn't match reference (%s):\n%s\n", basename, - (const char *) g_bytes_get_data (diff, NULL)); + diff); g_free (basename); result = FALSE; } g_clear_pointer (&offload_state, g_bytes_unref); - g_clear_pointer (&diff, g_bytes_unref); + g_clear_pointer (&diff, g_free); g_clear_pointer (&reference_file, g_free); path = test_get_sibling_file (g_file_peek_path (file), ".node", ".node2"); @@ -464,20 +446,20 @@ parse_node_file (GFile *file, const char *generate) if (reference_file == NULL) return FALSE; - diff = diff_with_file (reference_file, offload_state, &error); + diff = diff_bytes_with_file (reference_file, offload_state, &error); g_assert_no_error (error); - if (diff && g_bytes_get_size (diff) > 0) + if (diff) { char *basename = g_path_get_basename (reference_file); g_print ("Resulting file doesn't match reference (%s):\n%s\n", basename, - (const char *) g_bytes_get_data (diff, NULL)); + diff); g_free (basename); result = FALSE; } g_clear_pointer (&offload_state, g_bytes_unref); - g_clear_pointer (&diff, g_bytes_unref); + g_clear_pointer (&diff, g_free); g_clear_pointer (&reference_file, g_free); gsk_render_node_diff (node, node2, &(GskDiffData) { clip, surface }); From ef839e650546a94428ad17c74c92679ed729e81a Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Thu, 31 Oct 2024 14:25:52 +0100 Subject: [PATCH 13/17] win32: Add gdk_win32_com_clear() Like g_clear_object(), but for COM objects. --- gdk/win32/gdkprivate-win32.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/gdk/win32/gdkprivate-win32.h b/gdk/win32/gdkprivate-win32.h index 4bc1a99612..24e8089097 100644 --- a/gdk/win32/gdkprivate-win32.h +++ b/gdk/win32/gdkprivate-win32.h @@ -95,6 +95,29 @@ gboolean _gdk_modal_blocked (GdkSurface *surface); gboolean gdk_win32_ensure_com (void); gboolean gdk_win32_ensure_ole (void); +/* + * gdk_win32_com_clear: + * @com_ptr: pointer to a COM object pointer + * + * Clears a reference to a COM object. + * + * `com_ptr` must not be `NULL`. + * + * If the reference is `NULL` then this function does nothing. + * Otherwise, the reference count of the object is decreased + * and the pointer is set to NULL. + * + * Think of this function like g_clear_object() but for COM objects. + */ +#define gdk_win32_com_clear(com_ptr) \ +G_STMT_START {\ + if (*(com_ptr)) \ + { \ + (*(com_ptr))->lpVtbl->Release (*(com_ptr)); \ + *(com_ptr) = NULL; \ + } \ +}G_STMT_END + void _gdk_win32_print_dc (HDC hdc); char *_gdk_win32_surface_state_to_string (GdkToplevelState state); From 2ed6867084766a0a4766d78c63b4de9446228cd5 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sun, 3 Nov 2024 03:46:53 +0100 Subject: [PATCH 14/17] win32: Define COBJMACROS via project argument We use it everywhere, so it makes sense to enable it everywhere. For anyone not in the know, defining COBJMACROS makes Micrsoft headers for COM objects provide C macros so that instead of having to call foo->lpVtbl->Release(); to unref a COM object, one can call IFoo_Release (foo); Note that thes macros are implemented with inheritance as Release() is defined on the IUnknown base interface (MS' equivalent to GObject) and would otherwise require IUnknown_Release ((IUnknown *) foo); That line works, too - but it is not necessary. --- gdk/win32/gdkclipdrop-win32.c | 3 --- gdk/win32/gdkdisplay-win32.h | 1 - gdk/win32/gdkdrag-win32.c | 3 --- gdk/win32/gdkdrop-win32.c | 3 --- gdk/win32/gdkinput-dmanipulation.c | 1 - gtk/gtkfilechoosernativewin32.c | 2 -- gtk/print/gtkprintoperation-win32.c | 2 +- meson.build | 1 + 8 files changed, 2 insertions(+), 14 deletions(-) diff --git a/gdk/win32/gdkclipdrop-win32.c b/gdk/win32/gdkclipdrop-win32.c index ba713f40ec..15e5855cc7 100644 --- a/gdk/win32/gdkclipdrop-win32.c +++ b/gdk/win32/gdkclipdrop-win32.c @@ -267,9 +267,6 @@ Otherwise it's similar to how the clipboard works. Only the DnD server #include #include -/* For C-style COM wrapper macros */ -#define COBJMACROS - /* for CIDA */ #include diff --git a/gdk/win32/gdkdisplay-win32.h b/gdk/win32/gdkdisplay-win32.h index a917e2e30f..de9c36a5a5 100644 --- a/gdk/win32/gdkdisplay-win32.h +++ b/gdk/win32/gdkdisplay-win32.h @@ -28,7 +28,6 @@ #include "gdkglversionprivate.h" /* Used for active language or text service change notifications */ -#define COBJMACROS #include #ifdef HAVE_EGL diff --git a/gdk/win32/gdkdrag-win32.c b/gdk/win32/gdkdrag-win32.c index 87fc0ae1c8..6a23ca6c47 100644 --- a/gdk/win32/gdkdrag-win32.c +++ b/gdk/win32/gdkdrag-win32.c @@ -192,9 +192,6 @@ #define INITGUID #endif -/* For C-style COM wrapper macros */ -#define COBJMACROS - #include "gdkdrag.h" #include "gdkprivate-win32.h" #include "gdkwin32.h" diff --git a/gdk/win32/gdkdrop-win32.c b/gdk/win32/gdkdrop-win32.c index 94ed2e89fb..7ca058aa14 100644 --- a/gdk/win32/gdkdrop-win32.c +++ b/gdk/win32/gdkdrop-win32.c @@ -36,9 +36,6 @@ #define INITGUID #endif -/* For C-style COM wrapper macros */ -#define COBJMACROS - #include "gdkdropprivate.h" #include "gdkdrag.h" diff --git a/gdk/win32/gdkinput-dmanipulation.c b/gdk/win32/gdkinput-dmanipulation.c index e257f656a5..6148e512eb 100644 --- a/gdk/win32/gdkinput-dmanipulation.c +++ b/gdk/win32/gdkinput-dmanipulation.c @@ -28,7 +28,6 @@ #endif #define WINVER 0x0603 #define _WIN32_WINNT 0x0603 -#define COBJMACROS #include "config.h" #include diff --git a/gtk/gtkfilechoosernativewin32.c b/gtk/gtkfilechoosernativewin32.c index 01164d357b..bdb4b8f70d 100644 --- a/gtk/gtkfilechoosernativewin32.c +++ b/gtk/gtkfilechoosernativewin32.c @@ -19,8 +19,6 @@ #include "config.h" -#define COBJMACROS - #include "gtkfilechoosernativeprivate.h" #include "gtknativedialogprivate.h" diff --git a/gtk/print/gtkprintoperation-win32.c b/gtk/print/gtkprintoperation-win32.c index 85c04ad2a1..eedd385192 100644 --- a/gtk/print/gtkprintoperation-win32.c +++ b/gtk/print/gtkprintoperation-win32.c @@ -16,8 +16,8 @@ * License along with this library. If not, see . */ -#define COBJMACROS #include "config.h" + #include #ifdef HAVE_UNISTD_H #include diff --git a/meson.build b/meson.build index 781f6b13af..a87d202421 100644 --- a/meson.build +++ b/meson.build @@ -63,6 +63,7 @@ add_project_arguments('-D_GNU_SOURCE', language: 'c') if host_machine.system() == 'windows' add_project_arguments(['-DUNICODE', '-D_UNICODE', + '-DCOBJMACROS', '-D_WIN32_WINNT=_WIN32_WINNT_WIN7'], language: 'c') endif From 3d578e8db5142a7900d72bda66133f34590f69f9 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Sun, 3 Nov 2024 12:28:12 +0100 Subject: [PATCH 15/17] win32: Use gdk_win32_com_clear() where appropriate --- gdk/win32/gdkdrop-win32.c | 6 ++---- gdk/win32/gdkinput-dmanipulation.c | 9 ++++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/gdk/win32/gdkdrop-win32.c b/gdk/win32/gdkdrop-win32.c index 7ca058aa14..4c7f4dfafe 100644 --- a/gdk/win32/gdkdrop-win32.c +++ b/gdk/win32/gdkdrop-win32.c @@ -300,8 +300,7 @@ query_object_formats (GdkDisplay *display, hr = IEnumFORMATETC_Next (pfmt, 1, &fmt, NULL); } - if (pfmt) - IEnumFORMATETC_Release (pfmt); + gdk_win32_com_clear (&pfmt); result_formats = gdk_content_formats_builder_free_to_formats (builder); @@ -311,8 +310,7 @@ query_object_formats (GdkDisplay *display, static void set_data_object (LPDATAOBJECT *location, LPDATAOBJECT data_object) { - if (*location != NULL) - IDataObject_Release (*location); + gdk_win32_com_clear (location); *location = data_object; diff --git a/gdk/win32/gdkinput-dmanipulation.c b/gdk/win32/gdkinput-dmanipulation.c index 6148e512eb..5e9ea0eebe 100644 --- a/gdk/win32/gdkinput-dmanipulation.c +++ b/gdk/win32/gdkinput-dmanipulation.c @@ -37,6 +37,7 @@ #include "gdkdevice-virtual.h" #include "gdkdeviceprivate.h" #include "gdkdisplay-win32.h" +#include "gdkprivate-win32.h" #include "gdkeventsprivate.h" #include "gdkseatdefaultprivate.h" #include "gdkinput-dmanipulation.h" @@ -359,7 +360,7 @@ reset_viewport (IDirectManipulationViewport *viewport) HR_CHECK_GOTO (hr, failed); failed: - IUnknown_Release (content); + gdk_win32_com_clear (&content); } static void @@ -391,8 +392,7 @@ gdk_win32_display_close_dmanip_manager (GdkDisplay *display) { IDirectManipulationManager *manager = GDK_DISPLAY_GET_DMANIP_MANAGER (display); - if (manager != NULL) - IUnknown_Release (manager); + gdk_win32_com_clear (&manager); g_clear_pointer (&GDK_WIN32_DISPLAY (display)->dmanip_items, g_free); } @@ -451,8 +451,7 @@ create_viewport (GdkSurface *surface, return; failed: - if (handler) - IUnknown_Release (handler); + gdk_win32_com_clear (&handler); close_viewport (pViewport); } From 01453c733cfb9c5a2531fd9854f5e88d036fd8f0 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Mon, 4 Nov 2024 23:12:47 +0100 Subject: [PATCH 16/17] testsuite: Rename "diff" test to "not-diff" 3 things you need to know about this change: 1. We use diff(1) in various tests to check generated text against reference output 2. Windows locates executables not just in $PATH, it also looks in $cwd and the directory of the current process' binary 3. Multiple tests live together in the same directory Windows is fun. --- testsuite/gsk/meson.build | 2 +- testsuite/gsk/{diff.c => not-diff.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename testsuite/gsk/{diff.c => not-diff.c} (100%) diff --git a/testsuite/gsk/meson.build b/testsuite/gsk/meson.build index 5eb636f1cf..00e7a05976 100644 --- a/testsuite/gsk/meson.build +++ b/testsuite/gsk/meson.build @@ -570,8 +570,8 @@ internal_tests = [ [ 'boundingbox'], [ 'curve', [ ], [ 'flaky' ]], [ 'curve-special-cases' ], - [ 'diff' ], [ 'half-float' ], + [ 'not-diff' ], [ 'misc'], [ 'path-private' ], [ 'rounded-rect'], diff --git a/testsuite/gsk/diff.c b/testsuite/gsk/not-diff.c similarity index 100% rename from testsuite/gsk/diff.c rename to testsuite/gsk/not-diff.c From b8d24d0259f4de255d687b5ffaed8e48378d61a1 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Tue, 5 Nov 2024 02:06:30 +0100 Subject: [PATCH 17/17] win32: Require Windows 10 All versions older than Windows 10 are out of support and no longer receive updates, so we do not want to support them. We also want to move towards APIs that requires Windows 10 - like Direct3D 12 - and not having them optional simplifies our code. See the discussion in !7895 for more details. --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index a87d202421..c410c05fd2 100644 --- a/meson.build +++ b/meson.build @@ -64,7 +64,7 @@ if host_machine.system() == 'windows' add_project_arguments(['-DUNICODE', '-D_UNICODE', '-DCOBJMACROS', - '-D_WIN32_WINNT=_WIN32_WINNT_WIN7'], language: 'c') + '-D_WIN32_WINNT=_WIN32_WINNT_WIN10'], language: 'c') endif # Use debug/optimization flags to determine whether to enable debug or disable