[Spice-devel] [spice-gtk v1 1/2] Deprecate “color-depth” properties

Uri Lublin uril at redhat.com
Thu Dec 13 16:16:36 UTC 2018


On 12/11/18 2:48 PM, Victor Toso wrote:
> From: Victor Toso <me at victortoso.com>
> 
> 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 at redhat.com>
> ---
>   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;
> 



More information about the Spice-devel mailing list