GNOME Bugzilla – Bug 693737
no longer filtering out screenshots
Last modified: 2013-02-14 18:21:51 UTC
It seems that once we changed to have the shell save screenshots we broke the filtering them out of the background selector.
The metadata isn't written in the PNG files, as gnome-shell uses cairo_surface_write_to_png_stream() which lacks any kind of metadata.
(In reply to comment #1) > The metadata isn't written in the PNG files, as gnome-shell uses > cairo_surface_write_to_png_stream() which lacks any kind of metadata. What metadata do we need?
(In reply to comment #2) > (In reply to comment #1) > > The metadata isn't written in the PNG files, as gnome-shell uses > > cairo_surface_write_to_png_stream() which lacks any kind of metadata. > > What metadata do we need? tEXt::Software gnome-screensaver apparently, see http://git.gnome.org/browse/gnome-screenshot/tree/src/screenshot-application.c#n229
Created attachment 236112 [details] [review] screenshot: Write metadata to png file Use GdkPixbuf rather then cairo_surface_write_to_png_stream when saving screenshots because this allows us to embedded metadata into the file which is used by the background control panel to filter out screenshots.
Created attachment 236113 [details] [review] screenshot: Write metadata to png file Use GdkPixbuf rather then cairo_surface_write_to_png_stream when saving screenshots because this allows us to embedded metadata into the file which is used by the background control panel to filter out screenshots.
Review of attachment 236113 [details] [review]: Thanks, this makes sense. ::: src/shell-screenshot.c @@ +194,3 @@ + NULL); + if (gdk_pixbuf_save_to_stream (pixbuf, stream, "png", NULL, NULL, + 8, Should use gdk_pixbuf_save_to_stream_async() @@ +199,3 @@ + status = CAIRO_STATUS_WRITE_ERROR; + + cairo_image_surface_get_stride (screenshot_data->image), Why this?
(In reply to comment #6) > > ::: src/shell-screenshot.c > @@ +194,3 @@ > + NULL); > + if (gdk_pixbuf_save_to_stream (pixbuf, stream, "png", NULL, NULL, > + 8, > > Should use gdk_pixbuf_save_to_stream_async() Ignore this, we're in a thread already
(In reply to comment #6) > Review of attachment 236113 [details] [review]: > > Thanks, this makes sense. > > ::: src/shell-screenshot.c > @@ +194,3 @@ > + NULL); > + if (gdk_pixbuf_save_to_stream (pixbuf, stream, "png", NULL, NULL, > + 8, > > Should use gdk_pixbuf_save_to_stream_async() Why? We are not in the main thread anyway ... doing it async wouldn't gain us anything. > @@ +199,3 @@ > + status = CAIRO_STATUS_WRITE_ERROR; > + > + cairo_image_surface_get_stride > (screenshot_data->image), > > Why this? Why not? Should I hardcode the stride instead?
(In reply to comment #8) > Why not? Should I hardcode the stride instead? Somehow, Splinter had a bug. If you view the review page manually you'll see it was supposed to refer to the mark_dirty call.
(In reply to comment #9) > (In reply to comment #8) > > Why not? Should I hardcode the stride instead? > > Somehow, Splinter had a bug. If you view the review page manually you'll see it > was supposed to refer to the mark_dirty call. Oh ... well because I don't know whether GdkPixbuf messes with the data or not. Even though we do not do anything with the surface later on it feels more correct this way. I can remove it though (actually the first one I attached does not have it).
Comment on attachment 236112 [details] [review] screenshot: Write metadata to png file This is the one without the mark_dirty call.
Review of attachment 236112 [details] [review]: GdkPixbuf shouldn't touch the passed-in data, so I don't think we should call mark_dirty. This one looks good to me.
Attachment 236112 [details] pushed as eb5a836 - screenshot: Write metadata to png file