> >
> > Add a couple new functions to the header so that they can be called by
> > other objects rather than poking into the internals of the struct.
> > ---
> > server/dcc-send.c | 16 +++++------
> > server/display-channel.c | 71
> > ++++++++++++++++++++++++++++++++++++++++++++----
> > server/display-channel.h | 37 +++++++++++++------------
> > server/red-worker.c | 44 ++++++------------------------
> > server/stream.c | 18 ++++++------
> > 5 files changed, 109 insertions(+), 77 deletions(-)
> >
> ....
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index 108e69b..56bb029 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -203,6 +203,13 @@ void display_channel_surface_unref(DisplayChannel
> > *display, uint32_t surface_id)
> > spice_warn_if_fail(ring_is_empty(&surface->depend_on_me));
> > }
> >
> > +/* TODO: perhaps rename to "ready" or "realized" ? */
> > +bool display_channel_surface_has_canvas(DisplayChannel *display,
> > + uint32_t surface_id)
> > +{
> > + return display->surfaces[surface_id].context.canvas != NULL;
> > +}
> > +
>
> I honestly prefer bool and true/false but we decided to use gboolean and not
> bool.
> This function is mainly doing the same think of validate_surface without
> the logging stuff.
> Perhaps adding a flag "warnings" to validate_surface is enough?
>
>
> ...
>
> > diff --git a/server/display-channel.h b/server/display-channel.h
> > index 7b71480..022cd93 100644
> > --- a/server/display-channel.h
> > +++ b/server/display-channel.h
> > @@ -208,10 +208,17 @@ struct DisplayChannel {
> > ImageEncoderSharedData encoder_shared_data;
> > };
> >
> > -static inline int get_stream_id(DisplayChannel *display, Stream *stream)
> > -{
> > - return (int)(stream - display->streams_buf);
> > -}
> > +
> > +#define FOREACH_DCC(channel, _link, _next, _data) \
> > + for (_link = (channel ? RED_CHANNEL(channel)->clients : NULL), \
> > + _next = (_link ? _link->next : NULL), \
> > + _data = (_link ? _link->data : NULL); \
> > + _link; \
> > + _link = _next, \
> > + _next = (_link ? _link->next : NULL), \
> > + _data = (_link ? _link->data : NULL))
> > +
> > +int display_channel_get_stream_id(DisplayChannel *display, Stream
> > *stream);
> >
> > typedef struct RedSurfaceDestroyItem {
> > RedPipeItem pipe_item;
> > @@ -258,6 +265,8 @@ void
> > display_channel_compress_stats_print (const Disp
> > void display_channel_compress_stats_reset
> > (DisplayChannel *display);
> > void display_channel_surface_unref
> > (DisplayChannel *display,
> > uint32_t
> > surface_id);
> > +bool display_channel_surface_has_canvas
> > (DisplayChannel *display,
> > +
>
> Although this is consistent with the rest it does not follow our style.
>
> > uint32_t
> > surface_id);
> > void display_channel_current_flush
> > (DisplayChannel *display,
> > int
> > surface_id);
> > int display_channel_wait_for_migrate_data
> > (DisplayChannel *display);
> ...
>
Actually.
Would not be easier to have a
RedSurface *display_channel_get_surface(DisplayChannel *display, uint32_t surface_id);
which check surface_id and canvas and return the valid surface or NULL?
This at the same time replace validation of surface ids and accessing surfaces
array inside DisplayChannel.
Personally I don't like that much the display_channel_surface_has_canvas as
it gives implementation detail to the user which could ignore the presence
of a "canvas" in the surface.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel