From e518b9753dd4f23edec79a8b525c26aafc9cdf1e Mon Sep 17 00:00:00 2001 From: Michael Natterer Date: Tue, 31 Jan 2017 21:26:44 +0100 Subject: [PATCH] Bug 723498 - Gimp changes contrast and color of images Add color management options to the screenshot plug-in: By default, it tries to tag the image with the monitor profile; alternatively, there is an option to convert the image to sRGB. This works mostly fine on *one* monitor given its profile is configured correctly. With more than one monitor, funny things happen depending on the platform and on what we are shooting (window, screen, area). There are some FIXMEs left in the code. --- plug-ins/screenshot/screenshot-gnome-shell.c | 20 +++++ plug-ins/screenshot/screenshot-osx.c | 3 + plug-ins/screenshot/screenshot-win32.c | 70 ++++++++++----- plug-ins/screenshot/screenshot-x11.c | 44 +++++++--- plug-ins/screenshot/screenshot.c | 92 ++++++++++++++------ plug-ins/screenshot/screenshot.h | 26 ++++-- 6 files changed, 183 insertions(+), 72 deletions(-) diff --git a/plug-ins/screenshot/screenshot-gnome-shell.c b/plug-ins/screenshot/screenshot-gnome-shell.c index 84e36061b1..406b98a289 100644 --- a/plug-ins/screenshot/screenshot-gnome-shell.c +++ b/plug-ins/screenshot/screenshot-gnome-shell.c @@ -84,6 +84,7 @@ screenshot_gnome_shell_shoot (ScreenshotValues *shootvals, const gchar *method = NULL; GVariant *args = NULL; GVariant *retval; + gint monitor = shootvals->monitor; gboolean success; if (shootvals->select_delay > 0) @@ -99,6 +100,8 @@ screenshot_gnome_shell_shoot (ScreenshotValues *shootvals, shootvals->show_cursor, TRUE, /* flash */ filename); + + /* FIXME: figure profile */ break; case SHOOT_REGION: @@ -126,6 +129,11 @@ screenshot_gnome_shell_shoot (ScreenshotValues *shootvals, shootvals->y2 - shootvals->y1, TRUE, /* flash */ filename); + + monitor = + gdk_screen_get_monitor_at_point (screen, + (shootvals->x1 + shootvals->x2) / 2, + (shootvals->y1 + shootvals->y2) / 2); break; case SHOOT_WINDOW: @@ -135,6 +143,8 @@ screenshot_gnome_shell_shoot (ScreenshotValues *shootvals, shootvals->show_cursor, TRUE, /* flash */ filename); + + /* FIXME: figure monitor */ break; } @@ -154,10 +164,20 @@ screenshot_gnome_shell_shoot (ScreenshotValues *shootvals, if (success && filename) { + GimpColorProfile *profile; + *image_ID = gimp_file_load (GIMP_RUN_NONINTERACTIVE, filename, filename); gimp_image_set_filename (*image_ID, "screenshot.png"); + profile = gimp_screen_get_color_profile (screen, monitor); + + if (profile) + { + gimp_image_set_color_profile (*image_ID, profile); + g_object_unref (profile); + } + g_unlink (filename); g_free (filename); diff --git a/plug-ins/screenshot/screenshot-osx.c b/plug-ins/screenshot/screenshot-osx.c index 781d6781a5..74e1466a1a 100644 --- a/plug-ins/screenshot/screenshot-osx.c +++ b/plug-ins/screenshot/screenshot-osx.c @@ -125,6 +125,9 @@ screenshot_osx_shoot (ScreenshotValues *shootvals, if (system ((const char *) command) == EXIT_SUCCESS) { + /* don't attach a profile, screencapture attached one + */ + *image_ID = gimp_file_load (GIMP_RUN_NONINTERACTIVE, filename, filename); gimp_image_set_filename (*image_ID, "screenshot.png"); diff --git a/plug-ins/screenshot/screenshot-win32.c b/plug-ins/screenshot/screenshot-win32.c index ee2556356a..08501ab0cf 100644 --- a/plug-ins/screenshot/screenshot-win32.c +++ b/plug-ins/screenshot/screenshot-win32.c @@ -133,24 +133,47 @@ screenshot_win32_shoot (ScreenshotValues *shootvals, gint32 *image_ID, GError **error) { + GimpPDBStatusType status = GIMP_PDB_EXECUTION_ERROR; + + /* leave "shootvals->monitor" alone until somebody patches the code + * to be able to get a monitor's color profile + */ + image_id = image_ID; winsnapvals.delay = shootvals->select_delay; if (shootvals->shoot_type == SHOOT_ROOT) { - doCapture(0); - return GIMP_PDB_SUCCESS; + doCapture (0); + + status = GIMP_PDB_SUCCESS; } else if (shootvals->shoot_type == SHOOT_WINDOW) { - doWindowCapture(); - return GIMP_PDB_SUCCESS; + doWindowCapture (); + + status = GIMP_PDB_SUCCESS; } else if (shootvals->shoot_type == SHOOT_REGION) - return GIMP_PDB_EXECUTION_ERROR; + { + /* FIXME */ + } - return GIMP_PDB_EXECUTION_ERROR; + if (status == GIMP_PDB_SUCCESS) + { + GimpColorProfile *profile; + + profile = gimp_screen_get_color_profile (screen, monitor); + + if (profile) + { + gimp_image_set_color_profile (*image_ID, profile); + g_object_unref (profile); + } + } + + return status; } @@ -213,25 +236,25 @@ sendBMPToGimp(HBITMAP hBMP, HDC hDC, RECT rect) /* Check that we got the memory */ if (!capBytes) - { - g_message (_("No data captured")); - return; - } + { + g_message (_("No data captured")); + return; + } /* Flip the red and blue bytes */ - flipRedAndBlueBytes(width, height); + flipRedAndBlueBytes (width, height); /* Set up the image and layer types */ imageType = GIMP_RGB; layerType = GIMP_RGB_IMAGE; /* Create the GIMP image and layers */ - new_image_id = gimp_image_new(width, height, imageType); - layer_id = gimp_layer_new(new_image_id, _("Background"), - ROUND4(width), height, - layerType, - 100, GIMP_LAYER_MODE_NORMAL); - gimp_image_insert_layer(new_image_id, layer_id, -1, 0); + new_image_id = gimp_image_new (width, height, imageType); + layer_id = gimp_layer_new (new_image_id, _("Background"), + ROUND4 (width), height, + layerType, + 100, GIMP_LAYER_MODE_NORMAL); + gimp_image_insert_layer (new_image_id, layer_id, -1, 0); /* make rectangle */ rectangle = g_new (GeglRectangle, 1); @@ -244,17 +267,18 @@ sendBMPToGimp(HBITMAP hBMP, HDC hDC, RECT rect) buffer = gimp_drawable_get_buffer (layer_id); /* fill the buffer */ - gegl_buffer_set (buffer, rectangle, 0, NULL, (guchar *) capBytes, GEGL_AUTO_ROWSTRIDE); + gegl_buffer_set (buffer, rectangle, 0, NULL, (guchar *) capBytes, + GEGL_AUTO_ROWSTRIDE); /* flushing data */ gegl_buffer_flush (buffer); /* Now resize the layer down to the correct size if necessary. */ - if (width != ROUND4(width)) { - gimp_layer_resize (layer_id, width, height, 0, 0); - gimp_image_resize (new_image_id, width, height, 0, 0); - } - /* Finish up */ + if (width != ROUND4 (width)) + { + gimp_layer_resize (layer_id, width, height, 0, 0); + gimp_image_resize (new_image_id, width, height, 0, 0); + } *image_id = new_image_id; diff --git a/plug-ins/screenshot/screenshot-x11.c b/plug-ins/screenshot/screenshot-x11.c index fbede767b7..9b7bc51a00 100644 --- a/plug-ins/screenshot/screenshot-x11.c +++ b/plug-ins/screenshot/screenshot-x11.c @@ -553,17 +553,19 @@ screenshot_x11_shoot (ScreenshotValues *shootvals, gint32 *image_ID, GError **error) { - GdkDisplay *display; - GdkWindow *window; - cairo_surface_t *screenshot; - cairo_region_t *shape = NULL; - cairo_t *cr; - GdkRectangle rect; - GdkRectangle screen_rect; - gchar *name = NULL; - gint screen_x; - gint screen_y; - gint x, y; + GdkDisplay *display; + GdkWindow *window; + cairo_surface_t *screenshot; + cairo_region_t *shape = NULL; + cairo_t *cr; + GimpColorProfile *profile; + GdkRectangle rect; + GdkRectangle screen_rect; + gchar *name = NULL; + gint screen_x; + gint screen_y; + gint monitor = shootvals->monitor; + gint x, y; /* use default screen if we are running non-interactively */ if (screen == NULL) @@ -589,21 +591,29 @@ screenshot_x11_shoot (ScreenshotValues *shootvals, if (shootvals->shoot_type == SHOOT_REGION) { - rect.x = MIN (shootvals->x1, shootvals->x2); - rect.y = MIN (shootvals->y1, shootvals->y2); + rect.x = MIN (shootvals->x1, shootvals->x2); + rect.y = MIN (shootvals->y1, shootvals->y2); rect.width = ABS (shootvals->x2 - shootvals->x1); rect.height = ABS (shootvals->y2 - shootvals->y1); + + monitor = gdk_screen_get_monitor_at_point (screen, + rect.x + rect.width / 2, + rect.y + rect.height / 2); } else { if (shootvals->shoot_type == SHOOT_ROOT) { window = gdk_screen_get_root_window (screen); + + /* FIXME: figure monitor */ } else { window = gdk_x11_window_foreign_new_for_display (display, shootvals->window_id); + + monitor = gdk_screen_get_monitor_at_window (screen, window); } if (! window) @@ -665,6 +675,14 @@ screenshot_x11_shoot (ScreenshotValues *shootvals, if (shootvals->shoot_type == SHOOT_ROOT && shootvals->show_cursor) add_cursor_image (*image_ID, display); + profile = gimp_screen_get_color_profile (screen, monitor); + + if (profile) + { + gimp_image_set_color_profile (*image_ID, profile); + g_object_unref (profile); + } + return GIMP_PDB_SUCCESS; } diff --git a/plug-ins/screenshot/screenshot.c b/plug-ins/screenshot/screenshot.c index e46063d9bf..df7903d353 100644 --- a/plug-ins/screenshot/screenshot.c +++ b/plug-ins/screenshot/screenshot.c @@ -77,12 +77,14 @@ static ScreenshotValues shootvals = SHOOT_WINDOW, /* root window */ TRUE, /* include WM decorations */ 0, /* window ID */ + 0, /* monitor */ 0, /* select delay */ 0, /* coords of region dragged out by pointer */ 0, 0, 0, - FALSE /* show cursor */ + FALSE, /* show cursor */ + SCREENSHOT_PROFILE_POLICY_MONITOR }; const GimpPlugInInfo PLUG_IN_INFO = @@ -217,7 +219,7 @@ run (const gchar *name, gimp_get_data (PLUG_IN_PROC, &shootvals); shootvals.window_id = 0; - /* Get information from the dialog */ + /* Get information from the dialog */ if (! shoot_dialog (&screen)) status = GIMP_PDB_CANCEL; break; @@ -253,7 +255,9 @@ run (const gchar *name, { if (shootvals.shoot_type == SHOOT_WINDOW || shootvals.shoot_type == SHOOT_REGION) - status = GIMP_PDB_CALLING_ERROR; + { + status = GIMP_PDB_CALLING_ERROR; + } } break; @@ -275,6 +279,17 @@ run (const gchar *name, { gchar *comment = gimp_get_default_comment (); + if (shootvals.profile_policy == SCREENSHOT_PROFILE_POLICY_SRGB) + { + GimpColorProfile *srgb_profile = gimp_color_profile_new_rgb_srgb (); + + gimp_image_convert_color_profile (image_ID, + srgb_profile, + GIMP_COLOR_RENDERING_INTENT_RELATIVE_COLORIMETRIC, + TRUE); + g_object_unref (srgb_profile); + } + if (comment) { GimpParasite *parasite; @@ -435,28 +450,13 @@ shoot_dialog (GdkScreen **screen) main_vbox, FALSE, FALSE, 0); gtk_widget_show (main_vbox); - /* Hints */ + + /* Create delay hints notebook early */ notebook = g_object_new (GTK_TYPE_NOTEBOOK, "show-border", FALSE, "show-tabs", FALSE, NULL); - gtk_box_pack_end (GTK_BOX (main_vbox), notebook, FALSE, FALSE, 0); - gtk_widget_show (notebook); - shoot_dialog_add_hint (GTK_NOTEBOOK (notebook), SHOOT_ROOT, - _("After the delay, the screenshot is taken.")); - shoot_dialog_add_hint (GTK_NOTEBOOK (notebook), SHOOT_REGION, - _("After the delay, drag your mouse to select " - "the region for the screenshot.")); -#ifdef G_OS_WIN32 - shoot_dialog_add_hint (GTK_NOTEBOOK (notebook), SHOOT_WINDOW, - _("Click in a window to snap it after delay.")); -#else - shoot_dialog_add_hint (GTK_NOTEBOOK (notebook), SHOOT_WINDOW, - _("At the end of the delay, click in a window " - "to snap it.")); -#endif - gtk_notebook_set_current_page (GTK_NOTEBOOK (notebook), shootvals.shoot_type); /* Area */ frame = gimp_frame_new (_("Area")); @@ -467,8 +467,7 @@ shoot_dialog (GdkScreen **screen) gtk_container_add (GTK_CONTAINER (frame), vbox); gtk_widget_show (vbox); - - /* single window */ + /* Aingle window */ button = gtk_radio_button_new_with_mnemonic (radio_group, _("Take a screenshot of " "a single _window")); @@ -483,7 +482,7 @@ shoot_dialog (GdkScreen **screen) G_CALLBACK (shoot_radio_button_toggled), notebook); - /* window decorations */ + /* Window decorations */ if (capabilities & SCREENSHOT_CAN_SHOOT_DECORATIONS) { hbox = gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 12); @@ -508,7 +507,7 @@ shoot_dialog (GdkScreen **screen) gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (button), shootvals.shoot_type == SHOOT_WINDOW); - /* whole screen */ + /* Whole screen */ button = gtk_radio_button_new_with_mnemonic (radio_group, _("Take a screenshot of " "the entire _screen")); @@ -523,7 +522,7 @@ shoot_dialog (GdkScreen **screen) G_CALLBACK (shoot_radio_button_toggled), notebook); - /* mouse pointer */ + /* Mouse pointer */ if (capabilities & SCREENSHOT_CAN_SHOOT_POINTER) { hbox = gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 12); @@ -548,7 +547,7 @@ shoot_dialog (GdkScreen **screen) gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (button), shootvals.shoot_type == SHOOT_ROOT); - /* dragged region */ + /* Dragged region */ if (capabilities & SCREENSHOT_CAN_SHOOT_REGION) { button = gtk_radio_button_new_with_mnemonic (radio_group, @@ -592,11 +591,50 @@ shoot_dialog (GdkScreen **screen) G_CALLBACK (gimp_int_adjustment_update), &shootvals.select_delay); - /* this is the unit label of a spinbutton */ + /* translators: this is the unit label of a spinbutton */ label = gtk_label_new (_("seconds")); gtk_box_pack_start (GTK_BOX (hbox), label, FALSE, FALSE, 0); gtk_widget_show (label); + /* Delay hints */ + gtk_box_pack_start (GTK_BOX (vbox), notebook, FALSE, FALSE, 0); + gtk_widget_show (notebook); + + shoot_dialog_add_hint (GTK_NOTEBOOK (notebook), SHOOT_ROOT, + _("After the delay, the screenshot is taken.")); + shoot_dialog_add_hint (GTK_NOTEBOOK (notebook), SHOOT_REGION, + _("After the delay, drag your mouse to select " + "the region for the screenshot.")); +#ifdef G_OS_WIN32 + shoot_dialog_add_hint (GTK_NOTEBOOK (notebook), SHOOT_WINDOW, + _("Click in a window to snap it after delay.")); +#else + shoot_dialog_add_hint (GTK_NOTEBOOK (notebook), SHOOT_WINDOW, + _("At the end of the delay, click in a window " + "to snap it.")); +#endif + gtk_notebook_set_current_page (GTK_NOTEBOOK (notebook), shootvals.shoot_type); + + /* Color profile */ + frame = gimp_int_radio_group_new (TRUE, + _("Color Profile"), + G_CALLBACK (gimp_radio_button_update), + &shootvals.profile_policy, + SCREENSHOT_PROFILE_POLICY_MONITOR, + + _("Tag image with _monitor profile"), + SCREENSHOT_PROFILE_POLICY_MONITOR, + NULL, + + _("Convert image to sR_GB"), + SCREENSHOT_PROFILE_POLICY_SRGB, + NULL, + + NULL); + gtk_box_pack_start (GTK_BOX (main_vbox), frame, FALSE, FALSE, 0); + gtk_widget_show (frame); + + gtk_widget_show (dialog); run = (gimp_dialog_run (GIMP_DIALOG (dialog)) == GTK_RESPONSE_OK); diff --git a/plug-ins/screenshot/screenshot.h b/plug-ins/screenshot/screenshot.h index 1b16905165..592e6bc8cd 100644 --- a/plug-ins/screenshot/screenshot.h +++ b/plug-ins/screenshot/screenshot.h @@ -36,6 +36,12 @@ typedef enum SCREENSHOT_CAN_SHOOT_REGION = 0x1 << 3 } ScreenshotCapabilities; +typedef enum +{ + SCREENSHOT_PROFILE_POLICY_MONITOR, + SCREENSHOT_PROFILE_POLICY_SRGB +} ScreenshotProfilePolicy; + typedef enum { SHOOT_ROOT, @@ -45,15 +51,17 @@ typedef enum typedef struct { - ShootType shoot_type; - gboolean decorate; - guint window_id; - guint select_delay; - gint x1; - gint y1; - gint x2; - gint y2; - gboolean show_cursor; + ShootType shoot_type; + gboolean decorate; + guint window_id; + gint monitor; + guint select_delay; + gint x1; + gint y1; + gint x2; + gint y2; + gboolean show_cursor; + ScreenshotProfilePolicy profile_policy; } ScreenshotValues;