Hi, On Thu, Dec 13, 2018 at 06:16:36PM +0200, Uri Lublin wrote: > 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). Well, they still can go to the guest's settings in Display > QXL and change it. This would only affect users that change color-depth on guest often by spice command line which is a hard use case to make a case in keeping. Code with this patches will not interfere with color-depth so if it was 16, it'll be kept 16 till the user changes this default in the guest configs. > 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
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel