[Spice-devel] [PATCH RFC 01/14] Use direct pointers for surface and surface dependencies from Drawable
Frediano Ziglio
fziglio at redhat.com
Thu Sep 29 08:44:00 UTC 2016
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 at redhat.com>
---
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
More information about the Spice-devel
mailing list