x-export-frames for non-Cairo builds

x-export-frames for non-Cairo builds

[Clément Pit-Claudel]

Hi emacs-devel,

I'd like to capture screenshots of Emacs frames from inside Emacs (currently I use `import' or `xwd', but they don't always work well).
On Cairo there's x-export-frame, but I don't think there's an equivalent function for other builds.

The following draft seems to work nicely for me (on GTK3)

    DEFUN ("x-export-png", Fx_export_png, Sx_export_png, 0, 1, 0,
           doc: /* Save FRAME as 'screenshot.png'. */)
         (Lisp_Object frame)
    {
      struct frame *f = decode_window_system_frame(frame);
      int width = FRAME_PIXEL_WIDTH (f);
      int height = FRAME_PIXEL_HEIGHT (f);
      printf("W, H: %d, %d\n", width, height);

      GdkWindow *w = gtk_widget_get_window (FRAME_GTK_WIDGET (f));
      GdkPixbuf *pb = gdk_pixbuf_get_from_window(w, 0, 0, width, height);
      gdk_pixbuf_save (pb, "/tmp/screenshot.png", "png", NULL, NULL);

      return Qnil;
    }

However, I have multiple questions:

* Assuming proper error checking, documentation, and adjustments, would there be interest in merging this?
* IIUC, this code is GTK3-only.  On GTK2, it should be possible to use gdk_pixbuf_get_from_drawable, right? (or is it gdk_pixbuf_xlib_get_from_drawable?).  Both of these gave me linker errors, however.
* Where should this code go? Should it be merged into x-export-frames?
* Is this the right way to capture a screenshot of an Emacs frame in GTK?  Or is there a faster way? (for example, can I defer part of the work of creating the pixbuf, maybe by getting a GdkImage first, and converting later?  I'm hoping to reduce the latency of the screenshot-taking process, even if it requires some post-processing)

Thanks!
Clément.

Re: x-export-frames for non-Cairo builds

[Eli Zaretskii]

> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Thu, 25 Jan 2018 18:25:55 -0500
> 
> I'd like to capture screenshots of Emacs frames from inside Emacs (currently I use `import' or `xwd', but they don't always work well).
> On Cairo there's x-export-frame, but I don't think there's an equivalent function for other builds.
> 
> The following draft seems to work nicely for me (on GTK3)
> 
>     DEFUN ("x-export-png", Fx_export_png, Sx_export_png, 0, 1, 0,
>            doc: /* Save FRAME as 'screenshot.png'. */)
>          (Lisp_Object frame)
>     {

Please, not another x-FOO function that is very likely to have 4
different implementations.  Instead, please define a single function
whose name does NOT start with "x-", and make it have a GTK-specific
and a Cairo-specific implementation, which (the implementations)
ideally should not be exported to Lisp.

>       struct frame *f = decode_window_system_frame(frame);
>       int width = FRAME_PIXEL_WIDTH (f);
>       int height = FRAME_PIXEL_HEIGHT (f);
>       printf("W, H: %d, %d\n", width, height);

"printf"?

>       GdkWindow *w = gtk_widget_get_window (FRAME_GTK_WIDGET (f));
>       GdkPixbuf *pb = gdk_pixbuf_get_from_window(w, 0, 0, width, height);
>       gdk_pixbuf_save (pb, "/tmp/screenshot.png", "png", NULL, NULL);

That hard-coded file name is a no-no.  The function should probably
accept an optional file name, and by default use some defcustom, which
I think should be a relative file name, i.e. created in the current
directory.  AFAIU, the image format should also be an optional
argument (even though not all GUI frames will support that).

> * Assuming proper error checking, documentation, and adjustments, would there be interest in merging this?

Yes.  Implementations for other window-systems are welcome.

> * Where should this code go? Should it be merged into x-export-frames?

frame.c looks like the best place to me.

Thanks.

Re: x-export-frames for non-Cairo builds

[Stefan Monnier]

> Please, not another x-FOO function that is very likely to have 4
> different implementations.  Instead, please define a single function
> whose name does NOT start with "x-",

Hear! Hear!

> and make it have a GTK-specific and a Cairo-specific implementation,
> which (the implementations) ideally should not be exported to Lisp.

And which code to use should be based on dispatching on the frame type
(so it can work even if we have several different kinds of frame types:
currently that's only true of "X11-vs-tty", more or less, but there's no
reason why we couldn't have GNUstep frames, W32 frames, and Gtk frames
all within the same Emacs session, at some point in the future).

>>       struct frame *f = decode_window_system_frame(frame);
                                                    ^^
Missing space.


        Stefan

Re: x-export-frames for non-Cairo builds

[Clément Pit-Claudel]

On 2018-01-26 03:08, Eli Zaretskii wrote:
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Thu, 25 Jan 2018 18:25:55 -0500
>>
>> I'd like to capture screenshots of Emacs frames from inside Emacs (currently I use `import' or `xwd', but they don't always work well).
>> On Cairo there's x-export-frame, but I don't think there's an equivalent function for other builds.
>>
>> The following draft seems to work nicely for me (on GTK3)
>>
>>     DEFUN ("x-export-png", Fx_export_png, Sx_export_png, 0, 1, 0,
>>            doc: /* Save FRAME as 'screenshot.png'. */)
>>          (Lisp_Object frame)
>>     {
> 
> Please, not another x-FOO function that is very likely to have 4
> different implementations.  Instead, please define a single function
> whose name does NOT start with "x-", and make it have a GTK-specific
> and a Cairo-specific implementation, which (the implementations)
> ideally should not be exported to Lisp.

Thanks; understood. I will prepare a patch.

>>       GdkWindow *w = gtk_widget_get_window (FRAME_GTK_WIDGET (f));
>>       GdkPixbuf *pb = gdk_pixbuf_get_from_window(w, 0, 0, width, height);
>>       gdk_pixbuf_save (pb, "/tmp/screenshot.png", "png", NULL, NULL);
> 
> That hard-coded file name is a no-no. 

Yes, of course.  Sorry if it was unclear that I didn't intend to propose to merge this code as-is ^^

> The function should probably
> accept an optional file name, and by default use some defcustom, which
> I think should be a relative file name, i.e. created in the current
> directory.  AFAIU, the image format should also be an optional
> argument (even though not all GUI frames will support that).

OK, these are great ideas.  But do you think it's better to save the image, rather than what x-export-frame currently does?

>> * Assuming proper error checking, documentation, and adjustments, would there be interest in merging this?
> 
> Yes.  Implementations for other window-systems are welcome.

Super.

>> * Where should this code go? Should it be merged into x-export-frames?
> 
> frame.c looks like the best place to me.

Great.

I hope someone can comment on gdk_pixbuf_get_from_drawable/gdk_pixbuf_xlib_get_from_drawable :)

Thanks again,
Clément.

Re: x-export-frames for non-Cairo builds

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Fri, 26 Jan 2018 11:08:17 -0500
> 
> But do you think it's better to save the image, rather than what x-export-frame currently does?

Yes, I do.

Re: x-export-frames for non-Cairo builds

[Clément Pit-Claudel]

[-- Attachment #1: Type: text/plain, Size: 2407 bytes --]

On 2018-01-26 13:56, Eli Zaretskii wrote:
>> Cc: emacs-devel@gnu.org
>> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
>> Date: Fri, 26 Jan 2018 11:08:17 -0500
>>
>> But do you think it's better to save the image, rather than what x-export-frame currently does?
> 
> Yes, I do.

Understood, thanks.  Here's a first draft of the patch.  I have a few questions:

* Is there a way to wrap 'attributes: noreturn' in a preprocessor directive? I used an auxilliary C function instead because of that.
* Is it OK to reuse `frame' after calling `decode_window_system_frame (frame)'?
* What would be the proper way to save the output of Fx_export_frame (there's a FIXME at that point in the code)?  I could either use a_write on the actual string data, or add further branches to x_cr_export_frame to write to disk directly.
* x_cr_export_frame does a number of things that I don't understand too well:
    specbind (Qredisplay_dont_pause, Qt);
    redisplay_preserve_echo_area (31);
    block_input ();
    record_unwind_protect (x_cr_destroy, make_save_ptr (cr));
    x_clear_area (f, 0, 0, width, height);
    expose_frame (f, 0, 0, width, height);
  Do I need any of these in x_gtk3_export_frame?
* The docs of gdk_pixbuf_get_from_window say that "If the window you’re obtaining data from is partially obscured by other windows, then the contents of the pixbuf areas corresponding to the obscured regions are undefined".  Is there a way I can check for that?

Once the API is stabilized, I'll write a proper commit message and documentation.

I made it possible for the function to return a string instead of writing to disk to save time (I'm hoping to make Emacs screencasts).  One issue is that the cast to an Emacs string is still quite slow (quick benchmarks: I did 200 screenshots with xwd, with my new function saving a png and then a bmp into a string, and with my new function saving a png and then a bmp to disk: 1.82s, 5.1s, 1.84s, 5.2s, 0.7s[!]).  Is there a trick I could use to make image capture faster?  Could I store the GdkPixbuf directly into an Emacs image and save it later?

Thanks a lot!

On 2018-01-26 09:45, Stefan Monnier wrote:
> And which code to use should be based on dispatching on the frame type
> (so it can work even if we have several different kinds of frame types:

I'm not sure I understood this right.  Does the patch below do what you had in mind?

Cheers,
Clément.

[-- Attachment #2: export-frame.patch --]
[-- Type: text/x-patch, Size: 5966 bytes --]

diff --git a/src/frame.c b/src/frame.c
index 9b56080..f0c0d34 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -3508,6 +3508,67 @@ bottom edge of FRAME's display.  */)
 
   return Qt;
 }
+
+#ifdef HAVE_WINDOW_SYSTEM
+static
+#if ! (defined HAVE_GTK3 || defined USE_CAIRO)
+_Noreturn
+#endif
+Lisp_Object
+export_frame (Lisp_Object frame, Lisp_Object type, Lisp_Object fname)
+{
+  struct frame *f = decode_window_system_frame (frame);
+
+  if (!FRAME_VISIBLE_P (f))
+    error ("Frames to be exported must be visible");
+
+  if (FRAME_OBSCURED_P (f))
+    error ("Frames to be exported must not be obscured");
+
+  if (!NILP(fname))
+      CHECK_STRING(fname);
+
+  // Stefan, is this what you had in mind?
+  switch (f->output_method)
+    {
+    case output_x_window:
+#ifdef USE_CAIRO
+      // FIXME: save output when FNAME is non-nil
+      /* Question: Is it OK to reuse FRAME here? */
+      return Fx_export_frames(frame, type);
+#elif HAVE_GTK3
+      return x_gtk3_export_frame(f, type, fname);
+#endif
+      /* Fall through */
+    case output_initial:
+    case output_termcap:
+    case output_w32:
+    case output_msdos_raw:
+    case output_ns:
+    default:
+      error ("Unsupported toolkit");
+    }
+}
+
+DEFUN ("export-frame", Fexport_frame,
+       Sexport_frame, 0, 3, 0,
+       doc: /* Capture a screenshot of FRAME in TYPE format.
+Available formats depend on the graphic toolkit in use.
+Currently, this function only works with Cairo and GTK3.
+
+FRAME must be a live, visible, and unobscured frame.  It defaults to
+the selected one.  TYPE is a symbol: on Cairo, valid formats may
+include `pdf', `png', `postscript', and `svg', depending on
+compilation options; on GTK3, valid formats are `png' and `bmp'.
+
+If FNAME is non-nil, save the resulting capture under FNAME; if
+FNAME is nil, return the captured data as a unibyte string.  */)
+     (Lisp_Object frame, Lisp_Object type, Lisp_Object fname)
+{
+  return export_frame(frame, type, fname);
+}
+#endif // HAVE_WINDOW_SYSTEM
+
 \f
 /***********************************************************************
 				Frame Parameters
@@ -6141,6 +6202,7 @@ iconify the top level frame instead.  */);
   defsubr (&Sframe_pointer_visible_p);
 
 #ifdef HAVE_WINDOW_SYSTEM
+  defsubr (&Sexport_frame);
   defsubr (&Sx_get_resource);
   defsubr (&Sx_parse_geometry);
 #endif
diff --git a/src/xfns.c b/src/xfns.c
index 43c55cc..dc4ef8d 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -7419,6 +7419,48 @@ present and mapped to the usual X keysyms.  */)
 			       Printing
  ***********************************************************************/
 
+#ifdef HAVE_GTK3
+Lisp_Object
+x_gtk3_export_frame (struct frame* frame, Lisp_Object type, Lisp_Object fname)
+{
+  int width = FRAME_PIXEL_WIDTH (frame);
+  int height = FRAME_PIXEL_HEIGHT (frame);
+
+  const char* stype;
+  if (NILP (type) || EQ (type, Qpng))
+    stype = "png";
+  else if (EQ (type, Qbmp))
+    stype = "bmp";
+  else
+    error("Unsupported image type");
+
+  GdkWindow *w = gtk_widget_get_window (FRAME_GTK_WIDGET (frame));
+  if (!w)
+    error("Could not retrieve a GTK window for this frame");
+
+  // gdk_pixbuf_xlib_get_from_drawable(NULL, w, NULL, NULL, 0, 0, 0, 0, width, height);
+  GdkPixbuf *pb = gdk_pixbuf_get_from_window(w, 0, 0, width, height);
+  if (!pb)
+    error("Could not capture a screenshot of this frame");
+
+  if (NILP(fname))
+    {
+      char* buf;
+      long unsigned int buf_size;
+      if (!gdk_pixbuf_save_to_buffer (pb, &buf, &buf_size, stype, NULL, NULL))
+        error ("Could not convert the frame data");
+      return make_unibyte_string (buf, buf_size);
+    }
+  else
+    {
+      Lisp_Object encoded_fname = ENCODE_FILE (fname);
+      if (!gdk_pixbuf_save (pb, SSDATA (encoded_fname), stype, NULL, NULL))
+        error ("Could not save the frame data");
+      return Qnil;
+    }
+}
+#endif // HAVE_GTK3
+
 #ifdef USE_CAIRO
 DEFUN ("x-export-frames", Fx_export_frames, Sx_export_frames, 0, 2, 0,
        doc: /* Return image data of FRAMES in TYPE format.
@@ -7443,7 +7485,7 @@ compile-time configuration of cairo.  */)
 
       XSETFRAME (frame, f);
       if (!FRAME_VISIBLE_P (f))
-	error ("Frames to be exported must be visible.");
+	error ("Frames to be exported must be visible");
       tmp = Fcons (frame, tmp);
     }
   frames = Fnreverse (tmp);
@@ -7457,7 +7499,7 @@ compile-time configuration of cairo.  */)
   if (EQ (type, Qpng))
     {
       if (!NILP (XCDR (frames)))
-	error ("PNG export cannot handle multiple frames.");
+	error ("PNG export cannot handle multiple frames");
       surface_type = CAIRO_SURFACE_TYPE_IMAGE;
     }
   else
