pxcloud/spice
6
0
Fork 0
mirror of https://gitlab.uni-freiburg.de/opensourcevdi/spice synced 2026-08-08 19:19:05 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Allows surfaces to be updated without having to wait

Browse Source 
Store pointers to surface object in "surfaces" and allows to
have different surfaces with same ID in memory.
The surface was keep "busy" if there was pending drawing around.

Consider the following case:
 1- receive drawing command
 2- queue command on DCCs
 3- destroy surface
 4- send draw
Previously at point 4) you would have to use a surface from
"surfaces" which was destroyed, that is we would have to maintain
the pointer (and canvas) to the surface until reference counter
was 0.

However consider this case:
 1- receive drawing command
 2- queue command on DCCs
 3- destroy surface
 4- create surface
 5- send draw
What would happen in point 4) ?
We could not change the surface as it will be used by point 5).
To avoid this the code attempts to wait the commands to release the
surface. However this can be an issue, you can't force the clients
to receive pending data if network is slow.

So this patch change this allowing to create surfaces while the old
version will still be used.

This is also more clean from the reference pointer prospective,
as the reference is increased for a specific surface.

Note that now instead of checking for canvas to not be NULL a
simple check for surface pointer is enough.

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This commit is contained in:
Frediano Ziglio 2021-05-23 22:13:56 +01:00
parent f5c2043143
commit d8bca15f2b
6 changed files with 105 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
server/dcc-send.cpp
View File

@ -117,11 +117,23 @@ static bool is_surface_area_lossy(DisplayChannelClient *dcc, RedSurface *surface
return TRUE;
}
static RedSurface*
get_dependent_surface(const Drawable *drawable, uint32_t surface_id)
{
for (auto surface : drawable->surface_deps) {
if (surface && surface->id == surface_id) {
return surface;
}
}
return nullptr;
}
/* returns if the bitmap was already sent lossy to the client. If the bitmap hasn't been sent yet
to the client, returns false. "area" is for surfaces. If area = NULL,
all the surface is considered. out_lossy_data will hold info about the bitmap, and its lossy
area in case it is lossy and part of a surface. */
static bool is_bitmap_lossy(DisplayChannelClient *dcc, SpiceImage *image, SpiceRect *area,
static bool is_bitmap_lossy(DisplayChannelClient *dcc, const Drawable *drawable,
const SpiceImage *image, SpiceRect *area,
BitmapData *out_data)
{
out_data->type = BITMAP_DATA_TYPE_BITMAP;
@ -144,7 +156,8 @@ static bool is_bitmap_lossy(DisplayChannelClient *dcc, SpiceImage *image, SpiceR
return FALSE;
}
auto surface = display_channel_validate_surface(DCC_TO_DC(dcc), image->u.surface.surface_id);
// the surface should be in the dependent list
auto surface = get_dependent_surface(drawable, image->u.surface.surface_id);
if (!surface) {
return false;
}
@ -156,11 +169,12 @@ static bool is_bitmap_lossy(DisplayChannelClient *dcc, SpiceImage *image, SpiceR
area, &out_data->lossy_rect);
}
static bool is_brush_lossy(DisplayChannelClient *dcc, SpiceBrush *brush,
static bool is_brush_lossy(DisplayChannelClient *dcc,
const Drawable *drawable, SpiceBrush *brush,
BitmapData *out_data)
{
if (brush->type == SPICE_BRUSH_TYPE_PATTERN) {
return is_bitmap_lossy(dcc, brush->u.pattern.pat, nullptr,
return is_bitmap_lossy(dcc, drawable, brush->u.pattern.pat, nullptr,
out_data);
}
out_data->type = BITMAP_DATA_TYPE_INVALID;
@ -394,7 +408,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
RedSurface *surface;
auto surface_id = simage->u.surface.surface_id;
surface = display_channel_validate_surface(display, surface_id);
surface = get_dependent_surface(drawable, surface_id);
if (!surface) {
spice_warning("Invalid surface in SPICE_IMAGE_TYPE_SURFACE");
pthread_mutex_unlock(&dcc->priv->pixmap_cache->lock);
@ -811,7 +825,7 @@ static void red_lossy_marshall_qxl_draw_fill(DisplayChannelClient *dcc,
(rop & SPICE_ROPD_OP_AND) ||
(rop & SPICE_ROPD_OP_XOR));
brush_is_lossy = is_brush_lossy(dcc, &drawable->u.fill.brush,
brush_is_lossy = is_brush_lossy(dcc, item, &drawable->u.fill.brush,
&brush_bitmap_data);
if (!dest_allowed_lossy) {
dest_is_lossy = is_surface_area_lossy(dcc, item->surface, &drawable->bbox,
@ -899,11 +913,11 @@ static void red_lossy_marshall_qxl_draw_opaque(DisplayChannelClient *dcc,
(rop & SPICE_ROPD_OP_AND) ||
(rop & SPICE_ROPD_OP_XOR));
brush_is_lossy = is_brush_lossy(dcc, &drawable->u.opaque.brush,
brush_is_lossy = is_brush_lossy(dcc, item, &drawable->u.opaque.brush,
&brush_bitmap_data);
if (!src_allowed_lossy) {
src_is_lossy = is_bitmap_lossy(dcc, drawable->u.opaque.src_bitmap,
src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.opaque.src_bitmap,
&drawable->u.opaque.src_area,
&src_bitmap_data);
}
@ -980,7 +994,7 @@ static void red_lossy_marshall_qxl_draw_copy(DisplayChannelClient *dcc,
BitmapData src_bitmap_data;
FillBitsType src_send_type;
src_is_lossy = is_bitmap_lossy(dcc, drawable->u.copy.src_bitmap,
src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.copy.src_bitmap,
&drawable->u.copy.src_area, &src_bitmap_data);
src_send_type = red_marshall_qxl_draw_copy(dcc, base_marshaller, dpi, TRUE);
@ -1020,7 +1034,7 @@ static void red_lossy_marshall_qxl_draw_transparent(DisplayChannelClient *dcc,
int src_is_lossy;
BitmapData src_bitmap_data;
src_is_lossy = is_bitmap_lossy(dcc, drawable->u.transparent.src_bitmap,
src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.transparent.src_bitmap,
&drawable->u.transparent.src_area, &src_bitmap_data);
if (!src_is_lossy || (src_bitmap_data.type != BITMAP_DATA_TYPE_SURFACE)) {
@ -1071,7 +1085,7 @@ static void red_lossy_marshall_qxl_draw_alpha_blend(DisplayChannelClient *dcc,
BitmapData src_bitmap_data;
FillBitsType src_send_type;
src_is_lossy = is_bitmap_lossy(dcc, drawable->u.alpha_blend.src_bitmap,
src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.alpha_blend.src_bitmap,
&drawable->u.alpha_blend.src_area, &src_bitmap_data);
src_send_type = red_marshall_qxl_draw_alpha_blend(dcc, base_marshaller, dpi, TRUE);
@ -1164,7 +1178,7 @@ static void red_lossy_marshall_qxl_draw_blend(DisplayChannelClient *dcc,
int dest_is_lossy;
SpiceRect dest_lossy_area;
src_is_lossy = is_bitmap_lossy(dcc, drawable->u.blend.src_bitmap,
src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.blend.src_bitmap,
&drawable->u.blend.src_area, &src_bitmap_data);
dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
&drawable->bbox, &dest_lossy_area);
@ -1327,9 +1341,9 @@ static void red_lossy_marshall_qxl_draw_rop3(DisplayChannelClient *dcc,
int dest_is_lossy;
SpiceRect dest_lossy_area;
src_is_lossy = is_bitmap_lossy(dcc, drawable->u.rop3.src_bitmap,
src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.rop3.src_bitmap,
&drawable->u.rop3.src_area, &src_bitmap_data);
brush_is_lossy = is_brush_lossy(dcc, &drawable->u.rop3.brush,
brush_is_lossy = is_brush_lossy(dcc, item, &drawable->u.rop3.brush,
&brush_bitmap_data);
dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
&drawable->bbox, &dest_lossy_area);
@ -1405,10 +1419,10 @@ static void red_lossy_marshall_qxl_draw_composite(DisplayChannelClient *dcc,
int dest_is_lossy;
SpiceRect dest_lossy_area;
src_is_lossy = is_bitmap_lossy(dcc, drawable->u.composite.src_bitmap,
src_is_lossy = is_bitmap_lossy(dcc, item, drawable->u.composite.src_bitmap,
nullptr, &src_bitmap_data);
mask_is_lossy = drawable->u.composite.mask_bitmap &&
is_bitmap_lossy(dcc, drawable->u.composite.mask_bitmap, nullptr, &mask_bitmap_data);
is_bitmap_lossy(dcc, item, drawable->u.composite.mask_bitmap, nullptr, &mask_bitmap_data);
dest_is_lossy = is_surface_area_lossy(dcc, item->surface,
&drawable->bbox, &dest_lossy_area);
@ -1483,7 +1497,7 @@ static void red_lossy_marshall_qxl_draw_stroke(DisplayChannelClient *dcc,
SpiceRect dest_lossy_area;
int rop;
brush_is_lossy = is_brush_lossy(dcc, &drawable->u.stroke.brush,
brush_is_lossy = is_brush_lossy(dcc, item, &drawable->u.stroke.brush,
&brush_bitmap_data);
// back_mode is not used at the client. Ignoring.
@ -1565,9 +1579,9 @@ static void red_lossy_marshall_qxl_draw_text(DisplayChannelClient *dcc,
SpiceRect dest_lossy_area;
int rop = 0;
fg_is_lossy = is_brush_lossy(dcc, &drawable->u.text.fore_brush,
fg_is_lossy = is_brush_lossy(dcc, item, &drawable->u.text.fore_brush,
&fg_bitmap_data);
bg_is_lossy = is_brush_lossy(dcc, &drawable->u.text.back_brush,
bg_is_lossy = is_brush_lossy(dcc, item, &drawable->u.text.back_brush,
&bg_bitmap_data);
// assuming that if the brush type is solid, the destination can

11
server/dcc.cpp
View File

@ -243,7 +243,7 @@ void dcc_push_surface_image(DisplayChannelClient *dcc, RedSurface *surface)
return;
}
if (!surface->context.canvas) {
if (!surface) {
return;
}
area.top = area.left = 0;
@ -407,11 +407,12 @@ void dcc_start(DisplayChannelClient *dcc)
red::shared_ptr<DisplayChannelClient> self(dcc);
dcc->ack_zero_messages_window();
if (display->priv->surfaces[0].context.canvas) {
display_channel_current_flush(display, &display->priv->surfaces[0]);
auto surface0 = display->priv->surfaces[0];
if (surface0) {
display_channel_current_flush(display, surface0);
dcc->pipe_add_type(RED_PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE);
dcc_create_surface(dcc, &display->priv->surfaces[0]);
dcc_push_surface_image(dcc, &display->priv->surfaces[0]);
dcc_create_surface(dcc, surface0);
dcc_push_surface_image(dcc, surface0);
dcc_push_monitors_config(dcc);
dcc->pipe_add_empty_msg(SPICE_MSG_DISPLAY_MARK);
dcc_create_all_streams(dcc);

4
server/display-channel-private.h
View File

@ -36,6 +36,8 @@ typedef struct DrawContext {
} DrawContext;
struct RedSurface {
SPICE_CXX_GLIB_ALLOCATOR
uint32_t refs;
uint16_t id;
/* A Ring representing a hierarchical tree structure. This tree includes
@ -114,7 +116,7 @@ struct DisplayChannelPrivate
uint32_t next_item_trace;
uint64_t streams_size_total;
RedSurface surfaces[NUM_SURFACES];
RedSurface *surfaces[NUM_SURFACES];
uint32_t n_surfaces;
SpiceImageSurfaces image_surfaces;

101
server/display-channel.cpp
View File

@ -45,7 +45,7 @@ DisplayChannel::~DisplayChannel()
spice_assert(ring_is_empty(&priv->streams));
for (const auto &surface : priv->surfaces) {
spice_assert(surface.context.canvas == nullptr);
spice_assert(!surface);
}
}
@ -242,28 +242,30 @@ static void display_channel_surface_unref(DisplayChannel *display, RedSurface *s
spice_assert(surface->context.canvas);
surface->context.canvas->ops->destroy(surface->context.canvas);
surface->context.canvas = nullptr;
surface->create_cmd.reset();
surface->destroy_cmd.reset();
region_destroy(&surface->draw_dirty_region);
surface->context.canvas = nullptr;
FOREACH_DCC(display, dcc) {
dcc_destroy_surface(dcc, surface->id);
}
spice_warn_if_fail(ring_is_empty(&surface->depend_on_me));
delete surface;
}
/* TODO: perhaps rename to "ready" or "realized" ? */
gboolean display_channel_surface_has_canvas(DisplayChannel *display,
uint32_t surface_id)
{
return display->priv->surfaces[surface_id].context.canvas != nullptr;
return display->priv->surfaces[surface_id] != nullptr;
}
void display_channel_surface_id_unref(DisplayChannel *display, uint32_t surface_id)
{
display_channel_surface_unref(display, &display->priv->surfaces[surface_id]);
display_channel_surface_unref(display, display->priv->surfaces[surface_id]);
display->priv->surfaces[surface_id] = nullptr;
}
static void streams_update_visible_region(DisplayChannel *display, Drawable *drawable)
@ -1120,7 +1122,7 @@ static void drawable_ref_surface_deps(DisplayChannel *display, Drawable *drawabl
continue;
}
RedSurface *surface = &display->priv->surfaces[surface_id];
RedSurface *surface = display->priv->surfaces[surface_id];
surface->refs++;
drawable->surface_deps[x] = surface;
}
@ -1299,7 +1301,7 @@ static Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t e
drawable->tree_item.effect = effect;
drawable->surface = &display->priv->surfaces[red_drawable->surface_id];
drawable->surface = display->priv->surfaces[red_drawable->surface_id];
drawable->surface->refs++;
drawable->red_drawable = red_drawable;
@ -1424,11 +1426,9 @@ bool display_channel_wait_for_migrate_data(DisplayChannel *display)
void display_channel_flush_all_surfaces(DisplayChannel *display)
{
int x;
for (x = 0; x < NUM_SURFACES; ++x) {
if (display->priv->surfaces[x].context.canvas) {
display_channel_current_flush(display, &display->priv->surfaces[x]);
for (const auto& surface : display->priv->surfaces) {
if (surface) {
display_channel_current_flush(display, surface);
}
}
}
@ -1914,7 +1914,7 @@ void display_channel_draw(DisplayChannel *display, const SpiceRect *area, int su
spice_return_if_fail(area->left >= 0 && area->top >= 0 &&
area->left < area->right && area->top < area->bottom);
surface = &display->priv->surfaces[surface_id];
surface = display->priv->surfaces[surface_id];
display_channel_surface_draw(display, surface, area);
}
@ -2016,17 +2016,15 @@ void display_channel_destroy_surface_wait(DisplayChannel *display, uint32_t surf
/* TODO: split me*/
void display_channel_destroy_surfaces(DisplayChannel *display)
{
int i;
spice_debug("trace");
//to handle better
for (i = 0; i < NUM_SURFACES; ++i) {
if (display->priv->surfaces[i].context.canvas) {
display_channel_destroy_surface_wait(display, i);
if (display->priv->surfaces[i].context.canvas) {
display_channel_surface_unref(display, &display->priv->surfaces[i]);
for (auto& surface : display->priv->surfaces) {
if (surface) {
display_channel_destroy_surface_wait(display, surface->id);
if (surface) {
display_channel_surface_unref(display, surface);
surface = nullptr;
}
spice_assert(!display->priv->surfaces[i].context.canvas);
}
}
spice_warn_if_fail(ring_is_empty(&display->priv->streams));
@ -2072,13 +2070,14 @@ create_canvas_for_surface(DisplayChannel *display, RedSurface *surface, uint32_t
return nullptr;
}
void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id, uint32_t width,
uint32_t height, int32_t stride, uint32_t format,
void *line_0, int data_is_valid, int send_client)
RedSurface*
display_channel_create_surface(DisplayChannel *display, uint32_t surface_id, uint32_t width,
uint32_t height, int32_t stride, uint32_t format,
void *line_0, int data_is_valid, int send_client)
{
RedSurface *surface = &display->priv->surfaces[surface_id];
RedSurface *surface = new RedSurface;
spice_warn_if_fail(!surface->context.canvas);
spice_warn_if_fail(!display->priv->surfaces[surface_id]);
surface->context.canvas_draws_on_surface = FALSE;
surface->context.width = width;
@ -2095,10 +2094,6 @@ void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id
}
g_warn_if_fail(!surface->create_cmd);
g_warn_if_fail(!surface->destroy_cmd);
ring_init(&surface->current);
ring_init(&surface->current_list);
ring_init(&surface->depend_on_me);
region_init(&surface->draw_dirty_region);
surface->refs = 1;
surface->id = surface_id;
@ -2118,10 +2113,26 @@ void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id
surface->context.canvas = create_canvas_for_surface(display, surface, display->priv->renderer);
}
spice_return_if_fail(surface->context.canvas);
if (!surface->context.canvas) {
delete surface;
return nullptr;
}
// finish initialization
ring_init(&surface->current);
ring_init(&surface->current_list);
ring_init(&surface->depend_on_me);
region_init(&surface->draw_dirty_region);
if (display->priv->surfaces[surface_id]) {
display_channel_surface_unref(display, display->priv->surfaces[surface_id]);
}
display->priv->surfaces[surface_id] = surface;
if (send_client) {
send_create_surface(display, surface, data_is_valid);
}
return surface;
}
void DisplayChannelClient::handle_migrate_flush_mark()
@ -2239,7 +2250,7 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
return;
}
surface = &display->priv->surfaces[surface_id];
surface = display->priv->surfaces[surface_id];
switch (surface_cmd->type) {
case QXL_SURFACE_CMD_CREATE: {
@ -2248,7 +2259,7 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
int32_t stride = create->stride;
int reloaded_surface = loadvm || (surface_cmd->flags & QXL_SURF_FLAG_KEEP_DATA);
if (surface->refs) {
if (surface) {
spice_warning("avoiding creating a surface twice");
break;
}
@ -2258,21 +2269,24 @@ void display_channel_process_surface_cmd(DisplayChannel *display,
* when it is read, specifically red_get_surface_cmd */
data -= (int32_t)(stride * (height - 1));
}
display_channel_create_surface(display, surface_id, create->width,
height, stride, create->format, data,
reloaded_surface,
// reloaded surfaces will be sent on demand
!reloaded_surface);
surface->create_cmd = surface_cmd;
surface = display_channel_create_surface(display, surface_id, create->width,
height, stride, create->format, data,
reloaded_surface,
// reloaded surfaces will be sent on demand
!reloaded_surface);
if (surface) {
surface->create_cmd = surface_cmd;
}
break;
}
case QXL_SURFACE_CMD_DESTROY:
if (!surface->refs) {
if (!surface) {
spice_warning("avoiding destroying a surface twice");
break;
}
surface->destroy_cmd = surface_cmd;
display_channel_destroy_surface(display, surface);
display->priv->surfaces[surface_id] = nullptr;
break;
default:
spice_warn_if_reached();
@ -2341,11 +2355,10 @@ RedSurface *display_channel_validate_surface(DisplayChannel *display, uint32_t s
spice_warning("invalid surface_id %u", surface_id);
return nullptr;
}
RedSurface *surface = &display->priv->surfaces[surface_id];
if (!surface->context.canvas) {
spice_warning("canvas address is %p for %d (and is NULL)",
&(surface->context.canvas), surface_id);
spice_warning("failed on %d", surface_id);
RedSurface *surface = display->priv->surfaces[surface_id];
if (!surface) {
spice_warning("surface %d is NULL", surface_id);
return nullptr;
}
return surface;

8
server/display-channel.h
View File

@ -95,10 +95,10 @@ display_channel_new(RedsState *reds, QXLInstance *qxl,
GArray *video_codecs,
uint32_t n_surfaces);
void display_channel_surface_id_unref(DisplayChannel *display, uint32_t surface_id);
void display_channel_create_surface (DisplayChannel *display, uint32_t surface_id,
uint32_t width, uint32_t height,
int32_t stride, uint32_t format, void *line_0,
int data_is_valid, int send_client);
RedSurface *display_channel_create_surface(DisplayChannel *display, uint32_t surface_id,
uint32_t width, uint32_t height,
int32_t stride, uint32_t format, void *line_0,
int data_is_valid, int send_client);
void display_channel_draw (DisplayChannel *display,
const SpiceRect *area,
int surface_id);

4
server/video-stream.cpp
View File

@ -852,11 +852,11 @@ static void dcc_detach_stream_gracefully(DisplayChannelClient *dcc,
stream_id, stream->current != nullptr);
rect_debug(&upgrade_area);
if (update_area_limit) {
display_channel_draw_until(display, &upgrade_area, &display->priv->surfaces[0], update_area_limit);
display_channel_draw_until(display, &upgrade_area, display->priv->surfaces[0], update_area_limit);
} else {
display_channel_draw(display, &upgrade_area, 0);
}
dcc_add_surface_area_image(dcc, &display->priv->surfaces[0], &upgrade_area,
dcc_add_surface_area_image(dcc, display->priv->surfaces[0], &upgrade_area,
dcc->get_pipe().end(), false);
}
clear_vis_region: