PGTK double-buffer Optimisations

[fejfighter]

This is an improvement over previous work to reduce flickering.

Rather than delete and recreate a context every render cycle, we swap them once a new context is required. There are probably some other tweaks that could be made but it's already much better.

[fejfighter]

I have also cleaned up some of the compiler warnings listed in pgtkmenu.c

I have left the memcopy warning as that also appears in master/ xmenu.c

that whole code feels like it could be simplified but should be in a different branch

[masm11]

Sorry, I should have noticed about compiler warnings more clearly.

My compiler says as follows:

In file included from gtkutil.h:29,
                 from pgtkterm.c:42:
pgtkterm.c: In function 'x_set_parent_frame':
pgtkterm.h:424:53: warning: passing argument 1 of 'gtk_window_get_size' from incompatible pointer type [-Wincompatible-pointer-types]
  424 | #define FRAME_GTK_WIDGET(f)       (FRAME_X_OUTPUT(f)->edit_widget)
      |                                   ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
      |                                                     |
      |                                                     GtkWidget * {aka struct _GtkWidget *}
pgtkterm.h:428:36: note: in expansion of macro 'FRAME_GTK_WIDGET'
  428 | #define FRAME_X_WINDOW(f)          FRAME_GTK_WIDGET(f)
      |                                    ^~~~~~~~~~~~~~~~
pgtkterm.c:703:23: note: in expansion of macro 'FRAME_X_WINDOW'
  703 |   gtk_window_get_size(FRAME_X_WINDOW(f), &width, &height);
      |                       ^~~~~~~~~~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtkdialog.h:32,
                 from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30,
                 from /usr/include/gtk-3.0/gtk/gtk.h:31,
                 from gtkutil.h:26,
                 from pgtkterm.c:42:
/usr/include/gtk-3.0/gtk/gtkwindow.h:431:52: note: expected 'GtkWindow *' {aka 'struct _GtkWindow *'} but argument is of type 'GtkWidget *' {aka 'struct _GtkWidget *'}
  431 | void     gtk_window_get_size         (GtkWindow   *window,
      |                                       ~~~~~~~~~~~~~^~~~~~
In file included from gtkutil.h:29,
                 from pgtkterm.c:42:
pgtkterm.h:424:53: warning: passing argument 1 of 'gtk_window_set_transient_for' from incompatible pointer type [-Wincompatible-pointer-types]
  424 | #define FRAME_GTK_WIDGET(f)       (FRAME_X_OUTPUT(f)->edit_widget)
      |                                   ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
      |                                                     |
      |                                                     GtkWidget * {aka struct _GtkWidget *}
pgtkterm.h:428:36: note: in expansion of macro 'FRAME_GTK_WIDGET'
  428 | #define FRAME_X_WINDOW(f)          FRAME_GTK_WIDGET(f)
      |                                    ^~~~~~~~~~~~~~~~
pgtkterm.c:720:36: note: in expansion of macro 'FRAME_X_WINDOW'
  720 |       gtk_window_set_transient_for(FRAME_X_WINDOW(f), FRAME_X_WINDOW(p));
      |                                    ^~~~~~~~~~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtkdialog.h:32,
                 from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30,
                 from /usr/include/gtk-3.0/gtk/gtk.h:31,
                 from gtkutil.h:26,
                 from pgtkterm.c:42:
/usr/include/gtk-3.0/gtk/gtkwindow.h:190:70: note: expected 'GtkWindow *' {aka 'struct _GtkWindow *'} but argument is of type 'GtkWidget *' {aka 'struct _GtkWidget *'}
  190 | void       gtk_window_set_transient_for        (GtkWindow           *window,
      |                                                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~
In file included from gtkutil.h:29,
                 from pgtkterm.c:42:
pgtkterm.h:424:53: warning: passing argument 2 of 'gtk_window_set_transient_for' from incompatible pointer type [-Wincompatible-pointer-types]
  424 | #define FRAME_GTK_WIDGET(f)       (FRAME_X_OUTPUT(f)->edit_widget)
      |                                   ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
      |                                                     |
      |                                                     GtkWidget * {aka struct _GtkWidget *}
pgtkterm.h:428:36: note: in expansion of macro 'FRAME_GTK_WIDGET'
  428 | #define FRAME_X_WINDOW(f)          FRAME_GTK_WIDGET(f)
      |                                    ^~~~~~~~~~~~~~~~
pgtkterm.c:720:55: note: in expansion of macro 'FRAME_X_WINDOW'
  720 |       gtk_window_set_transient_for(FRAME_X_WINDOW(f), FRAME_X_WINDOW(p));
      |                                                       ^~~~~~~~~~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtkdialog.h:32,
                 from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30,
                 from /usr/include/gtk-3.0/gtk/gtk.h:31,
                 from gtkutil.h:26,
                 from pgtkterm.c:42:
/usr/include/gtk-3.0/gtk/gtkwindow.h:191:28: note: expected 'GtkWindow *' {aka 'struct _GtkWindow *'} but argument is of type 'GtkWidget *' {aka 'struct _GtkWidget *'}
  191 |       GtkWindow           *parent);
      |       ~~~~~~~~~~~~~~~~~~~~~^~~~~~
In file included from gtkutil.h:29,
                 from pgtkterm.c:42:
pgtkterm.h:424:53: warning: passing argument 1 of 'gtk_window_set_attached_to' from incompatible pointer type [-Wincompatible-pointer-types]
  424 | #define FRAME_GTK_WIDGET(f)       (FRAME_X_OUTPUT(f)->edit_widget)
      |                                   ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
      |                                                     |
      |                                                     GtkWidget * {aka struct _GtkWidget *}
pgtkterm.h:428:36: note: in expansion of macro 'FRAME_GTK_WIDGET'
  428 | #define FRAME_X_WINDOW(f)          FRAME_GTK_WIDGET(f)
      |                                    ^~~~~~~~~~~~~~~~
pgtkterm.c:721:34: note: in expansion of macro 'FRAME_X_WINDOW'
  721 |       gtk_window_set_attached_to(FRAME_X_WINDOW(f), FRAME_X_WINDOW(p));
      |                                  ^~~~~~~~~~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtkdialog.h:32,
                 from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30,
                 from /usr/include/gtk-3.0/gtk/gtk.h:31,
                 from gtkutil.h:26,
                 from pgtkterm.c:42:
/usr/include/gtk-3.0/gtk/gtkwindow.h:195:70: note: expected 'GtkWindow *' {aka 'struct _GtkWindow *'} but argument is of type 'GtkWidget *' {aka 'struct _GtkWidget *'}
  195 | void       gtk_window_set_attached_to          (GtkWindow           *window,
      |                                                 ~~~~~~~~~~~~~~~~~~~~~^~~~~~
In file included from gtkutil.h:29,
                 from pgtkterm.c:42:
pgtkterm.h:424:53: warning: passing argument 1 of 'gtk_window_move' from incompatible pointer type [-Wincompatible-pointer-types]
  424 | #define FRAME_GTK_WIDGET(f)       (FRAME_X_OUTPUT(f)->edit_widget)
      |                                   ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
      |                                                     |
      |                                                     GtkWidget * {aka struct _GtkWidget *}
pgtkterm.h:428:36: note: in expansion of macro 'FRAME_GTK_WIDGET'
  428 | #define FRAME_X_WINDOW(f)          FRAME_GTK_WIDGET(f)
      |                                    ^~~~~~~~~~~~~~~~
pgtkterm.c:722:23: note: in expansion of macro 'FRAME_X_WINDOW'
  722 |       gtk_window_move(FRAME_X_WINDOW(f), f->left_pos, f->top_pos);
      |                       ^~~~~~~~~~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtkdialog.h:32,
                 from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30,
                 from /usr/include/gtk-3.0/gtk/gtk.h:31,
                 from gtkutil.h:26,
                 from pgtkterm.c:42:
/usr/include/gtk-3.0/gtk/gtkwindow.h:435:52: note: expected 'GtkWindow *' {aka 'struct _GtkWindow *'} but argument is of type 'GtkWidget *' {aka 'struct _GtkWidget *'}
  435 | void     gtk_window_move             (GtkWindow   *window,
      |                                       ~~~~~~~~~~~~~^~~~~~
In file included from gtkutil.h:29,
                 from pgtkterm.c:42:
pgtkterm.h:424:53: warning: passing argument 1 of 'gtk_window_set_keep_above' from incompatible pointer type [-Wincompatible-pointer-types]
  424 | #define FRAME_GTK_WIDGET(f)       (FRAME_X_OUTPUT(f)->edit_widget)
      |                                   ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
      |                                                     |
      |                                                     GtkWidget * {aka struct _GtkWidget *}
pgtkterm.h:428:36: note: in expansion of macro 'FRAME_GTK_WIDGET'
  428 | #define FRAME_X_WINDOW(f)          FRAME_GTK_WIDGET(f)
      |                                    ^~~~~~~~~~~~~~~~
pgtkterm.c:723:33: note: in expansion of macro 'FRAME_X_WINDOW'
  723 |       gtk_window_set_keep_above(FRAME_X_WINDOW(f), true);
      |                                 ^~~~~~~~~~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtkdialog.h:32,
                 from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30,
                 from /usr/include/gtk-3.0/gtk/gtk.h:31,
                 from gtkutil.h:26,
                 from pgtkterm.c:42:
/usr/include/gtk-3.0/gtk/gtkwindow.h:397:51: note: expected 'GtkWindow *' {aka 'struct _GtkWindow *'} but argument is of type 'GtkWidget *' {aka 'struct _GtkWidget *'}
  397 | void     gtk_window_set_keep_above    (GtkWindow *window, gboolean setting);
      |                                        ~~~~~~~~~~~^~~~~~
pgtkterm.c: In function 'pgtk_draw_window_cursor':
pgtkterm.c:2685:17: warning: unused variable 'f' [-Wunused-variable]
 2685 |   struct frame *f = XFRAME (WINDOW_FRAME (w));
      |                 ^
In file included from gtkutil.h:29,
                 from pgtkterm.c:42:
pgtkterm.c: In function 'x_set_parent_frame':
pgtkterm.h:414:52: warning: potential null pointer dereference [-Wnull-dereference]
  414 | #define FRAME_X_OUTPUT(f)         ((f)->output_data.pgtk)
      |                                   ~~~~~~~~~~~~~~~~~^~~~~~
pgtkterm.h:424:36: note: in expansion of macro 'FRAME_X_OUTPUT'
  424 | #define FRAME_GTK_WIDGET(f)       (FRAME_X_OUTPUT(f)->edit_widget)
      |                                    ^~~~~~~~~~~~~~
pgtkterm.h:428:36: note: in expansion of macro 'FRAME_GTK_WIDGET'
  428 | #define FRAME_X_WINDOW(f)          FRAME_GTK_WIDGET(f)
      |                                    ^~~~~~~~~~~~~~~~
pgtkterm.c:721:53: note: in expansion of macro 'FRAME_X_WINDOW'
  721 |       gtk_window_set_attached_to(FRAME_X_WINDOW(f), FRAME_X_WINDOW(p));
      |                                                     ^~~~~~~~~~~~~~
pgtkterm.h:414:52: warning: potential null pointer dereference [-Wnull-dereference]
  414 | #define FRAME_X_OUTPUT(f)         ((f)->output_data.pgtk)
      |                                   ~~~~~~~~~~~~~~~~~^~~~~~
pgtkterm.h:424:36: note: in expansion of macro 'FRAME_X_OUTPUT'
  424 | #define FRAME_GTK_WIDGET(f)       (FRAME_X_OUTPUT(f)->edit_widget)
      |                                    ^~~~~~~~~~~~~~
pgtkterm.h:428:36: note: in expansion of macro 'FRAME_GTK_WIDGET'
  428 | #define FRAME_X_WINDOW(f)          FRAME_GTK_WIDGET(f)
      |                                    ^~~~~~~~~~~~~~~~
pgtkterm.c:720:55: note: in expansion of macro 'FRAME_X_WINDOW'
  720 |       gtk_window_set_transient_for(FRAME_X_WINDOW(f), FRAME_X_WINDOW(p));
      |                                                       ^~~~~~~~~~~~~~

They are in your code of PR #14, aren't they?

[fejfighter]

I was definitely not getting those warnings, I thought few I got for pgtkmenu were strict, but fair.

I also noticed that I was not working off the correct branch last night and pushed my fixes to the #12 branch instead.

[fejfighter]

compiler warnings/ castings should now be gone, I'l look at fixing the (likely) childframe branch conflicts tomorrow

[masm11]

You pushed 1a0d9ce to #12. Is that mistake, or intentional?

[masm11]

You said:

Rather than delete and recreate a context every render cycle, we try and swap them once a new context is completed.

How do you swap contexts? In flip_cr_context, there is the following line:

FRAME_CR_ACTIVE_CONTEXT(f) = cairo_reference(FRAME_CR_CONTEXT(f));

but there is not a line like FRAME_CR_CONTEXT(f) = ....

[fejfighter]

You said:

Rather than delete and recreate a context every render cycle, we try and swap them once a new context is completed.

How do you swap contexts? In flip_cr_context, there is the following line:

FRAME_CR_ACTIVE_CONTEXT(f) = cairo_reference(FRAME_CR_CONTEXT(f));

but there is not a line like FRAME_CR_CONTEXT(f) = ....

I have re-read my code updated the original PR message to be more accurate

FRAME_CR_CONTEXT(f) = ... occurs in pgtk_begin_cr_clip()

Cairo does not allow resizing a surface/context after it is created, so we need to create a new context with the new dimensions and re-draw.

While the size does not change the CR_ACTIVE_CONTEXT is the same as the CR_CONTEXT, and updates continue as normal,

when the resize occurs, this code will remove the CR_CONTEXT reference, leaving the ACTIVE_CONTEXT with the old/outdated information if a draw request comes in.

when the emacs loop begins drawing the first request to pgtk_begin_cr_clip() will create a new context with the new size information, once a full draw has completed we de reference the old context being held by CR_ACTIVE_CONTEXT and have it refer to the CR_CONTEXT.

so it's not a flip in a front buffer/back buffer gpu rendering sense, (as it does not occur every frame) but only when a new context is required

[fejfighter]

You pushed 1a0d9ce to #12. Is that mistake, or intentional?

and yes, a mistake, it's now on this branch as well

[masm11]

Thank you for the description. Great work!