Patch 8.2.4776

28 views
Skip to first unread message

Bram Moolenaar

unread,
Apr 17, 2022, 1:28:35 PM4/17/22
to vim...@googlegroups.com

Patch 8.2.4776
Problem: GTK: 'lines' and 'columns' may change during startup.
Solution: Ignore stale GTK resize events. (Ernie Rael, closes #10179)
Files: src/gui_gtk_x11.c


*** ../vim-8.2.4775/src/gui_gtk_x11.c 2022-04-15 13:53:30.052708679 +0100
--- src/gui_gtk_x11.c 2022-04-17 18:24:50.852088820 +0100
***************
*** 397,402 ****
--- 397,526 ----
#endif

/*
+ * Keep a short term resize history so that stale gtk responses can be
+ * discarded.
+ * When a gtk_window_resize() request is sent to gtk, the width/height of
+ * the request is saved. Recent stale requests are kept around in a list.
+ * See https://github.com/vim/vim/issues/10123
+ */
+ #if 0 // Change to 1 to enable ch_log() calls for debugging.
+ # ifdef FEAT_JOB_CHANNEL
+ # define ENABLE_RESIZE_HISTORY_LOG
+ # endif
+ #endif
+
+ /*
+ * History item of a resize request.
+ * Width and height are of gui.mainwin.
+ */
+ typedef struct resize_history {
+ int used; // If true, can't match for discard. Only matches once.
+ int width;
+ int height;
+ #ifdef ENABLE_RESIZE_HISTORY_LOG
+ int seq; // for ch_log messages
+ #endif
+ struct resize_history *next;
+ } resize_hist_T;
+
+ // never NULL during execution
+ static resize_hist_T *latest_resize_hist;
+ // list of stale resize requests
+ static resize_hist_T *old_resize_hists;
+
+ /*
+ * Used when calling gtk_window_resize().
+ * Create a resize request history item, put previous request on stale list.
+ * Width/height are the size of the request for the gui.mainwin.
+ */
+ static void
+ alloc_resize_hist(int width, int height)
+ {
+ // alloc a new resize hist, save current in list of old history
+ resize_hist_T *prev_hist = latest_resize_hist;
+ resize_hist_T *new_hist = ALLOC_CLEAR_ONE(resize_hist_T);
+
+ new_hist->width = width;
+ new_hist->height = height;
+ latest_resize_hist = new_hist;
+
+ // previous hist item becomes head of list
+ prev_hist->next = old_resize_hists;
+ old_resize_hists = prev_hist;
+
+ #ifdef ENABLE_RESIZE_HISTORY_LOG
+ new_hist->seq = prev_hist->seq + 1;
+ ch_log(NULL, "gui_gtk: New resize seq %d (%d, %d) [%d, %d]",
+ new_hist->seq, width, height, (int)Columns, (int)Rows);
+ #endif
+ }
+
+ /*
+ * Free everything on the stale resize history list.
+ * This list is empty when there are no outstanding resize requests.
+ */
+ static void
+ clear_resize_hists()
+ {
+ #ifdef ENABLE_RESIZE_HISTORY_LOG
+ int i = 0;
+ #endif
+
+ if (latest_resize_hist)
+ latest_resize_hist->used = TRUE;
+ while (old_resize_hists != NULL)
+ {
+ resize_hist_T *next_hist = old_resize_hists->next;
+
+ vim_free(old_resize_hists);
+ old_resize_hists = next_hist;
+ #ifdef ENABLE_RESIZE_HISTORY_LOG
+ i++;
+ #endif
+ }
+ #ifdef ENABLE_RESIZE_HISTORY_LOG
+ ch_log(NULL, "gui_gtk: free %d hists", i);
+ #endif
+ }
+
+ // true if hist item is unused and matches w,h
+ #define MATCH_WIDTH_HEIGHT(hist, w, h) \
+ (!hist->used && hist->width == w && hist->height == h)
+
+ /*
+ * Search the resize hist list.
+ * Return true if the specified width,height match an item in the list that
+ * has never matched before. Mark the matching item as used so it will
+ * not match again.
+ */
+ static int
+ match_stale_width_height(int width, int height)
+ {
+ resize_hist_T *hist = old_resize_hists;
+
+ for (hist = old_resize_hists; hist != NULL; hist = hist->next)
+ if (MATCH_WIDTH_HEIGHT(hist, width, height))
+ {
+ #ifdef ENABLE_RESIZE_HISTORY_LOG
+ ch_log(NULL, "gui_gtk: discard seq %d, cur seq %d",
+ hist->seq, latest_resize_hist->seq);
+ #endif
+ hist->used = TRUE;
+ return TRUE;
+ }
+ return FALSE;
+ }
+
+ #if defined(EXITFREE)
+ static void
+ free_all_resize_hist()
+ {
+ clear_resize_hists();
+ vim_free(latest_resize_hist);
+ }
+ #endif
+
+ /*
* GTK doesn't set the GDK_BUTTON1_MASK state when dragging a touch. Add this
* state when dragging.
*/
***************
*** 593,598 ****
--- 717,723 ----
#if defined(USE_GNOME_SESSION)
vim_free(abs_restart_command);
#endif
+ free_all_resize_hist();
}
#endif

***************
*** 709,715 ****
xev.xproperty.window = commWindow;
xev.xproperty.state = PropertyNewValue;
serverEventProc(GDK_WINDOW_XDISPLAY(gtk_widget_get_window(widget)),
! &xev, 0);
}
return FALSE;
}
--- 834,840 ----
xev.xproperty.window = commWindow;
xev.xproperty.state = PropertyNewValue;
serverEventProc(GDK_WINDOW_XDISPLAY(gtk_widget_get_window(widget)),
! &xev, 0);
}
return FALSE;
}
***************
*** 720,727 ****
*/
static void
gtk_settings_xft_dpi_changed_cb(GtkSettings *gtk_settings UNUSED,
! GParamSpec *pspec UNUSED,
! gpointer data UNUSED)
{
// Create a new PangoContext for this screen, and initialize it
// with the current font if necessary.
--- 845,852 ----
*/
static void
gtk_settings_xft_dpi_changed_cb(GtkSettings *gtk_settings UNUSED,
! GParamSpec *pspec UNUSED,
! gpointer data UNUSED)
{
// Create a new PangoContext for this screen, and initialize it
// with the current font if necessary.
***************
*** 4006,4012 ****
GdkEventConfigure *event,
gpointer data UNUSED)
{
! int usable_height = event->height;

#if GTK_CHECK_VERSION(3,22,2) && !GTK_CHECK_VERSION(3,22,4)
// As of 3.22.2, GdkWindows have started distributing configure events to
--- 4131,4166 ----
GdkEventConfigure *event,
gpointer data UNUSED)
{
! int usable_height = event->height;
! // Resize requests are made for gui.mainwin,
! // get it's dimensions for searching if this event
! // is a response to a vim request.
! GdkWindow *win = gtk_widget_get_window(gui.mainwin);
! int w = gdk_window_get_width(win);
! int h = gdk_window_get_height(win);
!
! #ifdef ENABLE_RESIZE_HISTORY_LOG
! ch_log(NULL, "gui_gtk: form_configure_event: (%d, %d) [%d, %d]",
! w, h, (int)Columns, (int)Rows);
! #endif
!
! // Look through history of recent vim resize reqeusts.
! // If this event matches:
! // - "latest resize hist" We're caught up;
! // clear the history and process this event.
! // If history is, old to new, 100, 99, 100, 99. If this event is
! // 99 for the stale, it is matched against the current. History
! // is cleared, we my bounce, but no worse than before.
! // - "older/stale hist" If match an unused event in history,
! // then discard this event, and mark the matching event as used.
! // - "no match" Figure it's a user resize event, clear history.
! // NOTE: clear history is default, then all incoming events are processed
!
! if (!MATCH_WIDTH_HEIGHT(latest_resize_hist, w, h)
! && match_stale_width_height(w, h))
! // discard stale event
! return TRUE;
! clear_resize_hists();

#if GTK_CHECK_VERSION(3,22,2) && !GTK_CHECK_VERSION(3,22,4)
// As of 3.22.2, GdkWindows have started distributing configure events to
***************
*** 4329,4343 ****
* manager upon us and should not interfere with what VIM is requesting
* upon startup.
*/
g_signal_connect(G_OBJECT(gui.formwin), "configure-event",
! G_CALLBACK(form_configure_event), NULL);

#ifdef FEAT_DND
// Set up for receiving DND items.
gui_gtk_set_dnd_targets();

g_signal_connect(G_OBJECT(gui.drawarea), "drag-data-received",
! G_CALLBACK(drag_data_received_cb), NULL);
#endif

// With GTK+ 2, we need to iconify the window before calling show()
--- 4483,4498 ----
* manager upon us and should not interfere with what VIM is requesting
* upon startup.
*/
+ latest_resize_hist = ALLOC_CLEAR_ONE(resize_hist_T);
g_signal_connect(G_OBJECT(gui.formwin), "configure-event",
! G_CALLBACK(form_configure_event), NULL);

#ifdef FEAT_DND
// Set up for receiving DND items.
gui_gtk_set_dnd_targets();

g_signal_connect(G_OBJECT(gui.drawarea), "drag-data-received",
! G_CALLBACK(drag_data_received_cb), NULL);
#endif

// With GTK+ 2, we need to iconify the window before calling show()
***************
*** 4516,4521 ****
--- 4671,4677 ----
width += get_menu_tool_width();
height += get_menu_tool_height();

+ alloc_resize_hist(width, height); // track the resize request
if (gtk_socket_id == 0)
gtk_window_resize(GTK_WINDOW(gui.mainwin), width, height);
else
*** ../vim-8.2.4775/src/version.c 2022-04-17 17:34:37.660960579 +0100
--- src/version.c 2022-04-17 18:14:28.856281284 +0100
***************
*** 748,749 ****
--- 748,751 ----
{ /* Add new patch number below this line */
+ /**/
+ 4776,
/**/

--
The Feynman problem solving Algorithm:
1) Write down the problem
2) Think real hard
3) Write down the answer

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Dominique Pellé

unread,
Apr 17, 2022, 3:13:22 PM4/17/22
to vim_dev
Hi.

I see typos in the comments of this commit 8.2.4776:

> ! // Resize requests are made for gui.mainwin,
> ! // get it's dimensions for searching if this event

it's -> its

! // Look through history of recent vim resize reqeusts.

requests -> requests

Regards
Dominique

Bram Moolenaar

unread,
Apr 17, 2022, 4:17:23 PM4/17/22
to vim...@googlegroups.com, Dominique Pellé
I'll correct it, thanks.

--
hundred-and-one symptoms of being an internet addict:
15. Your heart races faster and beats irregularly each time you see a new WWW
site address in print or on TV, even though you've never had heart
problems before.

Christ van Willegen

unread,
Apr 17, 2022, 5:05:07 PM4/17/22
to vim...@googlegroups.com
Hi, 

Op zo 17 apr. 2022 22:17 schreef Bram Moolenaar <Br...@moolenaar.net>:
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

---
You received this message because you are subscribed to the Google Groups "vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/vim_dev/20220417201719.8D9731C03BA%40moolenaar.net.

Here's another one:

   //                is cleared, we my bounce, but no worse than before.

May

Christ van Willegen 

Bram Moolenaar

unread,
Apr 18, 2022, 7:26:36 AM4/18/22
to vim...@googlegroups.com, Christ van Willegen

Christ van Willegen wrote:

[...]

> Here's another one:
>
> // is cleared, we my bounce, but no worse than before.
>
> May

Thanks.

--
When a fly lands on the ceiling, does it do a half roll or
a half loop?

Hisashi T Fujinaka

unread,
Apr 21, 2022, 4:35:18 PM4/21/22
to vim...@googlegroups.com
OK, so THIS is the commit that I'm having trouble with (makes more
sense).

% git bisect bad
9f53e7bd7f0865585a5447fa3665c5d4c4f91408 is the first bad commit
commit 9f53e7bd7f0865585a5447fa3665c5d4c4f91408
Author: Ernie Rael <err...@raelity.com>
Date: Sun Apr 17 18:27:49 2022 +0100

patch 8.2.4776: GTK: 'lines' and 'columns' may change during startup

Problem: GTK: 'lines' and 'columns' may change during startup.
Solution: Ignore stale GTK resize events. (Ernie Rael, closes #10179)

src/gui_gtk_x11.c | 168 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
src/version.c | 2 +
2 files changed, 164 insertions(+), 6 deletions(-)

I've diffed the output of config and make and no change on config. Make
only changes after the errors start:

gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK -I/opt/sw/include/gtk-2.0 -I/opt/sw/lib/gtk-2.0/include -I/opt/sw/include/gtk-2.0 -I/opt/sw/include/pango-1.0 -I/opt/sw/include/atk-1.0 -I/opt/sw/include/cairo -I/opt/sw/include/pango-1.0 -I/opt/sw/include/glib-2.0 -I/opt/sw/lib/glib-2.0/include -I/opt/sw/include/freetype2 -I/opt/sw/include -I/opt/X11/include -MD -I/opt/sw/include -DMACOS_X -g -O2 -I/opt/sw/include -D_REENTRANT -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -I/usr/X11/include -o objects/gui_gtk_x11.o gui_gtk_x11.c
gui_gtk_x11.c:4139:14: error: implicit declaration of function 'gdk_window_get_width' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
int w = gdk_window_get_width(win);
^
gui_gtk_x11.c:4140:14: error: implicit declaration of function 'gdk_window_get_height' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
int h = gdk_window_get_height(win);
^
gui_gtk_x11.c:4140:14: note: did you mean 'gdk_window_get_origin'?
/opt/sw/include/gtk-2.0/gdk/gdkwindow.h:541:12: note: 'gdk_window_get_origin' declared here
gint gdk_window_get_origin (GdkWindow *window,
^
2 errors generated.
make: *** [objects/gui_gtk_x11.o] Error 1
#

--
Hisashi T Fujinaka - ht...@twofifty.com
BSEE + BSChem + BAEnglish + MSCS + $2.50 = coffee

Ernie Rael

unread,
Apr 21, 2022, 5:41:17 PM4/21/22
to vim...@googlegroups.com
On 4/21/22 1:35 PM, Hisashi T Fujinaka wrote:
> OK, so THIS is the commit that I'm having trouble with (makes more
> sense).

On my system I get

$ srcgrep /usr/include gdk_window_get_height
/usr/include/gtk-3.0/gdk/gdkwindow.h:int gdk_window_get_height     
(GdkWindow       *window);

To propose a fix I need to know a bit about other implementations of
gtk, '1' and '2' I guess. Are these methods, or something similar
available? I could create a patch that only works with gtk3.

Hmm, in vim/src/Makefile there's the comment

# Use this with GCC to check for mistakes, unused arguments, etc.
# Note: If you use -Wextra and get warnings in GTK code about function
#       parameters, you can add -Wno-cast-function-type (but not
with clang)

I don't know if that has a bearing here, do these functions appear in gtk-2?

-ernie

Hisashi T Fujinaka

unread,
Apr 21, 2022, 6:34:23 PM4/21/22
to vim...@googlegroups.com
On Thu, 21 Apr 2022, Ernie Rael wrote:

> On 4/21/22 1:35 PM, Hisashi T Fujinaka wrote:
>> OK, so THIS is the commit that I'm having trouble with (makes more
>> sense).
>
> On my system I get
>
> $ srcgrep /usr/include gdk_window_get_height
> /usr/include/gtk-3.0/gdk/gdkwindow.h:int gdk_window_get_height
> (GdkWindow *window);
>
> To propose a fix I need to know a bit about other implementations of gtk, '1'
> and '2' I guess. Are these methods, or something similar available? I could
> create a patch that only works with gtk3.
>
> Hmm, in vim/src/Makefile there's the comment
>
> # Use this with GCC to check for mistakes, unused arguments, etc.
> # Note: If you use -Wextra and get warnings in GTK code about function
> # parameters, you can add -Wno-cast-function-type (but not
> with clang)
>
> I don't know if that has a bearing here, do these functions appear in gtk-2?

Doesn't look like it. I have a workaround (don't use X) but I'm an old
dinosaur who uses terminal windows.
Reply all
Reply to author
Forward
0 new messages