Hi Brendan,
Nicely done; this solves the problem I had with repeated test runs.
I have one trivial change to request:
On 8/5/19 2:12 PM, Brendan Shanks wrote:
The 'display->fullscreen' SHM segment was previously being used for both
x11spice's internal change scanning and as the spice primary surface.
I don't think spice wants anything else writing to the primary surface,
and this caused sporadic test failures and graphical corruption.
Create a separate SHM segment 'display->primary' and use it only for the
primary surface.
Signed-off-by: Brendan Shanks <bshanks@xxxxxxxxxxxxxxx>
---
src/display.c | 20 +++++++++++++++++++-
src/display.h | 1 +
src/main.c | 2 +-
src/session.c | 2 +-
4 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/display.c b/src/display.c
index 01e0e85..77b4d4e 100644
--- a/src/display.c
+++ b/src/display.c
@@ -461,12 +461,25 @@ void destroy_shm_image(display_t *d, shm_image_t *shmi)
int display_create_screen_images(display_t *d)
{
+ /* 'primary' and 'fullscreen' don't need to be SHM, normal buffers would work
+ * fine. Using SHM for all buffers is simpler though, and has no real downsides.
+ */
+ d->primary = create_shm_image(d, 0, 0);
+ if (!d->primary) {
+ return X11SPICE_ERR_NOSHM;
+ }
+
d->fullscreen = create_shm_image(d, 0, 0);
- if (!d->fullscreen)
+ if (!d->fullscreen) {
+ destroy_shm_image(d, d->primary);
+ d->primary = NULL;
return X11SPICE_ERR_NOSHM;
+ }
d->scanline = create_shm_image(d, 0, 1);
if (!d->scanline) {
+ destroy_shm_image(d, d->primary);
+ d->primary = NULL;
destroy_shm_image(d, d->fullscreen);
d->fullscreen = NULL;
return X11SPICE_ERR_NOSHM;
@@ -477,6 +490,11 @@ int display_create_screen_images(display_t *d)
void display_destroy_screen_images(display_t *d)
{
+ if (d->primary) {
+ destroy_shm_image(d, d->primary);
+ d->primary = NULL;
+ }
+
if (d->fullscreen) {
destroy_shm_image(d, d->fullscreen);
d->fullscreen = NULL;
diff --git a/src/display.h b/src/display.h
index dc4254b..27bc829 100644
--- a/src/display.h
+++ b/src/display.h
@@ -55,6 +55,7 @@ typedef struct {
const xcb_query_extension_reply_t *xfixes_ext;
+ shm_image_t *primary;
shm_image_t *fullscreen;
shm_image_t *scanline;
diff --git a/src/main.c b/src/main.c
index cecadc8..71cbb46 100644
--- a/src/main.c
+++ b/src/main.c
@@ -108,7 +108,7 @@ int main(int argc, char *argv[])
/*------------------------------------------------------------------------
** Start up a spice server
**----------------------------------------------------------------------*/
- rc = spice_start(&session.spice, &session.options, session.display.fullscreen);
+ rc = spice_start(&session.spice, &session.options, session.display.primary);
Could you modify the local variables inside the spice_start function to
use 'primary' instead of 'fullscreen' so we are not at a risk of
conflating them in the future?
Cheers,
Jeremy
if (rc)
goto exit;
spice_started = 1;
diff --git a/src/session.c b/src/session.c
index 1e59415..4d72e14 100644
--- a/src/session.c
+++ b/src/session.c
@@ -352,7 +352,7 @@ int session_recreate_primary(session_t *s)
rc = display_create_screen_images(&s->display);
if (rc == 0) {
- shm_image_t *f = s->display.fullscreen;
+ shm_image_t *f = s->display.primary;
rc = spice_create_primary(&s->spice, f->w, f->h, f->bytes_per_line, f->shmaddr);
}
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel