As we use reference counting is more direct to use direct pointers. Also this will allow to have a surface in a "released state" reducing the complexity of code destroying a surface. Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- server/dcc-send.c | 21 ++++++----- server/dcc.c | 21 ++++++----- server/display-channel.c | 91 +++++++++++++++++++++++------------------------- server/display-channel.h | 15 +++++--- 4 files changed, 78 insertions(+), 70 deletions(-) diff --git a/server/dcc-send.c b/server/dcc-send.c index c49adab..1e31584 100644 --- a/server/dcc-send.c +++ b/server/dcc-send.c @@ -316,7 +316,7 @@ static void fill_base(SpiceMarshaller *base_marshaller, Drawable *drawable) { SpiceMsgDisplayBase base; - base.surface_id = drawable->surface_id; + base.surface_id = drawable->surface->id; base.box = drawable->red_drawable->bbox; base.clip = drawable->red_drawable->clip; @@ -556,7 +556,7 @@ static void surface_lossy_region_update(DisplayChannelClient *dcc, return; } - surface_lossy_region = &dcc->priv->surface_client_lossy_region[item->surface_id]; + surface_lossy_region = &dcc->priv->surface_client_lossy_region[item->surface->id]; drawable = item->red_drawable; if (drawable->clip.type == SPICE_CLIP_TYPE_RECTS ) { @@ -653,7 +653,10 @@ static int drawable_depends_on_areas(Drawable *drawable, int surface_ids[], int dep_surface_id; for (x = 0; x < 3; ++x) { - dep_surface_id = drawable->surface_deps[x]; + if (drawable->surface_deps[x] == NULL) { + continue; + } + dep_surface_id = drawable->surface_deps[x]->id; if (dep_surface_id == surface_ids[i]) { if (rect_intersects(&surface_areas[i], &red_drawable->surfaces_rects[x])) { return TRUE; @@ -828,7 +831,7 @@ static void red_lossy_marshall_qxl_draw_fill(RedChannelClient *rcc, brush_is_lossy = is_brush_lossy(rcc, &drawable->u.fill.brush, &brush_bitmap_data); if (!dest_allowed_lossy) { - dest_is_lossy = is_surface_area_lossy(dcc, item->surface_id, &drawable->bbox, + dest_is_lossy = is_surface_area_lossy(dcc, item->surface->id, &drawable->bbox, &dest_lossy_area); } @@ -845,7 +848,7 @@ static void red_lossy_marshall_qxl_draw_fill(RedChannelClient *rcc, int num_resend = 0; if (dest_is_lossy) { - resend_surface_ids[num_resend] = item->surface_id; + resend_surface_ids[num_resend] = item->surface->id; resend_areas[num_resend] = &dest_lossy_area; num_resend++; } @@ -1148,7 +1151,7 @@ static void red_lossy_marshall_qxl_copy_bits(RedChannelClient *rcc, src_rect.right = drawable->bbox.right + horz_offset; src_rect.bottom = drawable->bbox.bottom + vert_offset; - src_is_lossy = is_surface_area_lossy(dcc, item->surface_id, + src_is_lossy = is_surface_area_lossy(dcc, item->surface->id, &src_rect, &src_lossy_area); surface_lossy_region_update(dcc, item, FALSE, src_is_lossy); @@ -1210,7 +1213,7 @@ static void red_lossy_marshall_qxl_draw_blend(RedChannelClient *rcc, } if (dest_is_lossy) { - resend_surface_ids[num_resend] = item->surface_id; + resend_surface_ids[num_resend] = item->surface->id; resend_areas[num_resend] = &dest_lossy_area; num_resend++; } @@ -1388,7 +1391,7 @@ static void red_lossy_marshall_qxl_draw_rop3(RedChannelClient *rcc, } if (dest_is_lossy) { - resend_surface_ids[num_resend] = item->surface_id; + resend_surface_ids[num_resend] = item->surface->id; resend_areas[num_resend] = &dest_lossy_area; num_resend++; } @@ -1469,7 +1472,7 @@ static void red_lossy_marshall_qxl_draw_composite(RedChannelClient *rcc, } if (dest_is_lossy) { - resend_surface_ids[num_resend] = item->surface_id; + resend_surface_ids[num_resend] = item->surface->id; resend_areas[num_resend] = &dest_lossy_area; num_resend++; } diff --git a/server/dcc.c b/server/dcc.c index ce8677d..af32de7 100644 --- a/server/dcc.c +++ b/server/dcc.c @@ -71,11 +71,16 @@ int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface GList *l; int x; RedChannelClient *rcc; + DisplayChannel *display; + RedSurface *surface; spice_return_val_if_fail(dcc != NULL, TRUE); /* removing the newest drawables that their destination is surface_id and no other drawable depends on them */ + display = DCC_TO_DC(dcc); + surface = &display->priv->surfaces[surface_id]; + rcc = RED_CHANNEL_CLIENT(dcc); for (l = rcc->priv->pipe.head; l != NULL; ) { Drawable *drawable; @@ -94,13 +99,13 @@ int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface continue; } - if (drawable->surface_id == surface_id) { + if (drawable->surface == surface) { red_channel_client_pipe_remove_and_release_pos(rcc, item_pos); continue; } for (x = 0; x < 3; ++x) { - if (drawable->surface_deps[x] == surface_id) { + if (drawable->surface_deps[x] == surface) { depend_found = TRUE; break; } @@ -251,8 +256,8 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra for (x = 0; x < 3; ++x) { int surface_id; - surface_id = drawable->surface_deps[x]; - if (surface_id != -1) { + if (drawable->surface_deps[x] != NULL) { + surface_id = drawable->surface_deps[x]->id; if (dcc->priv->surface_client_created[surface_id] == TRUE) { continue; } @@ -262,13 +267,13 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra } } - if (dcc->priv->surface_client_created[drawable->surface_id] == TRUE) { + if (dcc->priv->surface_client_created[drawable->surface->id] == TRUE) { return; } - dcc_create_surface(dcc, drawable->surface_id); - display_channel_current_flush(display, drawable->surface_id); - dcc_push_surface_image(dcc, drawable->surface_id); + dcc_create_surface(dcc, drawable->surface->id); + display_channel_current_flush(display, drawable->surface->id); + dcc_push_surface_image(dcc, drawable->surface->id); } static void red_drawable_pipe_item_free(RedPipeItem *item) diff --git a/server/display-channel.c b/server/display-channel.c index d7ea7d5..99082e6 100644 --- a/server/display-channel.c +++ b/server/display-channel.c @@ -176,6 +176,7 @@ void display_channel_surface_unref(DisplayChannel *display, uint32_t surface_id) DisplayChannelClient *dcc; GListIter iter; + spice_assert(surface->refs > 0); if (--surface->refs != 0) { return; } @@ -221,7 +222,7 @@ static void streams_update_visible_region(DisplayChannel *display, Drawable *dra return; } - if (!is_primary_surface(display, drawable->surface_id)) { + if (!is_primary_surface(display, drawable->surface->id)) { return; } @@ -305,9 +306,8 @@ static void current_add_drawable(DisplayChannel *display, Drawable *drawable, RingItem *pos) { RedSurface *surface; - uint32_t surface_id = drawable->surface_id; - surface = &display->priv->surfaces[surface_id]; + surface = drawable->surface; ring_add_after(&drawable->tree_item.base.siblings_link, pos); ring_add(&display->priv->current_list, &drawable->list_link); ring_add(&surface->current_list, &drawable->surface_list_link); @@ -625,7 +625,7 @@ static int current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawable // for now putting them on root. // only primary surface streams are supported - if (is_primary_surface(display, item->surface_id)) { + if (is_primary_surface(display, item->surface->id)) { stream_detach_behind(display, &shadow->base.rgn, NULL); } @@ -638,7 +638,7 @@ static int current_add_with_shadow(DisplayChannel *display, Ring *ring, Drawable region_destroy(&exclude_rgn); streams_update_visible_region(display, item); } else { - if (is_primary_surface(display, item->surface_id)) { + if (is_primary_surface(display, item->surface->id)) { stream_detach_behind(display, &item->tree_item.base.rgn, item); } } @@ -755,7 +755,7 @@ static int current_add(DisplayChannel *display, Ring *ring, Drawable *drawable) * stream_detach_behind */ current_add_drawable(display, drawable, ring); - if (is_primary_surface(display, drawable->surface_id)) { + if (is_primary_surface(display, drawable->surface->id)) { stream_detach_behind(display, &drawable->tree_item.base.rgn, drawable); } } @@ -773,7 +773,7 @@ static bool drawable_can_stream(DisplayChannel *display, Drawable *drawable) return FALSE; } - if (!is_primary_surface(display, drawable->surface_id)) { + if (!is_primary_surface(display, drawable->surface->id)) { return FALSE; } @@ -829,19 +829,21 @@ static void display_channel_print_stats(DisplayChannel *display) } #endif -static void drawable_ref_surface_deps(DisplayChannel *display, Drawable *drawable) +static void drawable_ref_surface_deps(DisplayChannel *display, Drawable *drawable, + RedDrawable *red_drawable) { int x; - int surface_id; - RedSurface *surface; for (x = 0; x < 3; ++x) { - surface_id = drawable->surface_deps[x]; + int surface_id = red_drawable->surface_deps[x]; if (surface_id == -1) { + drawable->surface_deps[x] = NULL; continue; } - surface = &display->priv->surfaces[surface_id]; + + RedSurface *surface = &display->priv->surfaces[surface_id]; surface->refs++; + drawable->surface_deps[x] = surface; } } @@ -867,7 +869,7 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable) int bpp; int all_set; - surface = &display->priv->surfaces[drawable->surface_id]; + surface = drawable->surface; bpp = SPICE_SURFACE_FMT_DEPTH(surface->context.format) / 8; width = red_drawable->self_bitmap_area.right - red_drawable->self_bitmap_area.left; @@ -890,13 +892,13 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable) image->u.bitmap.data = spice_chunks_new_linear(dest, height * dest_stride); image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE; - display_channel_draw(display, &red_drawable->self_bitmap_area, drawable->surface_id); - surface_read_bits(display, drawable->surface_id, + display_channel_draw(display, &red_drawable->self_bitmap_area, drawable->surface->id); + surface_read_bits(display, drawable->surface->id, &red_drawable->self_bitmap_area, dest, dest_stride); /* For 32bit non-primary surfaces we need to keep any non-zero high bytes as the surface may be used as source to an alpha_blend */ - if (!is_primary_surface(display, drawable->surface_id) && + if (!is_primary_surface(display, drawable->surface->id) && image->u.bitmap.format == SPICE_BITMAP_FMT_32BIT && rgb32_data_has_alpha(width, height, dest_stride, dest, &all_set)) { if (all_set) { @@ -909,18 +911,14 @@ static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable) red_drawable->self_bitmap_image = image; } -static void surface_add_reverse_dependency(DisplayChannel *display, int surface_id, - DependItem *depend_item, Drawable *drawable) +static void surface_add_reverse_dependency(DisplayChannel *display, RedSurface *surface, + DependItem *depend_item, Drawable *drawable) { - RedSurface *surface; - - if (surface_id == -1) { + if (surface == NULL) { depend_item->drawable = NULL; return; } - surface = &display->priv->surfaces[surface_id]; - depend_item->drawable = drawable; ring_add(&surface->depend_on_me, &depend_item->ring_item); } @@ -932,11 +930,11 @@ static int handle_surface_deps(DisplayChannel *display, Drawable *drawable) for (x = 0; x < 3; ++x) { // surface self dependency is handled by shadows in "current", or by // handle_self_bitmap - if (drawable->surface_deps[x] != drawable->surface_id) { + if (drawable->surface_deps[x] != drawable->surface) { surface_add_reverse_dependency(display, drawable->surface_deps[x], &drawable->depend_items[x], drawable); - if (drawable->surface_deps[x] == 0) { + if (drawable->surface_deps[x] != NULL && drawable->surface_deps[x]->id == 0) { QRegion depend_region; region_init(&depend_region); region_add(&depend_region, &drawable->red_drawable->surfaces_rects[x]); @@ -959,7 +957,7 @@ static void draw_depend_on_me(DisplayChannel *display, uint32_t surface_id) Drawable *drawable; DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, ring_item); drawable = depended_item->drawable; - display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface_id); + display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface->id); } } @@ -1027,17 +1025,16 @@ static Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t e drawable->tree_item.effect = effect; drawable->red_drawable = red_drawable_ref(red_drawable); - drawable->surface_id = red_drawable->surface_id; - display->priv->surfaces[drawable->surface_id].refs++; + drawable->surface = &display->priv->surfaces[red_drawable->surface_id]; + drawable->surface->refs++; - memcpy(drawable->surface_deps, red_drawable->surface_deps, sizeof(drawable->surface_deps)); /* surface->refs is affected by a drawable (that is dependent on the surface) as long as the drawable is alive. However, surface->depend_on_me is affected by a drawable only as long as it is in the current tree (hasn't been rendered yet). */ - drawable_ref_surface_deps(display, drawable); + drawable_ref_surface_deps(display, drawable, red_drawable); return drawable; } @@ -1048,7 +1045,7 @@ static Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t e */ static void display_channel_add_drawable(DisplayChannel *display, Drawable *drawable) { - int surface_id = drawable->surface_id; + int surface_id = drawable->surface->id; RedDrawable *red_drawable = drawable->red_drawable; red_drawable->mm_time = reds_get_mm_time(); @@ -1078,7 +1075,7 @@ static void display_channel_add_drawable(DisplayChannel *display, Drawable *draw return; } - Ring *ring = &display->priv->surfaces[surface_id].current; + Ring *ring = &drawable->surface->current; int add_to_pipe; if (has_shadow(red_drawable)) { add_to_pipe = current_add_with_shadow(display, ring, drawable); @@ -1319,11 +1316,10 @@ static void depended_item_remove(DependItem *item) static void drawable_remove_dependencies(DisplayChannel *display, Drawable *drawable) { int x; - int surface_id; for (x = 0; x < 3; ++x) { - surface_id = drawable->surface_deps[x]; - if (surface_id != -1 && drawable->depend_items[x].drawable) { + RedSurface* surface = drawable->surface_deps[x]; + if (surface != NULL && drawable->depend_items[x].drawable) { depended_item_remove(&drawable->depend_items[x]); } } @@ -1332,14 +1328,13 @@ static void drawable_remove_dependencies(DisplayChannel *display, Drawable *draw static void drawable_unref_surface_deps(DisplayChannel *display, Drawable *drawable) { int x; - int surface_id; for (x = 0; x < 3; ++x) { - surface_id = drawable->surface_deps[x]; - if (surface_id == -1) { + RedSurface *surface = drawable->surface_deps[x]; + if (surface == NULL) { continue; } - display_channel_surface_unref(display, surface_id); + display_channel_surface_unref(display, surface->id); } } @@ -1360,7 +1355,7 @@ void drawable_unref(Drawable *drawable) drawable_remove_dependencies(display, drawable); drawable_unref_surface_deps(display, drawable); - display_channel_surface_unref(display, drawable->surface_id); + display_channel_surface_unref(display, drawable->surface->id); glz_retention_detach_drawables(&drawable->glz_retention); @@ -1374,13 +1369,12 @@ void drawable_unref(Drawable *drawable) static void drawable_deps_draw(DisplayChannel *display, Drawable *drawable) { int x; - int surface_id; for (x = 0; x < 3; ++x) { - surface_id = drawable->surface_deps[x]; - if (surface_id != -1 && drawable->depend_items[x].drawable) { + RedSurface *surface = drawable->surface_deps[x]; + if (surface != NULL && drawable->depend_items[x].drawable) { depended_item_remove(&drawable->depend_items[x]); - display_channel_draw(display, &drawable->red_drawable->surfaces_rects[x], surface_id); + display_channel_draw(display, &drawable->red_drawable->surfaces_rects[x], surface->id); } } } @@ -1393,7 +1387,7 @@ static void drawable_draw(DisplayChannel *display, Drawable *drawable) drawable_deps_draw(display, drawable); - surface = &display->priv->surfaces[drawable->surface_id]; + surface = drawable->surface; canvas = surface->context.canvas; spice_return_if_fail(canvas); @@ -1596,7 +1590,7 @@ static Drawable* current_find_intersects_rect(Ring *current, RingItem *from, * FIXME: merge with display_channel_draw()? */ void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area, int surface_id, - Drawable *last) + Drawable *last) { RedSurface *surface; Drawable *surface_last = NULL; @@ -1609,13 +1603,13 @@ void display_channel_draw_until(DisplayChannel *display, const SpiceRect *area, surface = &display->priv->surfaces[surface_id]; - if (surface_id != last->surface_id) { + if (surface != last->surface) { // find the nearest older drawable from the appropriate surface ring = &display->priv->current_list; ring_item = &last->list_link; while ((ring_item = ring_next(ring, ring_item))) { now = SPICE_CONTAINEROF(ring_item, Drawable, list_link); - if (now->surface_id == surface_id) { + if (now->surface == surface) { surface_last = now; break; } @@ -1828,6 +1822,7 @@ void display_channel_create_surface(DisplayChannel *display, uint32_t surface_id ring_init(&surface->depend_on_me); region_init(&surface->draw_dirty_region); surface->refs = 1; + surface->id = surface_id; if (display->priv->renderer == RED_RENDERER_INVALID) { int i; diff --git a/server/display-channel.h b/server/display-channel.h index bbae6f1..8918ccb 100644 --- a/server/display-channel.h +++ b/server/display-channel.h @@ -44,6 +44,8 @@ #include "dcc.h" #include "image-encoders.h" +typedef struct RedSurface RedSurface; + typedef struct DependItem { Drawable *drawable; RingItem ring_item; @@ -69,8 +71,10 @@ struct Drawable { BitmapGradualType copy_bitmap_graduality; DependItem depend_items[3]; - int surface_id; - int surface_deps[3]; + /* destination surface. This pointer is not NULL. A reference is hold */ + RedSurface *surface; + /* dependency surfaces. They can be NULL. A reference is hold. */ + RedSurface *surface_deps[3]; uint32_t process_commands_generation; DisplayChannel *display; @@ -129,8 +133,9 @@ typedef struct DrawContext { void *line_0; } DrawContext; -typedef struct RedSurface { +struct RedSurface { uint32_t refs; + uint16_t id; Ring current; Ring current_list; DrawContext context; @@ -140,7 +145,7 @@ typedef struct RedSurface { //fix me - better handling here QXLReleaseInfoExt create, destroy; -} RedSurface; +}; #define NUM_DRAWABLES 1000 typedef struct _Drawable _Drawable; @@ -382,7 +387,7 @@ static inline int is_drawable_independent_from_surfaces(Drawable *drawable) int x; for (x = 0; x < 3; ++x) { - if (drawable->surface_deps[x] != -1) { + if (drawable->surface_deps[x]) { return FALSE; } } -- 2.7.4 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel