Re: [PATCH v2] tools: port perf ui from GTK 2 to GTK 3

From: Namhyung Kim
Date: Mon Oct 31 2022 - 15:03:35 EST


Hello,

On Wed, Oct 19, 2022 at 4:30 PM Matt Turner <mattst88@xxxxxxxxx> wrote:
>
> From: Jelle van der Waa <jvanderwaa@xxxxxxxxxx>
>
> GTK 2 is a legacy API and the newer version is GTK 3 which changes a few
> API functions. gtk_tree_view_set_rules_hint is no longer required since
> theme's are now responsible to handle this, gtk_vbox_new is deprecated
> and replaced by passing the orientation to gtk_box_new. Apart from these
> changes the accessing of the screen changed in GTK which requires using
> new functions for getting geometry. Lastly the GTK_STOCK_OK define is
> deprecated in GTK 3.
>
> Signed-off-by: Jelle van der Waa <jvanderwaa@xxxxxxxxxx>
> Signed-off-by: Matt Turner <mattst88@xxxxxxxxx>
> ---
> Originally sent in 2019. I've rebased it and given it a test.
>
> One problem I've noticed is that C++ symbol names are often broken.
> E.g.:
>
> (perf:3254940): Gtk-WARNING **: 21:47:46.663: Failed to set text from
> markup due to error parsing markup: Error on line 1 char 95:
> “std::char_traits<char” is not a valid name: “<”
>
> Presumably we need to call g_markup_escape_text() somewhere, but I
> struggled to find the right place.
>
> Without the patch, the GTK-2 version repeatedly segfaults, so this is an
> improvement regardless.

Thanks for working on this.

>
> v2: Dropped changes to test-all.c. I don't know how to build that
> apparently.
>
> tools/build/Makefile.feature | 5 +++--
> tools/build/feature/Makefile | 12 +++++-----
> ...est-gtk2-infobar.c => test-gtk3-infobar.c} | 0
> .../feature/{test-gtk2.c => test-gtk3.c} | 0
> tools/perf/Documentation/perf-report.txt | 2 +-
> tools/perf/Makefile | 2 +-
> tools/perf/Makefile.config | 22 +++++++++----------
> tools/perf/Makefile.perf | 6 ++---
> tools/perf/builtin-report.c | 2 +-
> tools/perf/builtin-version.c | 1 +
> tools/perf/tests/make | 12 +++++-----
> tools/perf/ui/gtk/annotate.c | 2 +-
> tools/perf/ui/gtk/browser.c | 16 ++++++++------
> tools/perf/ui/gtk/hists.c | 6 +----
> tools/perf/ui/gtk/progress.c | 2 +-
> tools/perf/ui/setup.c | 2 +-
> 16 files changed, 46 insertions(+), 46 deletions(-)
> rename tools/build/feature/{test-gtk2-infobar.c => test-gtk3-infobar.c} (100%)
> rename tools/build/feature/{test-gtk2.c => test-gtk3.c} (100%)
>
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index 57619f240b56..967b8be96b65 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -80,8 +80,8 @@ FEATURE_TESTS_EXTRA := \
> compile-32 \
> compile-x32 \
> cplus-demangle \
> - gtk2 \
> - gtk2-infobar \
> + gtk3 \
> + gtk3-infobar \

I don't think gtk3-inforbar check is necessary anymore. IIRC the infobar
was introduced in the middle of gtk2 so it needs to check if the current
version supports it or not. But I guess gtk3 should support it by default.

Thanks,
Namhyung


> hello \
> libbabeltrace \
> libbfd-liberty \
> @@ -119,6 +119,7 @@ FEATURE_DISPLAY ?= \
> dwarf \
> dwarf_getlocations \
> glibc \
> + gtk3 \
> libbfd \
> libbfd-buildid \
> libcap \