Re: [PATCH weston v2] tests: fix the cursor race in internal-screenshot
On Thu, 23 Jun 2016 09:45:56 -0500 Derek Foreman wrote: > On 23/06/16 07:17 AM, Pekka Paalanen wrote: > > From: Pekka Paalanen > > > > This fix also depends on "compositor-headless: do not create a seat". > > > > If we lose the race against weston-desktop-shell setting cursors, which > > is very rare, we get a cursor image in the screenshot, causing the test > > to fail. This is now fixed by moving the (remaining) cursor out of the > > way. > > > > Arguably we should have better solutions for this, but that is another > > story. This is a stop-gap measure we can copy also in new > > screenshooting tests. > > > > v2: Remove the example code for how to trigger the race, and rewrite the > > big comment. > > > > Cc: Derek Foreman > > Signed-off-by: Pekka Paalanen > > I find this exceptionally clear, thanks! > > Reviewed-by: Derek Foreman Cool, pushed: a1046ad..e651bb0 master -> master Thanks, pq > > --- > > tests/internal-screenshot-test.c | 18 ++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/tests/internal-screenshot-test.c > > b/tests/internal-screenshot-test.c > > index 563aa3d..0b2cb1e 100644 > > --- a/tests/internal-screenshot-test.c > > +++ b/tests/internal-screenshot-test.c > > @@ -68,6 +68,24 @@ TEST(internal_screenshot) > > assert(client); > > surface = client->surface->wl_surface; > > > > + /* > > +* We are racing our screenshooting against weston-desktop-shell > > +* setting the cursor. If w-d-s wins, our screenshot will have a cursor > > +* shown, which makes the image comparison fail. Our window and the > > +* default pointer position are accidentally causing an overlap that > > +* intersects our test clip rectangle. > > +* > > +* w-d-s wins very rarely though, so the race is easy to miss. You can > > +* make it happen by putting a delay before the call to > > +* create_client_and_test_surface(). > > +* > > +* The weston_test_move_pointer() below makes the race irrelevant, as > > +* the cursor won't overlap with anything we care about. > > +*/ > > + > > + /* Move the pointer away from the screenshot area. */ > > + weston_test_move_pointer(client->test->weston_test, 0, 0); > > + > > buf = create_shm_buffer(client, 100, 100, &pixels); > > draw_stuff(pixels, 100, 100); > > wl_surface_attach(surface, buf, 0, 0); > > > pgpmjbKpLfPbC.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v2] tests: fix the cursor race in internal-screenshot
On 23/06/16 07:17 AM, Pekka Paalanen wrote: > From: Pekka Paalanen > > This fix also depends on "compositor-headless: do not create a seat". > > If we lose the race against weston-desktop-shell setting cursors, which > is very rare, we get a cursor image in the screenshot, causing the test > to fail. This is now fixed by moving the (remaining) cursor out of the > way. > > Arguably we should have better solutions for this, but that is another > story. This is a stop-gap measure we can copy also in new > screenshooting tests. > > v2: Remove the example code for how to trigger the race, and rewrite the > big comment. > > Cc: Derek Foreman > Signed-off-by: Pekka Paalanen I find this exceptionally clear, thanks! Reviewed-by: Derek Foreman > --- > tests/internal-screenshot-test.c | 18 ++ > 1 file changed, 18 insertions(+) > > diff --git a/tests/internal-screenshot-test.c > b/tests/internal-screenshot-test.c > index 563aa3d..0b2cb1e 100644 > --- a/tests/internal-screenshot-test.c > +++ b/tests/internal-screenshot-test.c > @@ -68,6 +68,24 @@ TEST(internal_screenshot) > assert(client); > surface = client->surface->wl_surface; > > + /* > + * We are racing our screenshooting against weston-desktop-shell > + * setting the cursor. If w-d-s wins, our screenshot will have a cursor > + * shown, which makes the image comparison fail. Our window and the > + * default pointer position are accidentally causing an overlap that > + * intersects our test clip rectangle. > + * > + * w-d-s wins very rarely though, so the race is easy to miss. You can > + * make it happen by putting a delay before the call to > + * create_client_and_test_surface(). > + * > + * The weston_test_move_pointer() below makes the race irrelevant, as > + * the cursor won't overlap with anything we care about. > + */ > + > + /* Move the pointer away from the screenshot area. */ > + weston_test_move_pointer(client->test->weston_test, 0, 0); > + > buf = create_shm_buffer(client, 100, 100, &pixels); > draw_stuff(pixels, 100, 100); > wl_surface_attach(surface, buf, 0, 0); > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v2] tests: fix the cursor race in internal-screenshot
From: Pekka Paalanen This fix also depends on "compositor-headless: do not create a seat". If we lose the race against weston-desktop-shell setting cursors, which is very rare, we get a cursor image in the screenshot, causing the test to fail. This is now fixed by moving the (remaining) cursor out of the way. Arguably we should have better solutions for this, but that is another story. This is a stop-gap measure we can copy also in new screenshooting tests. v2: Remove the example code for how to trigger the race, and rewrite the big comment. Cc: Derek Foreman Signed-off-by: Pekka Paalanen --- tests/internal-screenshot-test.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/tests/internal-screenshot-test.c b/tests/internal-screenshot-test.c index 563aa3d..0b2cb1e 100644 --- a/tests/internal-screenshot-test.c +++ b/tests/internal-screenshot-test.c @@ -68,6 +68,24 @@ TEST(internal_screenshot) assert(client); surface = client->surface->wl_surface; + /* +* We are racing our screenshooting against weston-desktop-shell +* setting the cursor. If w-d-s wins, our screenshot will have a cursor +* shown, which makes the image comparison fail. Our window and the +* default pointer position are accidentally causing an overlap that +* intersects our test clip rectangle. +* +* w-d-s wins very rarely though, so the race is easy to miss. You can +* make it happen by putting a delay before the call to +* create_client_and_test_surface(). +* +* The weston_test_move_pointer() below makes the race irrelevant, as +* the cursor won't overlap with anything we care about. +*/ + + /* Move the pointer away from the screenshot area. */ + weston_test_move_pointer(client->test->weston_test, 0, 0); + buf = create_shm_buffer(client, 100, 100, &pixels); draw_stuff(pixels, 100, 100); wl_surface_attach(surface, buf, 0, 0); -- 2.7.3 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
