Re: [spice-gtk v1 1/2] Deprecate “color-depth” properties

Uri Lublin <uril@xxxxxxxxxx> · Thu, 13 Dec 2018 18:16:36 +0200

On 12/11/18 2:48 PM, Victor Toso wrote:
From: Victor Toso <me@xxxxxxxxxxxxxx>

With commit 1a980f3712 we deprecated some command line options. The
color-depth one is the only one which is not used for testing/debug
purposes.

The intention of this option is to set guest's video driver color
configuration, currently only windows guest agent uses this feature up
to Windows 7. Windows 8 and up, setting color depth below 32 would
fail. We should not encourage the usage of this feature.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1543538
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1350853

This patch deprecates both SpiceMainChannel::color-depth and
SpiceSession::color-depth while ignoring any set/get to it.

Hi,

Since Windows 7 guest is supported (still), I think we
better not deprecate features that work on Windows 7,
even if it's the only guest it works on (unless the
impact on user experience is pretty small).

Uri.


Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
---
  src/channel-main.c  | 27 +++++++++++++--------------
  src/spice-session.c | 13 +++++++------
  2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/channel-main.c b/src/channel-main.c
index 4c6bc70..b5e1930 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -89,7 +89,6 @@ struct _SpiceMainChannelPrivate  {
      bool                        agent_caps_received;
  
      gboolean                    agent_display_config_sent;
-    guint8                      display_color_depth;
      gboolean                    display_disable_wallpaper:1;
      gboolean                    display_disable_font_smooth:1;
      gboolean                    display_disable_animation:1;
@@ -297,8 +296,8 @@ static void spice_main_get_property(GObject    *object,
      case PROP_DISPLAY_DISABLE_ANIMATION:
          g_value_set_boolean(value, c->display_disable_animation);
          break;
-    case PROP_DISPLAY_COLOR_DEPTH:
-        g_value_set_uint(value, c->display_color_depth);
+    case PROP_DISPLAY_COLOR_DEPTH: /* FIXME: deprecated */
+        g_value_set_uint(value, 32);
          break;
      case PROP_DISABLE_DISPLAY_POSITION:
          g_value_set_boolean(value, c->disable_display_position);
@@ -331,12 +330,9 @@ static void spice_main_set_property(GObject *gobject, guint prop_id,
      case PROP_DISPLAY_DISABLE_ANIMATION:
          c->display_disable_animation = g_value_get_boolean(value);
          break;
-    case PROP_DISPLAY_COLOR_DEPTH: {
-        guint color_depth = g_value_get_uint(value);
-        g_return_if_fail(color_depth % 8 == 0);
-        c->display_color_depth = color_depth;
+    case PROP_DISPLAY_COLOR_DEPTH:
+        spice_info("SpiceMainChannel::color-depth has been deprecated. Property is ignored");
          break;
-    }
      case PROP_DISABLE_DISPLAY_POSITION:
          c->disable_display_position = g_value_get_boolean(value);
          break;
@@ -551,11 +547,19 @@ static void spice_main_channel_class_init(SpiceMainChannelClass *klass)
                                G_PARAM_CONSTRUCT |
                                G_PARAM_STATIC_STRINGS));
  
+    /**
+     * SpiceMainChannel:color-depth:
+     *
+     * Deprecated: 0.36: Deprecated due lack of support in drivers, only Windows 7 and older.
+     * This option is currently ignored.
+     *
+     **/
      g_object_class_install_property
          (gobject_class, PROP_DISPLAY_COLOR_DEPTH,
           g_param_spec_uint("color-depth",
                             "Color depth",
                             "Color depth", 0, 32, 0,
+                           G_PARAM_DEPRECATED |
                             G_PARAM_READWRITE |
                             G_PARAM_CONSTRUCT |
                             G_PARAM_STATIC_STRINGS));
@@ -1138,7 +1142,7 @@ gboolean spice_main_channel_send_monitor_config(SpiceMainChannel *channel)
                  j++;
              continue;
          }
-        mon->monitors[j].depth  = c->display_color_depth ? c->display_color_depth : 32;
+        mon->monitors[j].depth  = 32;
          mon->monitors[j].width  = c->display[i].width;
          mon->monitors[j].height = c->display[i].height;
          mon->monitors[j].x = c->display[i].x;
@@ -1302,11 +1306,6 @@ static void agent_display_config(SpiceMainChannel *channel)
          config.flags |= VD_AGENT_DISPLAY_CONFIG_FLAG_DISABLE_ANIMATION;
      }
  
-    if (c->display_color_depth != 0) {
-        config.flags |= VD_AGENT_DISPLAY_CONFIG_FLAG_SET_COLOR_DEPTH;
-        config.depth = c->display_color_depth;
-    }
-
      CHANNEL_DEBUG(channel, "display_config: flags: %u, depth: %u", config.flags, config.depth);
  
      agent_msg_queue(channel, VD_AGENT_DISPLAY_CONFIG, sizeof(VDAgentDisplayConfig), &config);
diff --git a/src/spice-session.c b/src/spice-session.c
index 7ea1c21..859ea74 100644
--- a/src/spice-session.c
+++ b/src/spice-session.c
@@ -83,7 +83,6 @@ struct _SpiceSessionPrivate {
  
      GStrv             disable_effects;
      GStrv             secure_channels;
-    gint              color_depth;
  
      int               connection_id;
      int               protocol;
@@ -667,8 +666,8 @@ static void spice_session_get_property(GObject    *gobject,
      case PROP_SECURE_CHANNELS:
          g_value_set_boxed(value, s->secure_channels);
          break;
-    case PROP_COLOR_DEPTH:
-        g_value_set_int(value, s->color_depth);
+    case PROP_COLOR_DEPTH: /* FIXME: deprecated */
+        g_value_set_int(value, 0);
          break;
      case PROP_AUDIO:
          g_value_set_boolean(value, s->audio);
@@ -808,7 +807,7 @@ static void spice_session_set_property(GObject      *gobject,
          s->secure_channels = g_value_dup_boxed(value);
          break;
      case PROP_COLOR_DEPTH:
-        s->color_depth = g_value_get_int(value);
+        spice_info("SpiceSession::color-depth has been deprecated. Property is ignored");
          break;
      case PROP_AUDIO:
          s->audio = g_value_get_boolean(value);
@@ -1106,6 +1105,9 @@ static void spice_session_class_init(SpiceSessionClass *klass)
       * Display color depth to set on new display channels. If 0, don't set.
       *
       * Since: 0.7
+     *
+     * Deprecated: 0.36: Deprecated due lack of support in drivers, only Windows 7 and older.
+     * This option is currently ignored.
       **/
      g_object_class_install_property
          (gobject_class, PROP_COLOR_DEPTH,
@@ -1113,6 +1115,7 @@ static void spice_session_class_init(SpiceSessionClass *klass)
                            "Color depth",
                            "Display channel color depth",
                            0, 32, 0,
+                          G_PARAM_DEPRECATED |
                            G_PARAM_READWRITE |
                            G_PARAM_STATIC_STRINGS));
  
@@ -2224,8 +2227,6 @@ void spice_session_channel_new(SpiceSession *session, SpiceChannel *channel)
                       "disable-font-smooth", all || spice_strv_contains(s->disable_effects, "font-smooth"),
                       "disable-animation", all || spice_strv_contains(s->disable_effects, "animation"),
                       NULL);
-        if (s->color_depth != 0)
-            g_object_set(channel, "color-depth", s->color_depth, NULL);
  
          CHANNEL_DEBUG(channel, "new main channel, switching");
          s->cmain = channel;


_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel







