From 5d033bc93f3ee36cb89a0cbdc0bb086ef2612ac3 Mon Sep 17 00:00:00 2001 From: hujianwei Date: Fri, 4 Nov 2022 05:36:10 +0000 Subject: [PATCH] Add new damage test and unify buffer release code in xdg_surface * 12to11-test.xml (test_surface): : Document when frame callbacks are run. * 12to11.c (HandleCmdline): Accept new arg `printsocket'. (XLMain): Pass socket to HandleCmdline. * 12to11.man: Document new `printsocket' option. * mime0.awk: * mime1.awk: * mime2.awk: * mime3.awk: * mime4.awk: Fix typos in copyright blurb. * test.c (NoteBounds): Sync with X server after XResizeWindow. (Commit): Run frame callbacks after unmap as well. (GetTestSurface): Make windows override redirect. * tests/Imakefile (SRCS2, OBJS2): New program. (PROGRAMS): Add damage_test. (damage_test): New program. Depend on SRCS2. * tests/README: Improve documentation on how to run tests. * tests/simple_test.c (test_names): New list. (LAST_TEST): New define. (verify_single_step): Complete test upon last test being run. (test_single_step): Submit correct surface damage. (test_next_step): New function. Does nothing here. (handle_wl_callback_done): Run the next test. (submit_surface_damage): New function. * tests/svnignore.txt: Add damage_test and Makefile.bak. * tests/test_harness.c (test_complete): New function. * tests/test_harness.h: New prototypes. * xdg_surface.c (struct _XdgRole): Use a buffer release helper. (struct _ReleaseLaterRecord, DeleteRecord, FreeRecords) (AddRecordAfter): Delete unused structs and functions. (RunFrameCallbacksConditionally, BufferIdleCallback) (ReleaseBacking, ReleaseBuffer, XLGetXdgSurface): Use and free buffer release helper; run frame callbacks from its callback. --- 12to11-test.xml | 3 + 12to11.c | 9 ++- 12to11.man | 5 ++ mime0.awk | 2 +- mime1.awk | 2 +- mime2.awk | 2 +- mime3.awk | 2 +- mime4.awk | 2 +- test.c | 16 ++++- tests/Imakefile | 7 ++- tests/README | 16 ++++- tests/simple_test.c | 58 ++++++++++++++++++- tests/svnignore.txt | 2 + tests/test_harness.c | 7 +++ tests/test_harness.h | 4 +- xdg_surface.c | 135 ++++++------------------------------------- 16 files changed, 134 insertions(+), 138 deletions(-) diff --git a/12to11-test.xml b/12to11-test.xml index 1b0946d..af8e62f 100644 --- a/12to11-test.xml +++ b/12to11-test.xml @@ -67,6 +67,9 @@ This role provides a test surface. Various buffers and subsurfaces can be attached, and the resulting display contents validated. + + When a buffer is commited to a test surface, the frame callback + is run after any window configuration or resize has completed. diff --git a/12to11.c b/12to11.c index 5e4839a..f447e6a 100644 --- a/12to11.c +++ b/12to11.c @@ -74,7 +74,7 @@ DetermineServerTime (void) } static void -HandleCmdline (Display *dpy, int argc, char **argv) +HandleCmdline (Display *dpy, const char *socket, int argc, char **argv) { int i; XrmDatabase rdb, initial_rdb; @@ -107,7 +107,8 @@ HandleCmdline (Display *dpy, int argc, char **argv) { print_usage: fprintf (stderr, - "usage: %s [-name name] [-class class] [-xrm resourcestring...]\n", + "usage: %s [-name name] [-class class] [-printsocket]" + " [-xrm resourcestring...]\n", argv[0]); exit (!strcmp (argv[i], "-help") ? 0 : 1); } @@ -144,6 +145,8 @@ HandleCmdline (Display *dpy, int argc, char **argv) XrmPutLineResource (&rdb, argv[++i]); } + else if (!strcmp (argv[i], "-printsocket")) + puts (socket); else { fprintf (stderr, "%s: bad command line option \"%s\"\n", @@ -194,7 +197,7 @@ XLMain (int argc, char **argv) XGetDefault (dpy, "dummmy", "value"); /* Parse command-line arguments. */ - HandleCmdline (dpy, argc, argv); + HandleCmdline (dpy, socket, argc, argv); compositor.display = dpy; compositor.conn = XGetXCBConnection (dpy); diff --git a/12to11.man b/12to11.man index 0d754f3..0b85d82 100644 --- a/12to11.man +++ b/12to11.man @@ -36,6 +36,11 @@ after printing the message. This option specifies a resource string to be used. This is especially useful for setting resources that do not have separate command line options. +.TP +.B \-printsocket\fP +This option makes the protocol translator print the name of the +Wayland socket to standard output. +.TP .SH RESOURCES \fI12to11\fP understands some resource names and classes that can be used to specify various settings that affect its behavior. Those diff --git a/mime0.awk b/mime0.awk index 0197697..ebd1664 100644 --- a/mime0.awk +++ b/mime0.awk @@ -1,4 +1,4 @@ -# Wayland compositor running on top of an X serer. +# Wayland compositor running on top of an X server. # Copyright (C) 2022 to various contributors. diff --git a/mime1.awk b/mime1.awk index a23f7ee..ddf74b4 100644 --- a/mime1.awk +++ b/mime1.awk @@ -1,4 +1,4 @@ -# Wayland compositor running on top of an X serer. +# Wayland compositor running on top of an X server. # Copyright (C) 2022 to various contributors. diff --git a/mime2.awk b/mime2.awk index 45fd021..3673b78 100644 --- a/mime2.awk +++ b/mime2.awk @@ -1,4 +1,4 @@ -# Wayland compositor running on top of an X serer. +# Wayland compositor running on top of an X server. # Copyright (C) 2022 to various contributors. diff --git a/mime3.awk b/mime3.awk index 65478ba..a309953 100644 --- a/mime3.awk +++ b/mime3.awk @@ -1,4 +1,4 @@ -# Wayland compositor running on top of an X serer. +# Wayland compositor running on top of an X server. # Copyright (C) 2022 to various contributors. diff --git a/mime4.awk b/mime4.awk index 9e8c6b7..fc51bcd 100644 --- a/mime4.awk +++ b/mime4.awk @@ -1,4 +1,4 @@ -# Wayland compositor running on top of an X serer. +# Wayland compositor running on top of an X server. # Copyright (C) 2022 to various contributors. diff --git a/test.c b/test.c index 4a99854..8e36110 100644 --- a/test.c +++ b/test.c @@ -151,6 +151,9 @@ NoteBounds (void *data, int min_x, int min_y, int max_x, int max_y) /* Resize the window to fit. */ XResizeWindow (compositor.display, test->window, bounds_width, bounds_height); + /* Sync with the X server. */ + XSync (compositor.display, False); + test->bounds_width = bounds_width; test->bounds_height = bounds_height; } @@ -206,8 +209,13 @@ Commit (Surface *surface, Role *role) /* Map the surface now. */ MapTestSurface (test); else if (!surface->current_state.buffer) - /* Unmap the surface now. */ - UnmapTestSurface (test); + { + /* Unmap the surface now. */ + UnmapTestSurface (test); + + /* Run frame callbacks if necessary. */ + RunFrameCallbacksConditionally (test); + } /* Finally, do a subcompositor update if the surface is now mapped. */ @@ -389,7 +397,9 @@ GetTestSurface (struct wl_client *client, struct wl_resource *resource, attrs.border_pixel = border_pixel; attrs.event_mask = DefaultEventMask; attrs.cursor = InitDefaultCursor (); - flags = CWColormap | CWBorderPixel | CWEventMask | CWCursor; + attrs.override_redirect = True; + flags = (CWColormap | CWBorderPixel | CWEventMask + | CWCursor | CWOverrideRedirect); test->window = XCreateWindow (compositor.display, DefaultRootWindow (compositor.display), diff --git a/tests/Imakefile b/tests/Imakefile index 2fb7655..1067a7c 100644 --- a/tests/Imakefile +++ b/tests/Imakefile @@ -23,7 +23,9 @@ ScannerTarget(12to11-test) SRCS1 = $(COMMONSRCS) simple_test.c OBJS1 = $(COMMONOBJS) simple_test.o - PROGRAMS = simple_test + SRCS2 = $(COMMONSRCS) damage_test.c + OBJS2 = $(COMMONOBJS) damage_test.o + PROGRAMS = simple_test damage_test /* Make all objects depend on HEADER. */ $(OBJS1): $(HEADER) @@ -32,6 +34,7 @@ $(OBJS1): $(HEADER) depend:: $(HEADER) $(SRCS1) NormalProgramTarget(simple_test,$(OBJS1),NullParameter,$(LOCAL_LIBRARIES),NullParameter) -DependTarget3($(SRCS1),NullParameter,NullParameter) +NormalProgramTarget(damage_test,$(OBJS2),NullParameter,$(LOCAL_LIBRARIES),NullParameter) +DependTarget3($(SRCS1),$(SRCS2),NullParameter) all:: $(PROGRAMS) diff --git a/tests/README b/tests/README index b236882..d9d7407 100644 --- a/tests/README +++ b/tests/README @@ -1,6 +1,16 @@ This directory holds some work-in-progress code for testing the -protocol translator. At present, there is only a simple smoke test of -limited utility. +protocol translator. The current test suite is nowhere near +comprehensive. Each test must be individually run on a system with an a8r8g8b8 -visual, GLOBAL_SCALE and OUTPUT_SCALE set to 1. +visual, GLOBAL_SCALE and OUTPUT_SCALE set to 1. They also rely on +reference data; if some legitimate changes are made that affect test +results, then the tests should be run with TEST_WRITE_REFERENCE=1, +which will make the test binaries write out reference data to disk. + +When tests are being run, the tester must be very careful to not +interfere with the test operation by moving or resizing the test +window. A compositing manager should be running along with the test. + +Most likely, you do not want to run these tests manually, as the +`run_tests.sh' script does all the setup for you. diff --git a/tests/simple_test.c b/tests/simple_test.c index b3a732f..d43b9a3 100644 --- a/tests/simple_test.c +++ b/tests/simple_test.c @@ -25,6 +25,14 @@ enum test_kind BASIC_TEST_CARD_IMAGE_KIND, }; +static const char *test_names[] = + { + "map_window", + "basic_test_card_image", + }; + +#define LAST_TEST BASIC_TEST_CARD_IMAGE_KIND + /* The display. */ static struct test_display *display; @@ -45,13 +53,26 @@ static struct wl_surface *wayland_surface; /* Forward declarations. */ static void submit_frame_callback (struct wl_surface *, enum test_kind); +static void submit_surface_damage (struct wl_surface *, int, int, int, int); static void verify_single_step (enum test_kind kind) { - verify_image_data (display, test_surface_window, "simple_test.dump"); + switch (kind) + { + case BASIC_TEST_CARD_IMAGE_KIND: + verify_image_data (display, test_surface_window, + "simple_test.dump"); + break; + + default: + break; + } + + if (kind == LAST_TEST) + test_complete (); } static void @@ -59,6 +80,8 @@ test_single_step (enum test_kind kind) { struct wl_buffer *buffer; + test_log ("running test step: %s", test_names[kind]); + switch (kind) { case MAP_WINDOW_KIND: @@ -68,6 +91,8 @@ test_single_step (enum test_kind kind) report_test_failure ("failed to load blue.png"); wl_surface_attach (wayland_surface, buffer, 0, 0); + submit_surface_damage (wayland_surface, 0, 0, + INT_MAX, INT_MAX); wl_surface_commit (wayland_surface); wl_buffer_destroy (buffer); break; @@ -80,12 +105,24 @@ test_single_step (enum test_kind kind) wl_surface_attach (wayland_surface, buffer, 0, 0); submit_frame_callback (wayland_surface, kind); + submit_surface_damage (wayland_surface, 0, 0, + INT_MAX, INT_MAX); wl_surface_commit (wayland_surface); wl_buffer_destroy (buffer); break; } } +static void +test_next_step (enum test_kind kind) +{ + switch (kind) + { + default: + break; + } +} + static void @@ -116,9 +153,15 @@ handle_wl_callback_done (void *data, struct wl_callback *callback, { enum test_kind kind; + /* kind is not a pointer. It is an enum test_kind stuffed into a + pointer. */ kind = (intptr_t) data; + wl_callback_destroy (callback); verify_single_step (kind); + + /* Now run the next test in this sequence. */ + test_next_step (kind); } static const struct wl_callback_listener wl_callback_listener = @@ -134,9 +177,18 @@ submit_frame_callback (struct wl_surface *surface, enum test_kind kind) struct wl_callback *callback; callback = wl_surface_frame (surface); - wl_callback_set_user_data (callback, (void *) (intptr_t) kind); wl_callback_add_listener (callback, &wl_callback_listener, - NULL); + (void *) (intptr_t) kind); +} + +static void +submit_surface_damage (struct wl_surface *surface, int x, int y, int width, + int height) +{ + test_log ("damaging surface by %d, %d, %d, %d", x, y, width, + height); + + wl_surface_damage (surface, x, y, width, height); } static void diff --git a/tests/svnignore.txt b/tests/svnignore.txt index ccbcac7..65d94fe 100644 --- a/tests/svnignore.txt +++ b/tests/svnignore.txt @@ -2,4 +2,6 @@ *-*.c vgcore* simple_test +damage_test Makefile +Makefile.bak diff --git a/tests/test_harness.c b/tests/test_harness.c index 826e62c..fc3319a 100644 --- a/tests/test_harness.c +++ b/tests/test_harness.c @@ -751,3 +751,10 @@ test_init (void) write_image_data_instead = getenv ("TEST_WRITE_REFERENCE") != NULL; } + +void __attribute__ ((noreturn)) +test_complete (void) +{ + test_log ("test ran successfully"); + exit (0); +} diff --git a/tests/test_harness.h b/tests/test_harness.h index 0e97f40..0ca48bd 100644 --- a/tests/test_harness.h +++ b/tests/test_harness.h @@ -20,6 +20,7 @@ along with 12to11. If not, see . */ #include #include #include +#include #include @@ -70,7 +71,7 @@ struct test_interface uint32_t version; }; -extern void die (const char *); +extern void die (const char *) __attribute__ ((noreturn)); extern struct test_display *open_test_display (struct test_interface *, int); extern int get_shm_file_descriptor (void); extern size_t get_image_stride (struct test_display *, int, int); @@ -87,5 +88,6 @@ extern bool make_test_surface (struct test_display *, struct wl_surface **, extern struct wl_buffer *load_png_image (struct test_display *, const char *); extern void verify_image_data (struct test_display *, Window, const char *); extern void test_init (void); +extern void test_complete (void) __attribute__ ((noreturn)); #define ARRAYELTS(arr) (sizeof (arr) / sizeof (arr)[0]) diff --git a/xdg_surface.c b/xdg_surface.c index 0389233..0d61240 100644 --- a/xdg_surface.c +++ b/xdg_surface.c @@ -46,6 +46,7 @@ enum StateTemporaryBounds = (1 << 8), StateFrameStarted = (1 << 9), StateAllowUnredirection = (1 << 10), + StatePendingBufferRelease = (1 << 11), }; typedef struct _XdgRole XdgRole; @@ -124,9 +125,8 @@ struct _XdgRole /* Various role state. */ int state; - /* Queue of buffers to release later (when the X server is done with - them). */ - ReleaseLaterRecord *release_records; + /* Buffer release helper. */ + BufferReleaseHelper *release_helper; /* The frame clock. */ FrameClock *clock; @@ -166,25 +166,6 @@ struct _XdgRole XdgRoleImplementationType type; }; -struct _ReleaseLaterRecord -{ - /* A monotonically (overflow aside) increasing identifier. */ - uint64_t id; - - /* The buffer that should be released upon receiving this - message. */ - ExtBuffer *buffer; - - /* The idle callback. */ - IdleCallbackKey key; - - /* The XdgRole. */ - XdgRole *role; - - /* The next and last records. */ - ReleaseLaterRecord *next, *last; -}; - struct _PingEvent { /* Function called to reply to this event. */ @@ -197,61 +178,6 @@ struct _PingEvent /* Event base of the XShape extension. */ int shape_base; -static void -DeleteRecord (ReleaseLaterRecord *record) -{ - /* Removing the sentinel record is invalid. */ - XLAssert (record->buffer != NULL); - - /* First, make the rest of the list skip RECORD. */ - record->last->next = record->next; - record->next->last = record->last; - - /* Finally, free RECORD. */ - XLFree (record); -} - -static void -FreeRecords (ReleaseLaterRecord *records) -{ - ReleaseLaterRecord *last, *tem; - - tem = records->next; - - while (tem != records) - { - last = tem; - tem = tem->next; - - /* Cancel the idle callback if it already exists. */ - if (last->key) - RenderCancelIdleCallback (last->key); - - /* Release the buffer now. */ - XLReleaseBuffer (last->buffer); - - /* Before freeing the record itself. */ - XLFree (last); - } - - XLFree (records); -} - -static ReleaseLaterRecord * -AddRecordAfter (ReleaseLaterRecord *start) -{ - ReleaseLaterRecord *record; - - record = XLMalloc (sizeof *record); - record->next = start->next; - record->last = start; - - start->next->last = record; - start->next = record; - - return record; -} - static ReconstrainCallback * AddCallbackAfter (ReconstrainCallback *start) { @@ -345,8 +271,7 @@ RunFrameCallbacks (Surface *surface, XdgRole *role) static void RunFrameCallbacksConditionally (XdgRole *role) { - if (role->release_records->last == role->release_records - && role->role.surface) + if (!(role->state & StatePendingBufferRelease)) RunFrameCallbacks (role->role.surface, role); else if (role->role.surface) /* weston-simple-shm seems to assume that a frame callback can @@ -355,24 +280,20 @@ RunFrameCallbacksConditionally (XdgRole *role) } static void -BufferIdleCallback (RenderBuffer buffer, void *data) +AllBuffersReleased (void *data) { - ReleaseLaterRecord *record; XdgRole *role; Surface *surface; - record = data; - role = record->role; - - XLReleaseBuffer (record->buffer); - DeleteRecord (record); - + role = data; surface = role->role.surface; - /* Run frame callbacks now, if no more buffers are waiting to be + /* Clear the buffer release flag. */ + role->state &= ~StatePendingBufferRelease; + + /* Run frame callbacks now, as no more buffers are waiting to be released. */ - if (surface && role->state & StatePendingFrameCallback - && role->release_records->next == role->release_records) + if (surface && role->state & StatePendingFrameCallback) { RunFrameCallbacks (surface, role); @@ -794,9 +715,7 @@ ReleaseBacking (XdgRole *role) /* Release all buffers pending release. The sync is necessary because the X server does not perform operations immediately after the Xlib function is called. */ - - XSync (compositor.display, False); - FreeRecords (role->release_records); + FreeBufferReleaseHelper (role->release_helper); /* Now release the reference to any toplevel implementation that might be attached. */ @@ -858,7 +777,6 @@ static void ReleaseBuffer (Surface *surface, Role *role, ExtBuffer *buffer) { RenderBuffer render_buffer; - ReleaseLaterRecord *record; XdgRole *xdg_role; render_buffer = XLRenderBufferFromBuffer (buffer); @@ -870,13 +788,9 @@ ReleaseBuffer (Surface *surface, Role *role, ExtBuffer *buffer) else { /* Release the buffer once it is destroyed or becomes idle. */ - record = AddRecordAfter (xdg_role->release_records); - record->buffer = buffer; - record->key = RenderAddIdleCallback (render_buffer, - xdg_role->target, - BufferIdleCallback, - record); - record->role = xdg_role; + ReleaseBufferWithHelper (xdg_role->release_helper, + buffer, xdg_role->target); + xdg_role->state |= StatePendingBufferRelease; } } @@ -1518,24 +1432,12 @@ XLGetXdgSurface (struct wl_client *client, struct wl_resource *resource, memset (role, 0, sizeof *role); - role->release_records - = XLSafeMalloc (sizeof *role->release_records); - - if (!role->release_records) - { - XLFree (role); - wl_client_post_no_memory (client); - - return; - } - role->role.resource = wl_resource_create (client, &xdg_surface_interface, wl_resource_get_version (resource), id); if (!role->role.resource) { - XLFree (role->release_records); XLFree (role); wl_client_post_no_memory (client); @@ -1579,17 +1481,14 @@ XLGetXdgSurface (struct wl_client *client, struct wl_resource *resource, attrs.cursor = InitDefaultCursor (); flags = CWColormap | CWBorderPixel | CWEventMask | CWCursor; - /* Sentinel node. */ - role->release_records->next = role->release_records; - role->release_records->last = role->release_records; - role->release_records->buffer = NULL; - role->window = XCreateWindow (compositor.display, DefaultRootWindow (compositor.display), 0, 0, 20, 20, 0, compositor.n_planes, InputOutput, compositor.visual, flags, &attrs); role->target = RenderTargetFromWindow (role->window, DefaultEventMask); + role->release_helper = MakeBufferReleaseHelper (AllBuffersReleased, + role); role->subcompositor = MakeSubcompositor (); role->clock = XLMakeFrameClockForWindow (role->window);