Re: [Spice-devel] [spice-gtk v2 1/2] Deprecate “color-depth” properties
Hey, On Wed, Mar 06, 2019 at 02:24:24PM +, Victor Toso wrote: > Hi, > > On Tue, Jan 08, 2019 at 04:22:55PM +0100, Victor Toso wrote: > > 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 > > > > > > > > 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 > > > > Acked-by: Christophe Fergeau > > > > > > 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. > > Now that 0.36 is out, I'm wondering if this two patches are fine > for you? CC: teuf and Uri :) Fine with me, Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [spice-gtk v2 1/2] Deprecate “color-depth” properties
Hi,
On Tue, Jan 08, 2019 at 04:22:55PM +0100, Victor Toso wrote:
> 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
> > >
> > > 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
> > > Acked-by: Christophe Fergeau
> >
> > 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.
Now that 0.36 is out, I'm wondering if this two patches are fine
for you? CC: teuf and Uri :)
> 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 {
> > > boolagent_caps_received;
> > >
> > > gbooleanagent_display_config_sent;
> > > -guint8 display_color_depth;
> > > gbooleandisplay_disable_wallpaper:1;
> > > gbooleandisplay_disable_font_smooth:1;
> > > gbooleandisplay_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
Re: [Spice-devel] [spice-gtk v2 1/2] Deprecate “color-depth” properties
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
> >
> > 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
> > Acked-by: Christophe Fergeau
>
> 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 {
> > boolagent_caps_received;
> >
> > gbooleanagent_display_config_sent;
> > -guint8 display_color_depth;
> > gbooleandisplay_disable_wallpaper:1;
> > gbooleandisplay_disable_font_smooth:1;
> > gbooleandisplay_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
Re: [Spice-devel] [spice-gtk v2 1/2] Deprecate “color-depth” properties
On Fri, Dec 14, 2018 at 04:29:46PM +0100, Victor Toso wrote:
> From: Victor Toso
>
> 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
> Acked-by: Christophe Fergeau
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.
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 {
> boolagent_caps_received;
>
> gbooleanagent_display_config_sent;
> -guint8 display_color_depth;
> gbooleandisplay_disable_wallpaper:1;
> gbooleandisplay_disable_font_smooth:1;
> gbooleandisplay_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-devel] [spice-gtk v2 1/2] Deprecate “color-depth” properties
From: Victor Toso
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
Acked-by: Christophe Fergeau
---
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 {
boolagent_caps_received;
gbooleanagent_display_config_sent;
-guint8 display_color_depth;
gbooleandisplay_disable_wallpaper:1;
gbooleandisplay_disable_font_smooth:1;
gbooleandisplay_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:
-