@@ -7472,7 +7514,7 @@ compile-time configuration of cairo.  */)
     {
       /* For now, we stick to SVG 1.1.  */
       if (!NILP (XCDR (frames)))
-	error ("SVG export cannot handle multiple frames.");
+	error ("SVG export cannot handle multiple frames");
       surface_type = CAIRO_SURFACE_TYPE_SVG;
     }
   else
@@ -7545,7 +7587,7 @@ visible.  */)
 
       XSETFRAME (frame, f);
       if (!FRAME_VISIBLE_P (f))
-	error ("Frames to be printed must be visible.");
+	error ("Frames to be printed must be visible");
       tmp = Fcons (frame, tmp);
     }
   frames = Fnreverse (tmp);
@@ -7633,6 +7675,7 @@ syms_of_xfns (void)
   DEFSYM (Qmono, "mono");
   DEFSYM (Qassq_delete_all, "assq-delete-all");
 
+  DEFSYM (Qbmp, "bmp");
 #ifdef USE_CAIRO
   DEFSYM (Qpdf, "pdf");
 
diff --git a/src/xterm.h b/src/xterm.h
index 1849a5c..267ade6 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -1082,6 +1082,9 @@ extern void x_real_pos_and_offsets (struct frame *f,
                                     int *xptr,
                                     int *yptr,
                                     int *outer_border);
+extern Lisp_Object x_gtk3_export_frame (struct frame* frame,
+                                        Lisp_Object fname,
+                                        Lisp_Object type);
 
 /* From xrdb.c.  */
 

Re: x-export-frames for non-Cairo builds

[Eli Zaretskii]

> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Fri, 26 Jan 2018 18:13:28 -0500
> 
> Understood, thanks.  Here's a first draft of the patch.  I have a few questions:

Thanks, and apologies for a long delay in responding.

> * Is there a way to wrap 'attributes: noreturn' in a preprocessor directive? I used an auxilliary C function instead because of that.

I don't understand why you needed that attribute.  Can you elaborate?

In general, the auxiliary function is short enough to make the
function redundant: just put its body inside Fexport_frame.

> * Is it OK to reuse `frame' after calling `decode_window_system_frame (frame)'?

Yes.  That function doesn't change its argument, it only extracts a C
pointer from it.

> * What would be the proper way to save the output of Fx_export_frame (there's a FIXME at that point in the code)?  I could either use a_write on the actual string data, or add further branches to x_cr_export_frame to write to disk directly.

Just open a file and write the data into it.

> * x_cr_export_frame does a number of things that I don't understand too well:
>     specbind (Qredisplay_dont_pause, Qt);
>     redisplay_preserve_echo_area (31);
>     block_input ();
>     record_unwind_protect (x_cr_destroy, make_save_ptr (cr));
>     x_clear_area (f, 0, 0, width, height);
>     expose_frame (f, 0, 0, width, height);
>   Do I need any of these in x_gtk3_export_frame?

These measures make sure the frame is fully visible and completely
redrawn, before you export it, so I think you do need them.

The call to record_unwind_protect is needed because otherwise the
data allocated by cairo_create will not be released if the function
signals an error, or the user types C-g.  If none of the objects
created inside x_gtk3_export_frame need to be released (the code
indicates they don't, but maybe you forgot something), then you don't
need a similar call to record_unwind_protect.

> * The docs of gdk_pixbuf_get_from_window say that "If the window you’re obtaining data from is partially obscured by other windows, then the contents of the pixbuf areas corresponding to the obscured regions are undefined".  Is there a way I can check for that?

I think the above measures eliminate this problem by calling
expose_frame.

> I made it possible for the function to return a string instead of writing to disk to save time (I'm hoping to make Emacs screencasts).  One issue is that the cast to an Emacs string is still quite slow (quick benchmarks: I did 200 screenshots with xwd, with my new function saving a png and then a bmp into a string, and with my new function saving a png and then a bmp to disk: 1.82s, 5.1s, 1.84s, 5.2s, 0.7s[!]).  Is there a trick I could use to make image capture faster?  Could I store the GdkPixbuf directly into an Emacs image and save it later?

Sorry, I don't know enough about GTK programming to answer that.

> On 2018-01-26 09:45, Stefan Monnier wrote:
> > And which code to use should be based on dispatching on the frame type
> > (so it can work even if we have several different kinds of frame types:
> 
> I'm not sure I understood this right.  Does the patch below do what you had in mind?

I believe so.

Some more comments below.

> 
> Cheers,
> Clément.
> 
> [2:text/x-patch Hide Save:export-frame.patch (6kB)]
> 
> diff --git a/src/frame.c b/src/frame.c
> index 9b56080..f0c0d34 100644
> --- a/src/frame.c
> +++ b/src/frame.c
> @@ -3508,6 +3508,67 @@ bottom edge of FRAME's display.  */)
>  
>    return Qt;
>  }
> +
> +#ifdef HAVE_WINDOW_SYSTEM
> +static
> +#if ! (defined HAVE_GTK3 || defined USE_CAIRO)
> +_Noreturn
> +#endif
> +Lisp_Object
> +export_frame (Lisp_Object frame, Lisp_Object type, Lisp_Object fname)
> +{
> +  struct frame *f = decode_window_system_frame (frame);
> +
> +  if (!FRAME_VISIBLE_P (f))
> +    error ("Frames to be exported must be visible");
> +
> +  if (FRAME_OBSCURED_P (f))
> +    error ("Frames to be exported must not be obscured");
> +
> +  if (!NILP(fname))
> +      CHECK_STRING(fname);

The Cairo code in x-export-frames also does these checks, so why not
call x_cr_export_frame directly?

> +#ifdef USE_CAIRO
> +      // FIXME: save output when FNAME is non-nil
> +      /* Question: Is it OK to reuse FRAME here? */
> +      return Fx_export_frames(frame, type);
> +#elif HAVE_GTK3
> +      return x_gtk3_export_frame(f, type, fname);
> +#endif

Here and elsewhere please make sure you leave a blank between the name
of a function/macro and the opening parenthesis.

> +      /* Fall through */
> +    case output_initial:
> +    case output_termcap:
> +    case output_w32:
> +    case output_msdos_raw:
> +    case output_ns:
> +    default:
> +      error ("Unsupported toolkit");

I think implementation for text-mode frames should not be too hard:
you could reuse some of the code in dump-frame-glyph-matrix and its
subroutines.  The result should be a text file, of course.

Also "Unsupported toolkit" is slightly misleading, it would be better
to say something like

  "This command does not yet support frames of type %s"

with a suitable string replacing %s.

> +DEFUN ("export-frame", Fexport_frame,
> +       Sexport_frame, 0, 3, 0,
> +       doc: /* Capture a screenshot of FRAME in TYPE format.
> +Available formats depend on the graphic toolkit in use.

This last sentence should be moved to after the description of TYPE.
(And I would call the argument FORMAT instead.)

> +Currently, this function only works with Cairo and GTK3.
> +
> +FRAME must be a live, visible, and unobscured frame.  It defaults to

Partially obscured frames are okay, right?  If so, the doc string
should tell that.

> +If FNAME is non-nil, save the resulting capture under FNAME; if
> +FNAME is nil, return the captured data as a unibyte string.  */)

This never says explicitly that FNAME is a file name.  Either tell
that or call the argument FILE.

Also, "save in file FNAME" is better; "under" is a strange way of
saying this.