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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Jan 08, 2019 at 04:09:54PM +0100, Christophe Fergeau wrote:
> On Fri, Dec 14, 2018 at 04:29:46PM +0100, 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.
> > 
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> 
> Still fine with me, though it's indeed a good question that Uri raised,
> whether we want to keep this alive a bit longer for win7 since it's
> still supported by MS.

  | Microsoft ended mainstream support for Windows 7 on January 13,
  | 2015, but extended support won't end until January 14, 2020.

Oh well, IMHO we could remove it for 0.36 but we can do it for
after 0.36 as well.

Thanks for the review
 
> Christophe
> 
> > ---
> >  src/channel-main.c  | 26 +++++++++++++-------------
> >  src/spice-session.c | 12 +++++++-----
> >  2 files changed, 20 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index 4c6bc70..02e2ffd 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;
> > @@ -298,7 +297,8 @@ static void spice_main_get_property(GObject    *object,
> >          g_value_set_boolean(value, c->display_disable_animation);
> >          break;
> >      case PROP_DISPLAY_COLOR_DEPTH:
> > -        g_value_set_uint(value, c->display_color_depth);
> > +        spice_info("SpiceMainChannel::color-depth has been deprecated. Property is ignored");
> > +        g_value_set_uint(value, 32);
> >          break;
> >      case PROP_DISABLE_DISPLAY_POSITION:
> >          g_value_set_boolean(value, c->disable_display_position);
> > @@ -331,12 +331,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 +548,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 to 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 +1143,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 +1307,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..682c3d9 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;
> > @@ -668,7 +667,8 @@ static void spice_session_get_property(GObject    *gobject,
> >          g_value_set_boxed(value, s->secure_channels);
> >          break;
> >      case PROP_COLOR_DEPTH:
> > -        g_value_set_int(value, s->color_depth);
> > +        spice_info("SpiceSession::color-depth has been deprecated. Property is ignored");
> > +        g_value_set_int(value, 0);
> >          break;
> >      case PROP_AUDIO:
> >          g_value_set_boolean(value, s->audio);
> > @@ -808,7 +808,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 +1106,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 to 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 +1116,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 +2228,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;
> > -- 
> > 2.19.2
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel


Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]