From 6c998dbc3281f341696db9f7b7dcbebd6096e94b Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Fri, 10 May 2024 17:24:18 +0000 Subject: [PATCH 01/16] Plumbs `ENABLE_UNIT_TESTS` through the build system, and conditionally compiles and links googletest into geeqie when unit tests are enabled. This is following the pattern from: https://stackoverflow.com/a/57478960 --- .github/workflows/check-build-actions.yml | 1 + config.h.in | 5 +++++ meson.build | 14 ++++++++++++++ meson_options.txt | 1 + scripts/test-all.sh | 1 + src/meson.build | 2 +- 6 files changed, 23 insertions(+), 1 deletion(-) diff --git a/.github/workflows/check-build-actions.yml b/.github/workflows/check-build-actions.yml index ad8e3c01..fbef1c5e 100644 --- a/.github/workflows/check-build-actions.yml +++ b/.github/workflows/check-build-actions.yml @@ -61,6 +61,7 @@ jobs: -Dpdf=disabled -Dspell=disabled -Dtiff=disabled + -Dunit_tests=disabled -Dvideothumbnailer=disabled -Dwebp=disabled -Dyelp-build=disabled diff --git a/config.h.in b/config.h.in index 48638558..d33db167 100644 --- a/config.h.in +++ b/config.h.in @@ -226,4 +226,9 @@ /* Do not use */ #mesondefine HAVE_GTK4 +/* Whether to enable a mode of Geeqie that executes unit tests instead of + running the actual app. Actually executing the unit tests _also_ requires + a command-line argument to be supplied. */ +#mesondefine ENABLE_UNIT_TESTS + #endif diff --git a/meson.build b/meson.build index 3a6764ec..80d32451 100644 --- a/meson.build +++ b/meson.build @@ -200,6 +200,13 @@ else summary({'execinfo' : ['stacktrace supported:', false]}, section : 'Debugging', bool_yn : true) endif +conf_data.set('ENABLE_UNIT_TESTS', 0) +option = get_option('unit_tests') +if not option.disabled() + conf_data.set('ENABLE_UNIT_TESTS', 1) + # Summary is handled below, where the test() itself is defined. +endif + conf_data.set('HAVE_ARCHIVE', 0) libarchive_dep = [] req_version = '>=3.4.0' @@ -611,6 +618,13 @@ ui_sources = [] subdir('po') subdir('plugins') +conditional_unit_test_deps = [] +if conf_data.get('ENABLE_UNIT_TESTS', 0) == 1 + gtest_proj = subproject('gtest') + conditional_unit_test_deps += gtest_proj.get_variable('gtest_dep') + conditional_unit_test_deps += gtest_proj.get_variable('gmock_dep') +endif + # Generate the executable subdir('src') diff --git a/meson_options.txt b/meson_options.txt index 9bc8508a..3f92bd0c 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -44,6 +44,7 @@ option('pandoc', type : 'feature', value : 'auto', description : 'README.html') option('pdf', type : 'feature', value : 'auto', description : 'pdf') option('spell', type : 'feature', value : 'auto', description : 'spelling checks') option('tiff', type : 'feature', value : 'auto', description : 'tiff') +option('unit_tests', type : 'feature', value : 'disabled', description : 'unit tests') option('videothumbnailer', type : 'feature', value : 'auto', description : 'video thumbnailer') option('webp', type : 'feature', value : 'auto', description : 'webp') option('yelp-build', type : 'feature', value : 'auto', description : 'help files') diff --git a/scripts/test-all.sh b/scripts/test-all.sh index 4d87a021..6d65bab5 100755 --- a/scripts/test-all.sh +++ b/scripts/test-all.sh @@ -64,6 +64,7 @@ meson setup \ -Dpdf=disabled \ -Dspell=disabled \ -Dtiff=disabled \ +-Dunit_tests=disabled \ -Dvideothumbnailer=disabled \ -Dwebp=disabled \ -Dyelp-build=disabled \ diff --git a/src/meson.build b/src/meson.build index 67330e30..c1528760 100644 --- a/src/meson.build +++ b/src/meson.build @@ -360,5 +360,5 @@ lua_dep, poppler_glib_dep, thread_dep, tiff_dep -], +] + conditional_unit_test_deps, include_directories : [configuration_inc], install : true) -- 2.20.1 From 97a889d17a7acce4a4ad2b6e596f601c7ebbf43e Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Fri, 10 May 2024 17:34:14 +0000 Subject: [PATCH 02/16] Creates some initial unit tests, and adds the build system infrastructure to compile and link them when appropriate. --- src/meson.build | 4 ++ src/tests/meson.build | 24 +++++++++++ src/tests/pixbuf-util.cc | 90 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 src/tests/meson.build create mode 100644 src/tests/pixbuf-util.cc diff --git a/src/meson.build b/src/meson.build index c1528760..e39330e7 100644 --- a/src/meson.build +++ b/src/meson.build @@ -309,6 +309,10 @@ subdir('third-party') subdir('ui') subdir('view-file') +if conf_data.get('ENABLE_UNIT_TESTS', 0) == 1 + subdir('tests') +endif + gq_marshal = gnome.genmarshal('gq-marshal', prefix : 'gq_marshal', sources : 'gq-marshal.list') project_sources += gq_marshal[1] diff --git a/src/tests/meson.build b/src/tests/meson.build new file mode 100644 index 00000000..b7a99513 --- /dev/null +++ b/src/tests/meson.build @@ -0,0 +1,24 @@ +# +# Copyright (C) 2024 The Geeqie Team +# +# Author: Omari Stephens +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Build file to configure and run unit tests. + +unit_test_sources = files('pixbuf-util.cc') + +project_sources += unit_test_sources diff --git a/src/tests/pixbuf-util.cc b/src/tests/pixbuf-util.cc new file mode 100644 index 00000000..1fcb1958 --- /dev/null +++ b/src/tests/pixbuf-util.cc @@ -0,0 +1,90 @@ +/* + * Copyright (C) 2024 The Geeqie Team + * + * Author: Omari Stephens + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * + * Unit tests for pixbuf-util.cc + * + */ + +#include "gtest/gtest.h" + +#include "pixbuf-util.h" + +namespace { + +class ClipRegionTest : public ::testing::Test +{ + protected: + gint rx{}; + gint ry{}; + gint rw{}; + gint rh{}; +}; + +TEST_F(ClipRegionTest, RegionAContainsRegionB) +{ + ASSERT_TRUE(util_clip_region(0, 0, 1000, 1000, + 50, 50, 100, 100, + &rx, &ry, &rw, &rh)); + + ASSERT_EQ(50, rx); + ASSERT_EQ(50, ry); + ASSERT_EQ(100, rw); + ASSERT_EQ(100, rh); +} + +TEST_F(ClipRegionTest, RegionBContainsRegionA) +{ + ASSERT_TRUE(util_clip_region(50, 50, 100, 100, + 0, 0, 1000, 1000, + &rx, &ry, &rw, &rh)); + + ASSERT_EQ(50, rx); + ASSERT_EQ(50, ry); + ASSERT_EQ(100, rw); + ASSERT_EQ(100, rh); +} + +TEST_F(ClipRegionTest, PartialOverlapWithBAfterA) +{ + ASSERT_TRUE(util_clip_region(0, 0, 1000, 1000, + 500, 500, 1000, 1000, + &rx, &ry, &rw, &rh)); + + ASSERT_EQ(500, rx); + ASSERT_EQ(500, ry); + ASSERT_EQ(500, rw); + ASSERT_EQ(500, rh); +} + +TEST_F(ClipRegionTest, PartialOverlapWithAAfterB) +{ + ASSERT_TRUE(util_clip_region(500, 500, 1000, 1000, + 0, 0, 1000, 1000, + &rx, &ry, &rw, &rh)); + + ASSERT_EQ(500, rx); + ASSERT_EQ(500, ry); + ASSERT_EQ(500, rw); + ASSERT_EQ(500, rh); +} + +} // anonymous namespace + +/* vim: set shiftwidth=8 softtabstop=0 cindent cinoptions={1s: */ -- 2.20.1 From 4bbcb0040a583968e9d9f7223f8e30b74fc17525 Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Fri, 10 May 2024 17:39:54 +0000 Subject: [PATCH 03/16] Cleans up and slightly refactors command line interrogation functions in main.cc. This is in preparation for conditionally executing unit tests when that argument is specified. --- src/main.cc | 80 ++++++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 38 deletions(-) diff --git a/src/main.cc b/src/main.cc index 08dee254..23195110 100644 --- a/src/main.cc +++ b/src/main.cc @@ -633,28 +633,24 @@ static void parse_command_line_for_debug_option(gint argc, gchar *argv[]) { #ifdef DEBUG const gchar *debug_option = "--debug"; - gint len = strlen(debug_option); + const gint len = strlen(debug_option); - if (argc > 1) + for (gint i = 1; i < argc; i++) { - gint i; - - for (i = 1; i < argc; i++) + // TODO(xsdg): Replace this with a regex match. Simpler and less error-prone. + const gchar *cmd_line = argv[i]; + if (strncmp(cmd_line, debug_option, len) == 0) { - const gchar *cmd_line = argv[i]; - if (strncmp(cmd_line, debug_option, len) == 0) - { - gint cmd_line_len = strlen(cmd_line); + const gint cmd_line_len = strlen(cmd_line); - /* we now increment the debug state for verbosity */ - if (cmd_line_len == len) - debug_level_add(1); - else if (cmd_line[len] == '=' && g_ascii_isdigit(cmd_line[len+1])) - { - gint n = atoi(cmd_line + len + 1); - if (n < 0) n = 1; - debug_level_add(n); - } + /* we now increment the debug state for verbosity */ + if (cmd_line_len == len) + debug_level_add(1); + else if (cmd_line[len] == '=' && g_ascii_isdigit(cmd_line[len+1])) + { + gint n = atoi(cmd_line + len + 1); + if (n < 0) n = 1; + debug_level_add(n); } } } @@ -663,47 +659,55 @@ static void parse_command_line_for_debug_option(gint argc, gchar *argv[]) #endif } -#if HAVE_CLUTTER -static gboolean parse_command_line_for_clutter_option(gint argc, gchar *argv[]) +static gboolean search_command_line_for_option(const gint argc, const gchar* const argv[], const gchar* option_name) { - const gchar *clutter_option = "--disable-clutter"; - gint len = strlen(clutter_option); - gboolean ret = FALSE; + const gint name_len = strlen(option_name); - if (argc > 1) + for (gint i = 1; i < argc; i++) { - gint i; - - for (i = 1; i < argc; i++) + const gchar *current_arg = argv[i]; + // TODO(xsdg): This actually only checks prefixes. We should + // probably replace this with strcmp, since strlen already has + // the shortcomings of strcmp (as compared to strncmp). + // + // That said, people may be unknowingly relying on the lenience + // of this parsing strategy, so that's also something to consider. + if (strncmp(current_arg, option_name, name_len) == 0) { - const gchar *cmd_line = argv[i]; - if (strncmp(cmd_line, clutter_option, len) == 0) - { - ret = TRUE; - } + return TRUE; } } - return ret; + return FALSE; +} + +static gboolean search_command_line_for_unit_test_option(gint argc, gchar *argv[]) +{ + return search_command_line_for_option(argc, argv, "--run-unit-tests"); +} + +#if HAVE_CLUTTER +static gboolean search_command_line_for_clutter_option(gint argc, gchar *argv[]) +{ + return search_command_line_for_option(argc, argv, "--disable-clutter"); } #endif static gboolean parse_command_line_for_cache_maintenance_option(gint argc, gchar *argv[]) { const gchar *cache_maintenance_option = "--cache-maintenance="; - gint len = strlen(cache_maintenance_option); - gboolean ret = FALSE; + const gint len = strlen(cache_maintenance_option); if (argc >= 2) { const gchar *cmd_line = argv[1]; if (strncmp(cmd_line, cache_maintenance_option, len) == 0) { - ret = TRUE; + return TRUE; } } - return ret; + return FALSE; } static void process_command_line_for_cache_maintenance_option(gint argc, gchar *argv[]) @@ -1313,7 +1317,7 @@ gint main(gint argc, gchar *argv[]) parse_command_line_for_debug_option(argc, argv); DEBUG_1("%s main: gtk_init", get_exec_time()); #if HAVE_CLUTTER - if (parse_command_line_for_clutter_option(argc, argv)) + if (search_command_line_for_clutter_option(argc, argv)) { disable_clutter = TRUE; gtk_init(&argc, &argv); -- 2.20.1 From 7436ae73e169ff83dd1f7fe4107266ff3c8037c5 Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Fri, 10 May 2024 17:43:21 +0000 Subject: [PATCH 04/16] Runs unit tests from main.cc when requested, and integrates that into the build system test plumbing. To run just the unit tests is now just two steps: ``` $ meson setup -D unit_tests=enabled build $ meson test -C build -v 'Unit tests' ``` --- meson.build | 8 ++++++++ src/main.cc | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/meson.build b/meson.build index 80d32451..21d4c3ae 100644 --- a/meson.build +++ b/meson.build @@ -823,3 +823,11 @@ test_ancillary_files_sh = find_program('test-ancillary-files.sh', dirs : scripts test('Ancillary files', test_ancillary_files_sh, args: [meson.current_source_dir()], timeout: 100) summary({'Ancillary files' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) + +# Unit tests +if conf_data.get('ENABLE_UNIT_TESTS', 0) == 1 + test('Unit tests', geeqie_exe, args: ['--run-unit-tests']) + summary({'unit_tests' : ['Tests run:', true]}, section : 'Testing', bool_yn : true) +else + summary({'unit_tests' : ['Tests run:', false]}, section : 'Testing', bool_yn : true) +endif diff --git a/src/main.cc b/src/main.cc index 23195110..58c80f2b 100644 --- a/src/main.cc +++ b/src/main.cc @@ -84,6 +84,10 @@ #include "ui-fileops.h" #include "ui-utildlg.h" +#if ENABLE_UNIT_TESTS +# include "gtest/gtest.h" +#endif + gboolean thumb_format_changed = FALSE; static RemoteConnection *remote_connection = nullptr; @@ -1254,6 +1258,19 @@ static void create_application_paths() gint main(gint argc, gchar *argv[]) { + // We handle unit tests here because it takes the place of running the + // rest of the app. + if (search_command_line_for_unit_test_option(argc, argv)) + { +#if ENABLE_UNIT_TESTS + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +#else + fprintf(stderr, "Unit tests are not enabled in this build.\n"); + return 1; +#endif + } + CollectionData *cd = nullptr; CollectionData *first_collection = nullptr; gboolean disable_clutter = FALSE; -- 2.20.1 From cc794921b898302faf7fddfc279dc901b48161e0 Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Fri, 10 May 2024 18:57:34 +0000 Subject: [PATCH 05/16] Partitions the Geeqie tests into three suites, and adds testing-related documentation. --- TESTING.md | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++ meson.build | 22 ++++----- 2 files changed, 151 insertions(+), 11 deletions(-) create mode 100644 TESTING.md diff --git a/TESTING.md b/TESTING.md new file mode 100644 index 00000000..2d7fdd64 --- /dev/null +++ b/TESTING.md @@ -0,0 +1,140 @@ +# Geeqie testing and validation patterns + +Geeqie incorporates a number of forms of validation, including functional tests, +unit tests, and static analysis. These tests are defined towards the end of the +root `meson.build` file (search the file for `test(`). + +You can run all enabled suites using: + +```text +meson test -C build +``` + +Three test suites exist: `functional`, `unit`, and `analysis`. You can pick out +particular suites to execute with commands like: + +```text +meson test -C build --suite functional +meson test -C build --suite analysis --suite unit +``` + +See the Unit tests section for how to enable unit tests. + +## Unit tests + +Unit tests live under `src/tests`. Because they include a lot of extra code in +the Geeqie binary, they must be manually enabled with a command like: + +```text +meson setup -D unit_tests=enabled build +``` + +After that point, they can be executed with: + +```text +meson test -C build -v --suite unit +``` + +Or you can run them by hand by starting geeqie with the `--run-unit-tests` +argument: + +```text +$ ninja -C build +... + +$ ./build/src/geeqie --run-unit-tests +[==========] Running N tests from 1 test suite. +... +[==========] N tests from 1 test suite ran. (0 ms total) +[ PASSED ] N tests. +``` + +### Adding or modifying unit tests + +Geeqie uses the Googletest framework, which is well-documented: \ + + +To add a new testcase in an existing test file, just add it to the test file. +That testcase will be automatically picked up and executed. + +To create a new test file, create the file under `src/tests/` with a name that +matches the file being tested. **Then make sure to add the file to +`src/tests/meson.build` or it won't be seen or executed.** + +## Functional tests + +The Geeqie functional tests rely on `xvfb` in order to be able to start the app +in a standard way without requiring access to a real X server on the test +machine. If `xvfb` is not present, these tests will not run. + +### Basic test + +This just ensures that Geeqie will start. It uses the `--version` flag to keep +Geeqie from staying running. + +### Image tests + +The image tests are only enabled in developer mode. You can set that with: + +```text +meson setup -C build -D devel=enabled +``` + +This tests that Geeqie can successfully open and provide metadata info about a +library of images of different types. + +See `scripts/image-test.sh` for more details. + +### Lua tests + +Verifies that Geeqie can successfully run lua scripts by opening a stock test +image and running a variety of lua operations on it. + +See `scripts/lua-test.sh` for more details. + +## Static Analysis + +### Code correctness + +Runs `clang-tidy` code correctness checks for every source file in the project. +Note that this will only execute when running from a clone of the Geeqie git +project. + +See `.clang-tidy` and +for more details. + +### Single value enum checks + +Checks for single-value enums. + +See `scripts/enum-check.sh` for more details. + +### Debug statement checks + +Checks for `DEBUG_0`, `DEBUG_BT`, or `DEBUG_FD` statements in the source tree. + +See `scripts/debug-check.sh` for more details. + +### Temporary comment checks + +Checks for comments starting with `//~` in the source tree. + +See `scripts/temporary-comments-check.sh` for more details. + +### Untranslated text checks + +Checks for strings that haven't been marked for translation (starting with `_(`) +in the source tree. + +See `scripts/untranslated-text.sh` for more details. + +### Ancillary files checks + +Performs validation of non-source files within the project. This includes +linting of `appdata` files, `desktop` files, Markdown files, GTK UI builder +files, and shell scripts, as well as ensuring that all relevant build options +are covered in the functional test configuration. + +These checks also require `xvfb` for the GTK UI builder validator to run. + +See `scripts/test-ancillary-files.sh` for more details. diff --git a/meson.build b/meson.build index 21d4c3ae..20f13768 100644 --- a/meson.build +++ b/meson.build @@ -676,7 +676,7 @@ configure_file(input: 'geeqie.spec.in', output: 'geeqie.spec', configuration: co # is_parallel false is to avoid problems with images tests xvfb = find_program('xvfb-run', required : false) if xvfb.found() - test('Basic test', xvfb, args: ['--auto-servernum', geeqie_exe, '--version'], is_parallel : false, timeout: 100) + test('Basic test', xvfb, args: ['--auto-servernum', geeqie_exe, '--version'], is_parallel : false, timeout: 100, suite: 'functional') summary({'xvfb' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) else summary({'xvfb' : ['Test runs:', false]}, section : 'Testing', bool_yn : true) @@ -705,9 +705,9 @@ if option.enabled() image_name = path_array[path_array.length() - 1] if image_name.startswith('fail') - test('Image_ ' + image_name, image_test_sh, args: [geeqie_exe, image], is_parallel : false, should_fail : true, timeout: 100) + test('Image_ ' + image_name, image_test_sh, args: [geeqie_exe, image], is_parallel : false, should_fail : true, timeout: 100, suite: 'functional') else - test('Image_ ' + image_name, image_test_sh, args: [geeqie_exe, image], is_parallel : false, timeout: 100) + test('Image_ ' + image_name, image_test_sh, args: [geeqie_exe, image], is_parallel : false, timeout: 100, suite: 'functional') endif endforeach summary({'Image tests' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) @@ -729,7 +729,7 @@ if running_from_git source_file_name = fs.name(source_file) config_file = join_paths(meson.project_source_root(), '.clang-tidy') - test('Code Correctness_ ' + source_file_name, clang_tidy_exe, args : ['-p', './build', '-quiet', '--config-file', config_file, source_file], timeout : 100) + test('Code Correctness_ ' + source_file_name, clang_tidy_exe, args : ['-p', './build', '-quiet', '--config-file', config_file, source_file], timeout : 100, suite : 'analysis') endif endforeach @@ -746,7 +746,7 @@ enum_check_sh = find_program('enum-check.sh', dirs : scriptsdir, required : true if enum_check_sh.found() foreach source_file : main_sources + pan_view_sources + view_file_sources source_file_name = fs.name(source_file) - test('Single Value enum_ ' + source_file_name, enum_check_sh, args : [source_file], timeout : 100) + test('Single Value enum_ ' + source_file_name, enum_check_sh, args : [source_file], timeout : 100, suite : 'analysis') endforeach summary({'Single Value enum' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) @@ -760,7 +760,7 @@ if debug_check_sh.found() foreach source_file : main_sources + pan_view_sources + view_file_sources source_file_name = fs.name(source_file) if (source_file_name != 'debug.h') - test('Debug Statements_ ' + source_file_name, debug_check_sh, args : [source_file], timeout : 100) + test('Debug Statements_ ' + source_file_name, debug_check_sh, args : [source_file], timeout : 100, suite : 'analysis') endif endforeach @@ -775,7 +775,7 @@ if tmp_comments_check_sh.found() foreach source_file : main_sources + pan_view_sources + view_file_sources source_file_name = fs.name(source_file) if (source_file_name != 'debug.h') - test('Temporary Comments_ ' + source_file_name, tmp_comments_check_sh, args : [source_file], timeout : 100) + test('Temporary Comments_ ' + source_file_name, tmp_comments_check_sh, args : [source_file], timeout : 100, suite : 'analysis') endif endforeach @@ -790,7 +790,7 @@ if untranslated_text_sh.found() foreach source_file : main_sources + pan_view_sources + view_file_sources if fs.name(source_file).endswith('.cc') source_file_name = fs.name(source_file) - test('Untranslated Text_ ' + source_file_name, untranslated_text_sh, args : [source_file], timeout : 200) + test('Untranslated Text_ ' + source_file_name, untranslated_text_sh, args : [source_file], timeout : 200, suite : 'analysis') endif endforeach @@ -805,7 +805,7 @@ if not option.disabled() if lua_dep.found() if xvfb.found() lua_test_sh = find_program('lua-test.sh', dirs : scriptsdir, required : true) - test('Lua test', lua_test_sh, args: [geeqie_exe], is_parallel : false, timeout: 100) + test('Lua test', lua_test_sh, args: [geeqie_exe], is_parallel : false, timeout: 100, suite : 'analysis') summary({'lua' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) else @@ -820,13 +820,13 @@ endif # Ancillary files test test_ancillary_files_sh = find_program('test-ancillary-files.sh', dirs : scriptsdir, required : true) -test('Ancillary files', test_ancillary_files_sh, args: [meson.current_source_dir()], timeout: 100) +test('Ancillary files', test_ancillary_files_sh, args: [meson.current_source_dir()], timeout: 100, suite : 'analysis') summary({'Ancillary files' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) # Unit tests if conf_data.get('ENABLE_UNIT_TESTS', 0) == 1 - test('Unit tests', geeqie_exe, args: ['--run-unit-tests']) + test('Unit tests', geeqie_exe, args: ['--run-unit-tests'], suite : 'unit') summary({'unit_tests' : ['Tests run:', true]}, section : 'Testing', bool_yn : true) else summary({'unit_tests' : ['Tests run:', false]}, section : 'Testing', bool_yn : true) -- 2.20.1 From 468bc433ac875fb08af0fc1e4932058400a5d947 Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Sat, 11 May 2024 00:33:38 +0000 Subject: [PATCH 06/16] Adds support to use the system gtest/gmock as a default, if available, and the subproject versions otherwise. This follows the pattern from: https://github.com/mesonbuild/meson/blob/master/test%20cases/frameworks/2%20gtest/meson.build --- meson.build | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index 20f13768..72b6986e 100644 --- a/meson.build +++ b/meson.build @@ -620,9 +620,17 @@ subdir('plugins') conditional_unit_test_deps = [] if conf_data.get('ENABLE_UNIT_TESTS', 0) == 1 - gtest_proj = subproject('gtest') - conditional_unit_test_deps += gtest_proj.get_variable('gtest_dep') - conditional_unit_test_deps += gtest_proj.get_variable('gmock_dep') + system_gtest_dep = dependency('gtest', main : false, required : false) + system_gmock_dep = dependency('gmock', required : false) + if system_gtest_dep.found() and system_gmock_dep.found() + conditional_unit_test_deps += system_gtest_dep + conditional_unit_test_deps += system_gmock_dep + else + # Use the subproject gtest as a fallback. + gtest_subproj = subproject('gtest') + conditional_unit_test_deps += gtest_subproj.get_variable('gtest_dep') + conditional_unit_test_deps += gtest_subproj.get_variable('gmock_dep') + endif endif # Generate the executable -- 2.20.1 From 5a3ef63bb7347c941bcd8c6b931680c6f98194b1 Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Sat, 11 May 2024 02:10:19 +0000 Subject: [PATCH 07/16] Creates a helper utility to run tests isolated from the user's actual environment. This avoids interference from/with other processes that might be running on the same host machine (such as the user's own Geeqie processes, or other processes that were started in parallel by Meson), and also ensures that we can clean up the test environment without making any permanent changes (especially important in case of bugs). --- scripts/isolate-test.sh | 64 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100755 scripts/isolate-test.sh diff --git a/scripts/isolate-test.sh b/scripts/isolate-test.sh new file mode 100755 index 00000000..c034a52a --- /dev/null +++ b/scripts/isolate-test.sh @@ -0,0 +1,64 @@ +#!/bin/sh +#********************************************************************** +# Copyright (C) 2024 - The Geeqie Team +# +# Author: Omari Stephens +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +#********************************************************************** + +## @file +## @brief Isolates the test from the rest of the environment. The goal is to +## make the test more reliable, and to avoid interrupting other processes +## that might be running on the host. Passes all args through and passes +## the return code back. +## +## $1 Test executable +## +## + +set -e + +TEST_HOME=$(mktemp -d) + +if [ -z "$TEST_HOME" ]; then + echo "Failed to create temporary home directory." >&2 + exit 1 +fi + +if [ "$TEST_HOME" = "$HOME" ]; then + # This both breaks isolation, and makes automatic cleanup extremely dangerous. + echo "Temporary homedir ($TEST_HOME) is the same as the actual homedir ($HOME)" >&2 + exit 1 +fi + +# Automatically clean up the temporary home directory on exit. +teardown() { + # echo "Cleaning up temporary homedir $TEST_HOME" >&2 + rm -rf "$TEST_HOME" +} +trap teardown EXIT + +export HOME="$TEST_HOME" +export XDG_CONFIG_HOME="${HOME}/.config" + +# Change to temporary homedir and ensure that XDG_CONFIG_HOME exists. +cd +mkdir -p "$XDG_CONFIG_HOME" + +# This will automatically pass the command name and args in the expected order. +# And `set -e` (above) means that we'll automatically exit with the same return +# code as our sub-command. +env -i HOME="$HOME" XDG_CONFIG_HOME="$XDG_CONFIG_HOME" "$@" -- 2.20.1 From a6613547d07a88cd78d1c6cb6e1fb05348eff4b0 Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Sat, 11 May 2024 05:19:11 +0000 Subject: [PATCH 08/16] Updates image-test.sh to include a more robust teardown, and to make it more independent of the rest of the system. In particular, avoids `pgrep` (which could detect geeqie processes unrelated to this test) in favor of stashing and checking the pid that was launched. Also makes some more minor fixups to lua-test.sh --- scripts/image-test.sh | 40 +++++++++++++++++++++++++++++++--------- scripts/lua-test.sh | 8 +++++--- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/scripts/image-test.sh b/scripts/image-test.sh index 2f36cb32..e7324fad 100755 --- a/scripts/image-test.sh +++ b/scripts/image-test.sh @@ -27,32 +27,55 @@ ## ## -xvfb-run --auto-servernum "$1" "$2" & +geeqie_exe="$1" +test_image="$2" + +xvfb-run --auto-servernum "$geeqie_exe" "$test_image" & + +xvfb_pid="$!" if [ -z "$XDG_CONFIG_HOME" ] then - config_home="$HOME/.config" + config_home="${HOME}/.config" else config_home="$XDG_CONFIG_HOME" fi +command_fifo="${config_home}/geeqie/.command" # Wait for remote to initialize -while [ ! -e "$config_home/geeqie/.command" ] ; +while [ ! -e "$command_fifo" ] ; do sleep 1 done +# We make sure Geeqie can stay alive for 2 seconds after initialization. sleep 2 -# Check if Geeqie crashed -if ! pgrep geeqie +# Check if Geeqie crashed (which would cause xvfb-run to terminate) +if ! ps "$xvfb_pid" >/dev/null 2>&1 then - rm "$config_home/geeqie/.command" exit 1 fi -result=$(xvfb-run --auto-servernum "$1" --remote --get-file-info) -xvfb-run --auto-servernum "$1" --remote --quit +result=$(xvfb-run --auto-servernum "$geeqie_exe" --remote --get-file-info) + +## Teardown: various increasingly-forceful attempts to kill the running geeqie process. +xvfb-run --auto-servernum "$geeqie_exe" --remote --quit + +sleep 1 + +if ps "$xvfb_pid" >/dev/null 2>&1 +then + echo "Quit command for xvfb geeqie failed for pid ${xvfb_pid}; sending sigterm" >&2 + kill -TERM "$xvfb_pid" + + sleep 1 + if ps "$xvfb_pid" >/dev/null 2>&1 + then + echo "kill -TERM failed to stop pid ${xvfb_pid}; sending sigkill" >&2 + kill -KILL "$xvfb_pid" + fi +fi if echo "$result" | grep -q "Class: Unknown" then @@ -60,4 +83,3 @@ then else exit 0 fi - diff --git a/scripts/lua-test.sh b/scripts/lua-test.sh index 37d94f4e..5b953e80 100755 --- a/scripts/lua-test.sh +++ b/scripts/lua-test.sh @@ -27,6 +27,8 @@ ## Create a basic image and run all lua built-in functions on it. ## The image file and the Lua test file are created within this script. +geeqie_exe="$1" + if [ -z "$XDG_CONFIG_HOME" ] then config_home="$HOME/.config" @@ -58,7 +60,7 @@ return ret " printf "%s" "$lua_test" > "$lua_test_file" -xvfb-run --auto-servernum "$1" & +xvfb-run --auto-servernum "$geeqie_exe" & # Wait for remote to initialize while [ ! -e "$config_home/geeqie/.command" ] ; @@ -69,8 +71,8 @@ done sleep 2 base_lua=$(basename "$lua_test_file") -result=$(xvfb-run --auto-servernum "$1" --remote --lua="$lua_test_image","$base_lua") -xvfb-run --auto-servernum "$1" --remote --quit +result=$(xvfb-run --auto-servernum "$geeqie_exe" --remote --lua="$lua_test_image","$base_lua") +xvfb-run --auto-servernum "$geeqie_exe" --remote --quit ## @FIXME Running on GitHub gives additional dbind-WARNINGs. The data required is the last n lines. result_tail=$(printf "%s" "$result" | tail --lines=7) -- 2.20.1 From 9f4787026fdbc9531080ca7c097c402db498f864 Mon Sep 17 00:00:00 2001 From: Omari Stephens Date: Sat, 11 May 2024 05:21:16 +0000 Subject: [PATCH 09/16] Switches to using isolate-test.sh everywhere we launch Geeqie from tests/checks. This allows us to run all of those tests in parallel, and without potentially interacting with processes that might be running outside of the build/test environment. --- meson.build | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/meson.build b/meson.build index 72b6986e..fbe227e6 100644 --- a/meson.build +++ b/meson.build @@ -680,11 +680,13 @@ i18n.merge_file( configure_file(input: 'geeqie.spec.in', output: 'geeqie.spec', configuration: conf_data) +isolate_test_sh = find_program('isolate-test.sh', dirs : scriptsdir, required : true) + # Basic test of the executable -# is_parallel false is to avoid problems with images tests xvfb = find_program('xvfb-run', required : false) if xvfb.found() - test('Basic test', xvfb, args: ['--auto-servernum', geeqie_exe, '--version'], is_parallel : false, timeout: 100, suite: 'functional') + test_cmd = [xvfb.full_path(), '--auto-servernum', geeqie_exe.full_path(), '--version'] + test('Basic test', isolate_test_sh, args: test_cmd, timeout: 100, suite: 'functional') summary({'xvfb' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) else summary({'xvfb' : ['Test runs:', false]}, section : 'Testing', bool_yn : true) @@ -712,11 +714,9 @@ if option.enabled() path_array = image.split('/') image_name = path_array[path_array.length() - 1] - if image_name.startswith('fail') - test('Image_ ' + image_name, image_test_sh, args: [geeqie_exe, image], is_parallel : false, should_fail : true, timeout: 100, suite: 'functional') - else - test('Image_ ' + image_name, image_test_sh, args: [geeqie_exe, image], is_parallel : false, timeout: 100, suite: 'functional') - endif + should_fail = image_name.startswith('fail') + test_cmd = [image_test_sh.full_path(), geeqie_exe.full_path(), image] + test('Image_ ' + image_name, isolate_test_sh, args: test_cmd, should_fail : should_fail, timeout: 100, suite: ['functional', 'image']) endforeach summary({'Image tests' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) else @@ -813,7 +813,7 @@ if not option.disabled() if lua_dep.found() if xvfb.found() lua_test_sh = find_program('lua-test.sh', dirs : scriptsdir, required : true) - test('Lua test', lua_test_sh, args: [geeqie_exe], is_parallel : false, timeout: 100, suite : 'analysis') + test('Lua test', isolate_test_sh, args: [lua_test_sh.full_path(), geeqie_exe.full_path()], timeout: 100, suite : 'analysis') summary({'lua' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) else @@ -834,7 +834,7 @@ summary({'Ancillary files' : ['Test runs:', true]}, section : 'Testing', bool_yn # Unit tests if conf_data.get('ENABLE_UNIT_TESTS', 0) == 1 - test('Unit tests', geeqie_exe, args: ['--run-unit-tests'], suite : 'unit') + test('Unit tests', isolate_test_sh, args: [geeqie_exe.full_path(), '--run-unit-tests'], suite : 'unit') summary({'unit_tests' : ['Tests run:', true]}, section : 'Testing', bool_yn : true) else summary({'unit_tests' : ['Tests run:', false]}, section : 'Testing', bool_yn : true) -- 2.20.1 From 7f162e09cada7e477db634515957bec81e6e356d Mon Sep 17 00:00:00 2001 From: Colin Clark Date: Tue, 14 May 2024 11:58:50 +0100 Subject: [PATCH 10/16] test 1 --- .github/workflows/check-build-actions.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/check-build-actions.yml b/.github/workflows/check-build-actions.yml index fbef1c5e..a67195af 100644 --- a/.github/workflows/check-build-actions.yml +++ b/.github/workflows/check-build-actions.yml @@ -117,7 +117,9 @@ jobs: with: action: test directory: build - setup-options: -Ddevel=enabled + setup-options: > + -Ddevel=enabled + -Dunit_tests=enabled options: --verbose meson-version: 1.0.0 - name: Upload logs -- 2.20.1 From 561144a17679c24eae53dd3e1472f7d2350f4e2c Mon Sep 17 00:00:00 2001 From: Colin Clark Date: Tue, 14 May 2024 12:53:55 +0100 Subject: [PATCH 11/16] Include unit_tests option in test runs Previous "test 1" commit should have been here. --- scripts/test-all.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/test-all.sh b/scripts/test-all.sh index 6d65bab5..4ce9ee3a 100755 --- a/scripts/test-all.sh +++ b/scripts/test-all.sh @@ -77,7 +77,7 @@ cp ./build/meson-logs/testlog.txt "$tmpdir/testlog-options-disabled.txt" rm --recursive --force build -meson setup -Ddevel=enabled build +meson setup -Ddevel=enabled -Dunit_tests=enabled build meson test -C build -- 2.20.1 From 1a43535282e5de44f1641a552ae81a6ad9d6b0b4 Mon Sep 17 00:00:00 2001 From: Colin Clark Date: Tue, 14 May 2024 14:39:57 +0100 Subject: [PATCH 12/16] Bug fix: rotate plugin Remove junk from the script file. --- plugins/rotate/geeqie-rotate | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/rotate/geeqie-rotate b/plugins/rotate/geeqie-rotate index b92906e8..703ba91c 100755 --- a/plugins/rotate/geeqie-rotate +++ b/plugins/rotate/geeqie-rotate @@ -10,7 +10,7 @@ GQ_METADATA_DIR="$HOME/.local/share/geeqie/metadata" gq_exiftran() { - if ! [ -x "$(command -v exiftranx)" ] + if ! [ -x "$(command -v exiftran)" ] then zenity --title="Geeqie rotate" --info --width=200 --text="Exiftran is not installed" --window-icon=/usr/local/share/pixmaps/geeqie.png 2> /dev/null exit 0 @@ -21,7 +21,7 @@ gq_exiftran() gq_exiv2() { - if ! [ -x "$(command -v exiv2x)" ] + if ! [ -x "$(command -v exiv2)" ] then zenity --title="Geeqie rotate" --info --width=200 --text="Exiv2 is not installed" --window-icon=/usr/local/share/pixmaps/geeqie.png 2> /dev/null exit 0 @@ -32,7 +32,7 @@ gq_exiv2() gq_mogrify() { - if ! [ -x "$(command -v mogrifyx)" ] + if ! [ -x "$(command -v mogrify)" ] then zenity --title="Geeqie rotate" --info --width=200 --text="ImageMagick is not installed" --window-icon=/usr/local/share/pixmaps/geeqie.png 2> /dev/null exit 0 -- 2.20.1 From 68904899908e958bf94298ffb38af027c6193368 Mon Sep 17 00:00:00 2001 From: Arkadiy Illarionov Date: Tue, 14 May 2024 21:30:29 +0300 Subject: [PATCH 13/16] Simplify util_clip_region() * use gdk_rectangle_intersect() * convert pointers to references --- src/pan-view/pan-item.cc | 74 +++++++++++++++++----------------- src/pan-view/pan-view.cc | 16 ++++---- src/pixbuf-util.cc | 85 ++++++++++++++++++---------------------- src/pixbuf-util.h | 4 +- src/tests/pixbuf-util.cc | 16 ++++---- 5 files changed, 94 insertions(+), 101 deletions(-) diff --git a/src/pan-view/pan-item.cc b/src/pan-view/pan-item.cc index 13b4dec0..4a10b8e4 100644 --- a/src/pan-view/pan-item.cc +++ b/src/pan-view/pan-item.cc @@ -217,42 +217,42 @@ gint pan_item_box_draw(PanWindow *, PanItem *pi, GdkPixbuf *pixbuf, PixbufRender } if (util_clip_region(x, y, width, height, - pi->x, pi->y, bw, bh, - &rx, &ry, &rw, &rh)) + pi->x, pi->y, bw, bh, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, pi->color.r, pi->color.g, pi->color.b, pi->color.a); } if (util_clip_region(x, y, width, height, - pi->x, pi->y, bw, pi->border, - &rx, &ry, &rw, &rh)) + pi->x, pi->y, bw, pi->border, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, pi->color2.r, pi->color2.g, pi->color2.b, pi->color2.a); } if (util_clip_region(x, y, width, height, - pi->x, pi->y + pi->border, pi->border, bh - pi->border * 2, - &rx, &ry, &rw, &rh)) + pi->x, pi->y + pi->border, pi->border, bh - pi->border * 2, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, pi->color2.r, pi->color2.g, pi->color2.b, pi->color2.a); } if (util_clip_region(x, y, width, height, - pi->x + bw - pi->border, pi->y + pi->border, - pi->border, bh - pi->border * 2, - &rx, &ry, &rw, &rh)) + pi->x + bw - pi->border, pi->y + pi->border, + pi->border, bh - pi->border * 2, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, pi->color2.r, pi->color2.g, pi->color2.b, pi->color2.a); } if (util_clip_region(x, y, width, height, - pi->x, pi->y + bh - pi->border, - bw, pi->border, - &rx, &ry, &rw, &rh)) + pi->x, pi->y + bh - pi->border, + bw, pi->border, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, @@ -319,8 +319,8 @@ gint pan_item_tri_draw(PanWindow *, PanItem *pi, GdkPixbuf *pixbuf, PixbufRender gint rh; if (util_clip_region(x, y, width, height, - pi->x, pi->y, pi->width, pi->height, - &rx, &ry, &rw, &rh) && pi->data) + pi->x, pi->y, pi->width, pi->height, + rx, ry, rw, rh) && pi->data) { auto coord = static_cast(pi->data); pixbuf_draw_triangle(pixbuf, @@ -504,8 +504,8 @@ gint pan_item_thumb_draw(PanWindow *pw, PanItem *pi, GdkPixbuf *pixbuf, PixbufRe if (gdk_pixbuf_get_has_alpha(pi->pixbuf)) { if (util_clip_region(x, y, width, height, - tx + PAN_SHADOW_OFFSET, ty + PAN_SHADOW_OFFSET, tw, th, - &rx, &ry, &rw, &rh)) + tx + PAN_SHADOW_OFFSET, ty + PAN_SHADOW_OFFSET, tw, th, + rx, ry, rw, rh)) { pixbuf_draw_shadow(pixbuf, rx - x, ry - y, rw, rh, @@ -517,9 +517,9 @@ gint pan_item_thumb_draw(PanWindow *pw, PanItem *pi, GdkPixbuf *pixbuf, PixbufRe else { if (util_clip_region(x, y, width, height, - tx + tw, ty + PAN_SHADOW_OFFSET, - PAN_SHADOW_OFFSET, th - PAN_SHADOW_OFFSET, - &rx, &ry, &rw, &rh)) + tx + tw, ty + PAN_SHADOW_OFFSET, + PAN_SHADOW_OFFSET, th - PAN_SHADOW_OFFSET, + rx, ry, rw, rh)) { pixbuf_draw_shadow(pixbuf, rx - x, ry - y, rw, rh, @@ -528,8 +528,8 @@ gint pan_item_thumb_draw(PanWindow *pw, PanItem *pi, GdkPixbuf *pixbuf, PixbufRe PAN_SHADOW_COLOR, PAN_SHADOW_ALPHA); } if (util_clip_region(x, y, width, height, - tx + PAN_SHADOW_OFFSET, ty + th, tw, PAN_SHADOW_OFFSET, - &rx, &ry, &rw, &rh)) + tx + PAN_SHADOW_OFFSET, ty + th, tw, PAN_SHADOW_OFFSET, + rx, ry, rw, rh)) { pixbuf_draw_shadow(pixbuf, rx - x, ry - y, rw, rh, @@ -540,8 +540,8 @@ gint pan_item_thumb_draw(PanWindow *pw, PanItem *pi, GdkPixbuf *pixbuf, PixbufRe } if (util_clip_region(x, y, width, height, - tx, ty, tw, th, - &rx, &ry, &rw, &rh)) + tx, ty, tw, th, + rx, ry, rw, rh)) { gdk_pixbuf_composite(pi->pixbuf, pixbuf, rx - x, ry - y, rw, rh, static_cast(tx) - x, @@ -551,34 +551,34 @@ gint pan_item_thumb_draw(PanWindow *pw, PanItem *pi, GdkPixbuf *pixbuf, PixbufRe } if (util_clip_region(x, y, width, height, - tx, ty, tw, PAN_OUTLINE_THICKNESS, - &rx, &ry, &rw, &rh)) + tx, ty, tw, PAN_OUTLINE_THICKNESS, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, PAN_OUTLINE_COLOR_1); } if (util_clip_region(x, y, width, height, - tx, ty, PAN_OUTLINE_THICKNESS, th, - &rx, &ry, &rw, &rh)) + tx, ty, PAN_OUTLINE_THICKNESS, th, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, PAN_OUTLINE_COLOR_1); } if (util_clip_region(x, y, width, height, - tx + tw - PAN_OUTLINE_THICKNESS, ty + PAN_OUTLINE_THICKNESS, - PAN_OUTLINE_THICKNESS, th - PAN_OUTLINE_THICKNESS, - &rx, &ry, &rw, &rh)) + tx + tw - PAN_OUTLINE_THICKNESS, ty + PAN_OUTLINE_THICKNESS, + PAN_OUTLINE_THICKNESS, th - PAN_OUTLINE_THICKNESS, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, PAN_OUTLINE_COLOR_2); } if (util_clip_region(x, y, width, height, - tx + PAN_OUTLINE_THICKNESS, ty + th - PAN_OUTLINE_THICKNESS, - tw - PAN_OUTLINE_THICKNESS * 2, PAN_OUTLINE_THICKNESS, - &rx, &ry, &rw, &rh)) + tx + PAN_OUTLINE_THICKNESS, ty + th - PAN_OUTLINE_THICKNESS, + tw - PAN_OUTLINE_THICKNESS * 2, PAN_OUTLINE_THICKNESS, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, @@ -593,8 +593,8 @@ gint pan_item_thumb_draw(PanWindow *pw, PanItem *pi, GdkPixbuf *pixbuf, PixbufRe ty = pi->y + PAN_SHADOW_OFFSET; if (util_clip_region(x, y, width, height, - tx, ty, tw, th, - &rx, &ry, &rw, &rh)) + tx, ty, tw, th, + rx, ry, rw, rh)) { gint d; @@ -677,8 +677,8 @@ gint pan_item_image_draw(PanWindow *, PanItem *pi, GdkPixbuf *pixbuf, PixbufRend gint rh; if (util_clip_region(x, y, width, height, - pi->x, pi->y, pi->width, pi->height, - &rx, &ry, &rw, &rh)) + pi->x, pi->y, pi->width, pi->height, + rx, ry, rw, rh)) { if (pi->pixbuf) { diff --git a/src/pan-view/pan-view.cc b/src/pan-view/pan-view.cc index aade3be3..4e0b8a71 100644 --- a/src/pan-view/pan-view.cc +++ b/src/pan-view/pan-view.cc @@ -382,8 +382,8 @@ static gboolean pan_window_request_tile_cb(PixbufRenderer *pr, gint x, gint y, gint rh; if (util_clip_region(x, y, width, height, - i, y, 1, height, - &rx, &ry, &rw, &rh)) + i, y, 1, height, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, @@ -398,8 +398,8 @@ static gboolean pan_window_request_tile_cb(PixbufRenderer *pr, gint x, gint y, gint rh; if (util_clip_region(x, y, width, height, - x, i, width, 1, - &rx, &ry, &rw, &rh)) + x, i, width, 1, + rx, ry, rw, rh)) { pixbuf_draw_rect_fill(pixbuf, rx - x, ry - y, rw, rh, @@ -829,8 +829,8 @@ static void pan_grid_build(PanWindow *pw, gint width, gint height, gint grid_siz grid = grid->next; if (util_clip_region(pi->x, pi->y, pi->width, pi->height, - pg->x, pg->y, pg->w, pg->h, - &rx, &ry, &rw, &rh)) + pg->x, pg->y, pg->w, pg->h, + rx, ry, rw, rh)) { pg->list = g_list_prepend(pg->list, pi); } @@ -986,8 +986,8 @@ static GList *pan_layout_intersect_l(GList *list, GList *item_list, work = work->next; if (util_clip_region(x, y, width, height, - pi->x, pi->y, pi->width, pi->height, - &rx, &ry, &rw, &rh)) + pi->x, pi->y, pi->width, pi->height, + rx, ry, rw, rh)) { list = g_list_prepend(list, pi); } diff --git a/src/pixbuf-util.cc b/src/pixbuf-util.cc index 3e2a2572..0735a047 100644 --- a/src/pixbuf-util.cc +++ b/src/pixbuf-util.cc @@ -395,30 +395,23 @@ GdkPixbuf *pixbuf_fallback(FileData *fd, gint requested_width, gint requested_he */ gboolean util_clip_region(gint x, gint y, gint w, gint h, - gint clip_x, gint clip_y, gint clip_w, gint clip_h, - gint *rx, gint *ry, gint *rw, gint *rh) + gint clip_x, gint clip_y, gint clip_w, gint clip_h, + gint &rx, gint &ry, gint &rw, gint &rh) { - // Ensures that clip region and main region have some overlap (they aren't - // completely disjoint). - if (clip_x + clip_w <= x || /* assert(x < clip_right) && */ - clip_x >= x + w || /* assert(clip_x < right) && */ - clip_y + clip_h <= y || /* assert(y < clip_bottom && */ - clip_y >= y + h) /* assert(bottom < clip_y) */ + GdkRectangle main{x, y, w, h}; + GdkRectangle clip{clip_x, clip_y, clip_w, clip_h}; + GdkRectangle r; + + const gboolean rectangles_intersect = gdk_rectangle_intersect(&main, &clip, &r); + if (rectangles_intersect) { - return FALSE; + rx = r.x; + ry = r.y; + rw = r.width; + rh = r.height; } - // We choose the right-most x coordinate. - *rx = MAX(x, clip_x); - // And the narrowest width. - *rw = MIN((x + w), (clip_x + clip_w)) - *rx; - - // We choose the bottom-most y coordinate. - *ry = MAX(y, clip_y); - // And the shortest height. - *rh = MIN((y + h), (clip_y + clip_h)) - *ry; - - return TRUE; + return rectangles_intersect; } /* @@ -1070,8 +1063,8 @@ void pixbuf_draw_triangle(GdkPixbuf *pb, // Intersects the clip region with the pixbuf. r{x,y,w,h} is that // intersecting region. if (!util_clip_region(0, 0, pw, ph, - clip_x, clip_y, clip_w, clip_h, - &rx, &ry, &rw, &rh)) return; + clip_x, clip_y, clip_w, clip_h, + rx, ry, rw, rh)) return; // Determine the bounding box for the triangle. util_clip_triangle(x1, y1, x2, y2, x3, y3, @@ -1079,8 +1072,8 @@ void pixbuf_draw_triangle(GdkPixbuf *pb, // And now clip the triangle bounding box to the pixbuf clipping region. if (!util_clip_region(rx, ry, rw, rh, - tx, ty, tw, th, - &fx1, &fy1, &fw, &fh)) return; + tx, ty, tw, th, + fx1, fy1, fw, fh)) return; fx2 = fx1 + fw; fy2 = fy1 + fh; @@ -1322,8 +1315,8 @@ void pixbuf_draw_line(GdkPixbuf *pb, // Intersects the clip region with the pixbuf. r{x,y,w,h} is that // intersecting region. if (!util_clip_region(0, 0, pw, ph, - clip_x, clip_y, clip_w, clip_h, - &rx, &ry, &rw, &rh)) return; + clip_x, clip_y, clip_w, clip_h, + rx, ry, rw, rh)) return; // TODO(xsdg): These explicit casts are unnecessary and harm readability. // Clips the specified line segment to the intersecting region from above. if (!util_clip_line(static_cast(rx), static_cast(ry), static_cast(rw), static_cast(rh), @@ -1528,8 +1521,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, // Intersects the clip region with the pixbuf. r{x,y,w,h} is that // intersecting region. if (!util_clip_region(0, 0, pw, ph, - clip_x, clip_y, clip_w, clip_h, - &rx, &ry, &rw, &rh)) return; + clip_x, clip_y, clip_w, clip_h, + rx, ry, rw, rh)) return; has_alpha = gdk_pixbuf_get_has_alpha(pb); prs = gdk_pixbuf_get_rowstride(pb); @@ -1539,8 +1532,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, // as contracted by `border` pixels, with a composition fraction that's defined // by the supplied `a` parameter. if (util_clip_region(x + border, y + border, w - border * 2, h - border * 2, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_rect_fill(pb, fx, fy, fw, fh, r, g, b, a); } @@ -1549,8 +1542,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, // Draws linear gradients along each of the 4 edges. if (util_clip_region(x, y + border, border, h - border * 2, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_fade_linear(p_pix, prs, has_alpha, x + border, TRUE, border, @@ -1558,8 +1551,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, r, g, b, a); } if (util_clip_region(x + w - border, y + border, border, h - border * 2, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_fade_linear(p_pix, prs, has_alpha, x + w - border, TRUE, border, @@ -1567,8 +1560,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, r, g, b, a); } if (util_clip_region(x + border, y, w - border * 2, border, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_fade_linear(p_pix, prs, has_alpha, y + border, FALSE, border, @@ -1576,8 +1569,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, r, g, b, a); } if (util_clip_region(x + border, y + h - border, w - border * 2, border, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_fade_linear(p_pix, prs, has_alpha, y + h - border, FALSE, border, @@ -1586,8 +1579,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, } // Draws radial gradients at each of the 4 corners. if (util_clip_region(x, y, border, border, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_fade_radius(p_pix, prs, has_alpha, x + border, y + border, border, @@ -1595,8 +1588,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, r, g, b, a); } if (util_clip_region(x + w - border, y, border, border, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_fade_radius(p_pix, prs, has_alpha, x + w - border, y + border, border, @@ -1604,8 +1597,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, r, g, b, a); } if (util_clip_region(x, y + h - border, border, border, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_fade_radius(p_pix, prs, has_alpha, x + border, y + h - border, border, @@ -1613,8 +1606,8 @@ void pixbuf_draw_shadow(GdkPixbuf *pb, r, g, b, a); } if (util_clip_region(x + w - border, y + h - border, border, border, - rx, ry, rw, rh, - &fx, &fy, &fw, &fh)) + rx, ry, rw, rh, + fx, fy, fw, fh)) { pixbuf_draw_fade_radius(p_pix, prs, has_alpha, x + w - border, y + h - border, border, diff --git a/src/pixbuf-util.h b/src/pixbuf-util.h index 87ff6add..c86026b0 100644 --- a/src/pixbuf-util.h +++ b/src/pixbuf-util.h @@ -233,8 +233,8 @@ void pixbuf_ignore_alpha_rect(GdkPixbuf *pb, * @retval TRUE The intersection operation was performed, and the output params were set. */ gboolean util_clip_region(gint x, gint y, gint w, gint h, - gint clip_x, gint clip_y, gint clip_w, gint clip_h, - gint *rx, gint *ry, gint *rw, gint *rh); + gint clip_x, gint clip_y, gint clip_w, gint clip_h, + gint &rx, gint &ry, gint &rw, gint &rh); // TODO(xsdg): Rename this function to util_triangle_bounding_box. /** diff --git a/src/tests/pixbuf-util.cc b/src/tests/pixbuf-util.cc index 1fcb1958..af300712 100644 --- a/src/tests/pixbuf-util.cc +++ b/src/tests/pixbuf-util.cc @@ -40,8 +40,8 @@ class ClipRegionTest : public ::testing::Test TEST_F(ClipRegionTest, RegionAContainsRegionB) { ASSERT_TRUE(util_clip_region(0, 0, 1000, 1000, - 50, 50, 100, 100, - &rx, &ry, &rw, &rh)); + 50, 50, 100, 100, + rx, ry, rw, rh)); ASSERT_EQ(50, rx); ASSERT_EQ(50, ry); @@ -52,8 +52,8 @@ TEST_F(ClipRegionTest, RegionAContainsRegionB) TEST_F(ClipRegionTest, RegionBContainsRegionA) { ASSERT_TRUE(util_clip_region(50, 50, 100, 100, - 0, 0, 1000, 1000, - &rx, &ry, &rw, &rh)); + 0, 0, 1000, 1000, + rx, ry, rw, rh)); ASSERT_EQ(50, rx); ASSERT_EQ(50, ry); @@ -64,8 +64,8 @@ TEST_F(ClipRegionTest, RegionBContainsRegionA) TEST_F(ClipRegionTest, PartialOverlapWithBAfterA) { ASSERT_TRUE(util_clip_region(0, 0, 1000, 1000, - 500, 500, 1000, 1000, - &rx, &ry, &rw, &rh)); + 500, 500, 1000, 1000, + rx, ry, rw, rh)); ASSERT_EQ(500, rx); ASSERT_EQ(500, ry); @@ -76,8 +76,8 @@ TEST_F(ClipRegionTest, PartialOverlapWithBAfterA) TEST_F(ClipRegionTest, PartialOverlapWithAAfterB) { ASSERT_TRUE(util_clip_region(500, 500, 1000, 1000, - 0, 0, 1000, 1000, - &rx, &ry, &rw, &rh)); + 0, 0, 1000, 1000, + rx, ry, rw, rh)); ASSERT_EQ(500, rx); ASSERT_EQ(500, ry); -- 2.20.1 From 16d72e231722dd204455d83ed6c529a13f4ee0a1 Mon Sep 17 00:00:00 2001 From: Colin Clark Date: Thu, 16 May 2024 16:47:14 +0100 Subject: [PATCH 14/16] Check for GTK4 migration regressions - Additional test to ensure that GTK4 compatibility functions are prefixed by gq_ - Fix found errors --- TESTING.md | 7 +++ meson.build | 17 ++++++ scripts/gtk4-migration-regression-check.sh | 64 ++++++++++++++++++++++ src/bar-gps.cc | 2 +- src/layout-util.cc | 6 +- src/layout.cc | 2 +- src/logwindow.cc | 8 +-- src/pan-view/pan-view-search.cc | 2 +- src/pan-view/pan-view.cc | 16 +++--- src/ui-pathsel.cc | 2 +- src/view-dir.cc | 2 +- 11 files changed, 108 insertions(+), 20 deletions(-) create mode 100755 scripts/gtk4-migration-regression-check.sh diff --git a/TESTING.md b/TESTING.md index 2d7fdd64..7a8ed186 100644 --- a/TESTING.md +++ b/TESTING.md @@ -121,6 +121,13 @@ Checks for comments starting with `//~` in the source tree. See `scripts/temporary-comments-check.sh` for more details. +### GTK4 migration regression checks + +Checks that gtk functions for which there is a Geeqie GTK4 compatibility +function have a `gq_` prefix. + +See `scripts/gtk4-migration-regression-check.sh` for more details. + ### Untranslated text checks Checks for strings that haven't been marked for translation (starting with `_(`) diff --git a/meson.build b/meson.build index fbe227e6..6048a092 100644 --- a/meson.build +++ b/meson.build @@ -792,6 +792,23 @@ else summary({'Temporary Comments' : ['Test runs:', false]}, section : 'Testing', bool_yn : true) endif +# GTK4 migration regression checks +gtk4_migration_check_sh = find_program('gtk4-migration-regression-check.sh', dirs : scriptsdir, required : true) +if gtk4_migration_check_sh.found() + compat_cc = join_paths(meson.project_source_root(), 'src', 'compat.cc') + compat_h = join_paths(meson.project_source_root(), 'src', 'compat.h') + foreach source_file : main_sources + pan_view_sources + view_file_sources + source_file_name = fs.name(source_file) + if (source_file_name != 'debug.h') + test('GTK4 migration_ ' + source_file_name, gtk4_migration_check_sh, args : [source_file, compat_cc, compat_h], timeout : 100, suite : 'analysis') + endif + endforeach + + summary({'GTK4 migration' : ['Test runs:', true]}, section : 'Testing', bool_yn : true) +else + summary({'GTK4 migration' : ['Test runs:', false]}, section : 'Testing', bool_yn : true) +endif + # Untranslated text checks untranslated_text_sh = find_program('untranslated-text.sh', dirs : scriptsdir, required : true) if untranslated_text_sh.found() diff --git a/scripts/gtk4-migration-regression-check.sh b/scripts/gtk4-migration-regression-check.sh new file mode 100755 index 00000000..6546d6d1 --- /dev/null +++ b/scripts/gtk4-migration-regression-check.sh @@ -0,0 +1,64 @@ +#!/bin/sh + +#********************************************************************** +# Copyright (C) 2024 - The Geeqie Team +# +# Author: Colin Clark +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +#********************************************************************** + +## @file +## @brief Look for function calls that do not conform to GTK4 migration format +## +## The files compat.cc and compat.h contain functions that should be used instead +## of the standard GTK calls. The compatibility calls are all prefixed by gq_ +## +## Create a list of function names to be looked for by searching +## compat.cc and compat.h for identifiers prefixed by gq_gtk_ +## +## Search the input file for any of the above gtk_ function names that +## are not prefixed by gq_ +## +## $1 full path to file to process +## $2 full path to compat.cc +## $3 full path to compat.h +## + +exit_status=0 + +if [ ! "${1#*compat.cc}" = "$1" ] +then + exit "$exit_status" +fi + +if [ ! "${1#*compat.h}" = "$1" ] +then + exit "$exit_status" +fi + +compat_functions=$(grep --only-matching --no-filename 'gq_gtk_\(\(\([[:alnum:]]*\)\_*\)*\)' "$2" "$3" | sort | uniq | cut --characters 4-) + +while read -r line +do + if grep --line-number --perl-regexp '(?application); g_object_unref(g_list_first(open_with_data->g_file_list)->data); g_list_free(open_with_data->g_file_list); - gtk_widget_destroy(GTK_WIDGET(open_with_data->app_chooser_dialog)); + gq_gtk_widget_destroy(GTK_WIDGET(open_with_data->app_chooser_dialog)); g_free(open_with_data); } @@ -1018,7 +1018,7 @@ static void open_with_application_activated_cb(GtkAppChooserWidget *, GAppInfo * g_object_unref(open_with_data->application); g_object_unref(g_list_first(open_with_data->g_file_list)->data); g_list_free(open_with_data->g_file_list); - gtk_widget_destroy(GTK_WIDGET(open_with_data->app_chooser_dialog)); + gq_gtk_widget_destroy(GTK_WIDGET(open_with_data->app_chooser_dialog)); g_free(open_with_data); } @@ -3374,7 +3374,7 @@ void toolbar_clear_cb(GtkWidget *widget, gpointer) g_signal_handler_disconnect(action, GPOINTER_TO_UINT(g_object_get_data(G_OBJECT(widget), "id"))); } } - gtk_widget_destroy(widget); + gq_gtk_widget_destroy(widget); } void layout_toolbar_clear(LayoutWindow *lw, ToolbarType type) diff --git a/src/layout.cc b/src/layout.cc index 1bec9663..1d9a6471 100644 --- a/src/layout.cc +++ b/src/layout.cc @@ -414,7 +414,7 @@ static GtkWidget *layout_tool_setup(LayoutWindow *lw) gq_gtk_container_add(GTK_WIDGET(scroll_window), menu_toolbar_box); gq_gtk_box_pack_start(GTK_BOX(box), scroll_window, FALSE, FALSE, 0); - gtk_widget_show_all(scroll_window); + gq_gtk_widget_show_all(scroll_window); } else { diff --git a/src/logwindow.cc b/src/logwindow.cc index 19ae7e23..ef3151d2 100644 --- a/src/logwindow.cc +++ b/src/logwindow.cc @@ -450,7 +450,7 @@ static LogWindow *log_window_create(LayoutWindow *lw) gq_gtk_container_add(GTK_WIDGET(logwin->pause), label) ; gq_gtk_box_pack_start(GTK_BOX(hbox),logwin->pause, FALSE, FALSE, 0) ; g_signal_connect(logwin->pause, "toggled", G_CALLBACK(log_window_pause_cb), logwin); - gtk_widget_show_all(logwin->pause); + gq_gtk_widget_show_all(logwin->pause); logwin->wrap = gtk_toggle_button_new(); label = gtk_label_new("Wrap"); @@ -458,7 +458,7 @@ static LogWindow *log_window_create(LayoutWindow *lw) gq_gtk_container_add(GTK_WIDGET(logwin->wrap), label) ; gq_gtk_box_pack_start(GTK_BOX(hbox),logwin->wrap, FALSE, FALSE, 0) ; g_signal_connect(logwin->wrap, "toggled", G_CALLBACK(log_window_line_wrap_cb), logwin); - gtk_widget_show_all(logwin->wrap); + gq_gtk_widget_show_all(logwin->wrap); logwin->timer_data = gtk_toggle_button_new(); label = gtk_label_new(_("Timer")); @@ -470,7 +470,7 @@ static LogWindow *log_window_create(LayoutWindow *lw) gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(logwin->timer_data), TRUE); } g_signal_connect(logwin->timer_data, "toggled", G_CALLBACK(log_window_timer_data_cb), logwin); - gtk_widget_show_all(logwin->timer_data); + gq_gtk_widget_show_all(logwin->timer_data); search_box = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 0); gq_gtk_container_add(GTK_WIDGET(hbox), search_box); @@ -514,7 +514,7 @@ static LogWindow *log_window_create(LayoutWindow *lw) gtk_widget_set_tooltip_text(GTK_WIDGET(all_button), _("Highlight all")); gq_gtk_box_pack_start(GTK_BOX(search_box), all_button, FALSE, FALSE, 0) ; g_signal_connect(all_button, "toggled", G_CALLBACK(all_keypress_event_cb), logwin); - gtk_widget_show_all(all_button); + gq_gtk_widget_show_all(all_button); g_object_unref(pixbuf); pref_label_new(hbox, _("Filter regexp")); diff --git a/src/pan-view/pan-view-search.cc b/src/pan-view/pan-view-search.cc index 5247054b..2ff48ae6 100644 --- a/src/pan-view/pan-view-search.cc +++ b/src/pan-view/pan-view-search.cc @@ -70,7 +70,7 @@ PanViewSearchUi *pan_search_ui_new(PanWindow *pw) gtk_button_set_relief(GTK_BUTTON(ui->search_button), GTK_RELIEF_NONE); gtk_widget_set_focus_on_click(ui->search_button, FALSE); hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, PREF_PAD_GAP); - gtk_container_add(GTK_CONTAINER(ui->search_button), hbox); + gq_gtk_container_add(GTK_WIDGET(ui->search_button), hbox); gtk_widget_show(hbox); ui->search_button_arrow = gtk_image_new_from_icon_name(GQ_ICON_PAN_UP, GTK_ICON_SIZE_BUTTON); gq_gtk_box_pack_start(GTK_BOX(hbox), ui->search_button_arrow, FALSE, FALSE, 0); diff --git a/src/pan-view/pan-view.cc b/src/pan-view/pan-view.cc index 4e0b8a71..f57b0231 100644 --- a/src/pan-view/pan-view.cc +++ b/src/pan-view/pan-view.cc @@ -1902,7 +1902,7 @@ static void pan_window_new_real(FileData *dir_fd) vbox = gtk_box_new(GTK_ORIENTATION_VERTICAL, 0); DEBUG_NAME(vbox); - gtk_container_add(GTK_CONTAINER(pw->window), vbox); + gq_gtk_container_add(GTK_WIDGET(pw->window), vbox); gtk_widget_show(vbox); box = pref_box_new(vbox, FALSE, GTK_ORIENTATION_HORIZONTAL, PREF_PAD_SPACE); @@ -1951,19 +1951,19 @@ static void pan_window_new_real(FileData *dir_fd) vbox_imd_widget = gtk_box_new(GTK_ORIENTATION_VERTICAL, 0); hbox_imd_widget = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, 0); - gtk_box_pack_start(GTK_BOX(vbox_imd_widget), pw->imd->widget, true, true, 0); + gq_gtk_box_pack_start(GTK_BOX(vbox_imd_widget), pw->imd->widget, true, true, 0); pw->scrollbar_h = gtk_scrollbar_new(GTK_ORIENTATION_HORIZONTAL, nullptr); g_signal_connect(G_OBJECT(pw->scrollbar_h), "value_changed", G_CALLBACK(pan_window_scrollbar_h_value_cb), pw); - gtk_box_pack_start(GTK_BOX(vbox_imd_widget), pw->scrollbar_h, false, false, 0); + gq_gtk_box_pack_start(GTK_BOX(vbox_imd_widget), pw->scrollbar_h, false, false, 0); - gtk_box_pack_start(GTK_BOX(hbox_imd_widget), vbox_imd_widget, true, true, 0); + gq_gtk_box_pack_start(GTK_BOX(hbox_imd_widget), vbox_imd_widget, true, true, 0); pw->scrollbar_v = gtk_scrollbar_new(GTK_ORIENTATION_VERTICAL, nullptr); g_signal_connect(G_OBJECT(pw->scrollbar_v), "value_changed", G_CALLBACK(pan_window_scrollbar_v_value_cb), pw); - gtk_box_pack_start(GTK_BOX(hbox_imd_widget), pw->scrollbar_v, false, false, 0); + gq_gtk_box_pack_start(GTK_BOX(hbox_imd_widget), pw->scrollbar_v, false, false, 0); - gtk_box_pack_start(GTK_BOX(vbox), hbox_imd_widget, true, true, 0); + gq_gtk_box_pack_start(GTK_BOX(vbox), hbox_imd_widget, true, true, 0); gtk_widget_show(GTK_WIDGET(hbox_imd_widget)); gtk_widget_show(GTK_WIDGET(pw->imd->widget)); @@ -1997,7 +1997,7 @@ static void pan_window_new_real(FileData *dir_fd) gtk_widget_show(frame); hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, PREF_PAD_SPACE); - gtk_container_add(GTK_CONTAINER(frame), hbox); + gq_gtk_container_add(GTK_WIDGET(frame), hbox); gtk_widget_show(hbox); pref_spacer(hbox, 0); @@ -2011,7 +2011,7 @@ static void pan_window_new_real(FileData *dir_fd) gtk_widget_show(frame); pw->label_zoom = gtk_label_new(""); - gtk_container_add(GTK_CONTAINER(frame), pw->label_zoom); + gq_gtk_container_add(GTK_WIDGET(frame), pw->label_zoom); gtk_widget_show(pw->label_zoom); // Add the "Find" button to the status bar area. diff --git a/src/ui-pathsel.cc b/src/ui-pathsel.cc index 986277d7..31f3ff41 100644 --- a/src/ui-pathsel.cc +++ b/src/ui-pathsel.cc @@ -1035,7 +1035,7 @@ GtkWidget *path_selection_new_with_files(GtkWidget *entry, const gchar *path, gtk_widget_show(dd->hidden_button); gq_gtk_box_pack_start(GTK_BOX(table), hbox1, FALSE, FALSE, 0); - gtk_widget_show_all(hbox1); + gq_gtk_widget_show_all(hbox1); hbox2 = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, PREF_PAD_GAP); if (filter) diff --git a/src/view-dir.cc b/src/view-dir.cc index 25d6fa77..47064c9b 100644 --- a/src/view-dir.cc +++ b/src/view-dir.cc @@ -179,7 +179,7 @@ ViewDir *vd_new(LayoutWindow *lw) case DIRVIEW_TREE: vd = vdtree_new(vd, lw->dir_fd); break; } - gtk_container_add(GTK_CONTAINER(vd->widget), vd->view); + gq_gtk_container_add(GTK_WIDGET(vd->widget), vd->view); vd_dnd_init(vd); -- 2.20.1 From db1325591f9603c16d79b6471848dc86d5e27557 Mon Sep 17 00:00:00 2001 From: Arkadiy Illarionov Date: Wed, 15 May 2024 22:17:31 +0300 Subject: [PATCH 15/16] Use gdk_rectangle_intersect() in pan-view --- src/pan-view/pan-view.cc | 103 ++++++++++++++------------------------- 1 file changed, 36 insertions(+), 67 deletions(-) diff --git a/src/pan-view/pan-view.cc b/src/pan-view/pan-view.cc index f57b0231..ef9acc11 100644 --- a/src/pan-view/pan-view.cc +++ b/src/pan-view/pan-view.cc @@ -368,42 +368,35 @@ static gboolean pan_window_request_tile_cb(PixbufRenderer *pr, gint x, gint y, auto pw = static_cast(data); GList *list; GList *work; - gint i; + const GdkRectangle request_rect{x, y, width, height}; + GdkRectangle pan_grid_rect; + GdkRectangle r; pixbuf_set_rect_fill(pixbuf, 0, 0, width, height, PAN_BACKGROUND_COLOR); - for (i = (x / PAN_GRID_SIZE) * PAN_GRID_SIZE; i < x + width; i += PAN_GRID_SIZE) + pan_grid_rect = request_rect; + pan_grid_rect.width = 1; + for (pan_grid_rect.x = (x / PAN_GRID_SIZE) * PAN_GRID_SIZE; pan_grid_rect.x < x + width; pan_grid_rect.x += PAN_GRID_SIZE) { - gint rx; - gint ry; - gint rw; - gint rh; - - if (util_clip_region(x, y, width, height, - i, y, 1, height, - rx, ry, rw, rh)) + if (gdk_rectangle_intersect(&request_rect, &pan_grid_rect, &r)) { pixbuf_draw_rect_fill(pixbuf, - rx - x, ry - y, rw, rh, - PAN_GRID_COLOR); + r.x - x, r.y - y, r.width, r.height, + PAN_GRID_COLOR); } } - for (i = (y / PAN_GRID_SIZE) * PAN_GRID_SIZE; i < y + height; i += PAN_GRID_SIZE) - { - gint rx; - gint ry; - gint rw; - gint rh; - if (util_clip_region(x, y, width, height, - x, i, width, 1, - rx, ry, rw, rh)) + pan_grid_rect = request_rect; + pan_grid_rect.height = 1; + for (pan_grid_rect.y = (y / PAN_GRID_SIZE) * PAN_GRID_SIZE; pan_grid_rect.y < y + height; pan_grid_rect.y += PAN_GRID_SIZE) + { + if (gdk_rectangle_intersect(&request_rect, &pan_grid_rect, &r)) { pixbuf_draw_rect_fill(pixbuf, - rx - x, ry - y, rw, rh, - PAN_GRID_COLOR); + r.x - x, r.y - y, r.width, r.height, + PAN_GRID_COLOR); } } @@ -759,7 +752,6 @@ static void pan_grid_clear(PanWindow *pw) static void pan_grid_build(PanWindow *pw, gint width, gint height, gint grid_size) { - GList *work; gint col; gint row; gint cw; @@ -807,43 +799,30 @@ static void pan_grid_build(PanWindow *pw, gint width, gint height, gint grid_siz } } - work = pw->list; - while (work) + for (GList *work = pw->list; work; work = work->next) { - PanItem *pi; - GList *grid; + auto *pi = static_cast(work->data); - pi = static_cast(work->data); - work = work->next; + // @todo use GdkRectangle in PanItem + const GdkRectangle pi_rect{pi->x, pi->y, pi->width, pi->height}; - grid = pw->list_grid; - while (grid) + for (GList *grid = pw->list_grid; grid; grid = grid->next) { - PanGrid *pg; - gint rx; - gint ry; - gint rw; - gint rh; + auto *pg = static_cast(grid->data); - pg = static_cast(grid->data); - grid = grid->next; + // @todo use GdkRectangle in PanGrid + const GdkRectangle pg_rect{pg->x, pg->y, pg->w, pg->h}; - if (util_clip_region(pi->x, pi->y, pi->width, pi->height, - pg->x, pg->y, pg->w, pg->h, - rx, ry, rw, rh)) + if (gdk_rectangle_intersect(&pi_rect, &pg_rect, nullptr)) { pg->list = g_list_prepend(pg->list, pi); } } } - work = pw->list_grid; - while (work) + for (GList *grid = pw->list_grid; grid; grid = grid->next) { - PanGrid *pg; - - pg = static_cast(work->data); - work = work->next; + auto *pg = static_cast(grid->data); pg->list = g_list_reverse(pg->list); } @@ -969,25 +948,14 @@ static void pan_layout_compute(PanWindow *pw, FileData *dir_fd, } static GList *pan_layout_intersect_l(GList *list, GList *item_list, - gint x, gint y, gint width, gint height) + const GdkRectangle &rect) { - GList *work; - - work = item_list; - while (work) + for (GList *work = item_list; work; work = work->next) { - PanItem *pi; - gint rx; - gint ry; - gint rw; - gint rh; - - pi = static_cast(work->data); - work = work->next; + auto *pi = static_cast(work->data); + const GdkRectangle pi_rect = {pi->x, pi->y, pi->width, pi->height}; - if (util_clip_region(x, y, width, height, - pi->x, pi->y, pi->width, pi->height, - rx, ry, rw, rh)) + if (gdk_rectangle_intersect(&rect, &pi_rect, nullptr)) { list = g_list_prepend(list, pi); } @@ -1001,6 +969,7 @@ GList *pan_layout_intersect(PanWindow *pw, gint x, gint y, gint width, gint heig GList *list = nullptr; GList *grid; PanGrid *pg = nullptr; + const GdkRectangle rect{x, y, width, height}; grid = pw->list_grid; while (grid && !pg) @@ -1015,15 +984,15 @@ GList *pan_layout_intersect(PanWindow *pw, gint x, gint y, gint width, gint heig } } - list = pan_layout_intersect_l(list, pw->list, x, y, width, height); + list = pan_layout_intersect_l(list, pw->list, rect); if (pg) { - list = pan_layout_intersect_l(list, pg->list, x, y, width, height); + list = pan_layout_intersect_l(list, pg->list, rect); } else { - list = pan_layout_intersect_l(list, pw->list_static, x, y, width, height); + list = pan_layout_intersect_l(list, pw->list_static, rect); } return list; -- 2.20.1 From f12d68b7a20c063f6e290da015c8a965c40150bd Mon Sep 17 00:00:00 2001 From: Arkadiy Illarionov Date: Thu, 16 May 2024 21:33:29 +0300 Subject: [PATCH 16/16] Use util_clip_triangle() in pan_item_tri_new() Remove unused parameters in pan_item_tri_new(). --- src/pan-view/pan-calendar.cc | 10 +++------- src/pan-view/pan-folder.cc | 16 +++------------- src/pan-view/pan-item.cc | 10 ++++------ src/pan-view/pan-item.h | 2 +- src/pan-view/pan-view.cc | 16 +++++----------- 5 files changed, 16 insertions(+), 38 deletions(-) diff --git a/src/pan-view/pan-calendar.cc b/src/pan-view/pan-calendar.cc index e3faa61f..e2a8bbad 100644 --- a/src/pan-view/pan-calendar.cc +++ b/src/pan-view/pan-calendar.cc @@ -88,8 +88,6 @@ void pan_calendar_update(PanWindow *pw, PanItem *pi_day) gint y3; gint x; gint y; - gint w; - gint h; gint grid; gint column; @@ -187,12 +185,10 @@ void pan_calendar_update(PanWindow *pw, PanItem *pi_day) y2 = pbox->y + MIN(42, pbox->height); x3 = pbox->x + 1; y3 = MAX(pbox->y, y2 - 30); - util_clip_triangle(x1, y1, x2, y2, x3, y3, - x, y, w, h); - pi = pan_item_tri_new(pw, nullptr, x, y, w, h, - x1, y1, x2, y2, x3, y3, - PAN_CAL_POPUP_COLOR); + pi = pan_item_tri_new(pw, + x1, y1, x2, y2, x3, y3, + PAN_CAL_POPUP_COLOR); pan_item_tri_border(pi, PAN_BORDER_1 | PAN_BORDER_3, PAN_CAL_POPUP_BORDER_COLOR); pan_item_set_key(pi, "day_bubble"); pan_item_added(pw, pi); diff --git a/src/pan-view/pan-folder.cc b/src/pan-view/pan-folder.cc index 9e3eff30..65485fe3 100644 --- a/src/pan-view/pan-folder.cc +++ b/src/pan-view/pan-folder.cc @@ -183,10 +183,6 @@ static void pan_flower_build(PanWindow *pw, FlowerGroup *group, FlowerGroup *par gint py; gint gx; gint gy; - gint x1; - gint y1; - gint x2; - gint y2; px = parent->x + parent->width / 2; py = parent->y + parent->height / 2; @@ -194,15 +190,9 @@ static void pan_flower_build(PanWindow *pw, FlowerGroup *group, FlowerGroup *par gx = group->x + group->width / 2; gy = group->y + group->height / 2; - x1 = MIN(px, gx); - y1 = MIN(py, gy); - - x2 = MAX(px, gx + 5); - y2 = MAX(py, gy + 5); - - pi = pan_item_tri_new(pw, nullptr, x1, y1, x2 - x1, y2 - y1, - px, py, gx, gy, gx + 5, gy + 5, - {255, 40, 40, 128}); + pi = pan_item_tri_new(pw, + px, py, gx, gy, gx + 5, gy + 5, + {255, 40, 40, 128}); pan_item_tri_border(pi, PAN_BORDER_1 | PAN_BORDER_3, {255, 0, 0, 128}); } diff --git a/src/pan-view/pan-item.cc b/src/pan-view/pan-item.cc index 4a10b8e4..0b4d9d46 100644 --- a/src/pan-view/pan-item.cc +++ b/src/pan-view/pan-item.cc @@ -269,7 +269,7 @@ gint pan_item_box_draw(PanWindow *, PanItem *pi, GdkPixbuf *pixbuf, PixbufRender *----------------------------------------------------------------------------- */ -PanItem *pan_item_tri_new(PanWindow *pw, FileData *, gint x, gint y, gint width, gint height, +PanItem *pan_item_tri_new(PanWindow *pw, gint x1, gint y1, gint x2, gint y2, gint x3, gint y3, const PanColor &color) { @@ -278,13 +278,11 @@ PanItem *pan_item_tri_new(PanWindow *pw, FileData *, gint x, gint y, gint width, pi = g_new0(PanItem, 1); pi->type = PAN_ITEM_TRIANGLE; - pi->x = x; - pi->y = y; - pi->width = width; - pi->height = height; - pi->color = color; + util_clip_triangle(x1, y1, x2, y2, x3, y3, + pi->x, pi->y, pi->width, pi->height); + coord = g_new0(gint, 6); coord[0] = x1; coord[1] = y1; diff --git a/src/pan-view/pan-item.h b/src/pan-view/pan-item.h index ec43470f..fba146ad 100644 --- a/src/pan-view/pan-item.h +++ b/src/pan-view/pan-item.h @@ -57,7 +57,7 @@ gint pan_item_box_draw(PanWindow *pw, PanItem *pi, GdkPixbuf *pixbuf, PixbufRend gint x, gint y, gint width, gint height); // Item triangle type -PanItem *pan_item_tri_new(PanWindow *pw, FileData *fd, gint x, gint y, gint width, gint height, +PanItem *pan_item_tri_new(PanWindow *pw, gint x1, gint y1, gint x2, gint y2, gint x3, gint y3, const PanColor &color); void pan_item_tri_border(PanItem *pi, gint borders, const PanColor &color); diff --git a/src/pan-view/pan-view.cc b/src/pan-view/pan-view.cc index ef9acc11..0a256e04 100644 --- a/src/pan-view/pan-view.cc +++ b/src/pan-view/pan-view.cc @@ -1410,10 +1410,6 @@ void pan_info_update(PanWindow *pw, PanItem *pi) gint y2; gint x3; gint y3; - gint x; - gint y; - gint w; - gint h; if (pw->click_pi == pi) return; if (pi && !pi->fd) pi = nullptr; @@ -1431,8 +1427,8 @@ void pan_info_update(PanWindow *pw, PanItem *pi) if (pi->type == PAN_ITEM_THUMB && pi->pixbuf) { - w = gdk_pixbuf_get_width(pi->pixbuf); - h = gdk_pixbuf_get_height(pi->pixbuf); + gint w = gdk_pixbuf_get_width(pi->pixbuf); + gint h = gdk_pixbuf_get_height(pi->pixbuf); x1 = pi->x + pi->width - (pi->width - w) / 2 - 8; y1 = pi->y + (pi->height - h) / 2 + 8; @@ -1447,12 +1443,10 @@ void pan_info_update(PanWindow *pw, PanItem *pi) y2 = pbox->y + 36; x3 = pbox->x + 1; y3 = pbox->y + 12; - util_clip_triangle(x1, y1, x2, y2, x3, y3, - x, y, w, h); - p = pan_item_tri_new(pw, nullptr, x, y, w, h, - x1, y1, x2, y2, x3, y3, - PAN_POPUP_COLOR); + p = pan_item_tri_new(pw, + x1, y1, x2, y2, x3, y3, + PAN_POPUP_COLOR); pan_item_tri_border(p, PAN_BORDER_1 | PAN_BORDER_3, PAN_POPUP_BORDER_COLOR); pan_item_set_key(p, "info"); pan_item_added(pw, p); -- 2.20.1