Release blocker (Re: [PATCH weston v3 00/15] Implement screenshot-based testing with the headless renderer)

[Pekka Paalanen]

Hi,

I confirm the out-of-tree test error is now fixed, thanks!


On Fri, 22 May 2015 19:19:46 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> On Fri, May 22, 2015 at 10:34:00AM +0300, Pekka Paalanen wrote:
> > 
> > Another failure:
> > 
> > $ make check TESTS=internal-screenshot.weston WESTON_TEST_REFERENCE_PATH=~/git/weston/tests/reference BACKEND=x11-backend.so
> > 
> > FAIL: internal-screenshot.weston
> > test-client: got seat 0x73a6e0 capabilities: 3
> > test-client: got seat 0x73a6e0 name: 'default'
> > test-client: got global pointer 100 100
> > test-client: got seat 0x73abe0 capabilities: 7
> > test-client: got seat 0x73abe0 name: 'test-seat'
> > test-client: got keyboard repeat_info 40 400
> > test-client: got keyboard keymap
> > test-client: got surface enter output 0x73a820
> > test-client: got keyboard modifiers 0 0 0 0
> > test-client: got pointer enter 0 0, surface 0x73ad50
> > Creating client for test
> > Taking a screenshot
> > Screenshot has been captured
> > Loading reference image /home/pq/git/weston/tests/reference/internal-screenshot-00.png
> > Loading reference image /home/pq/git/weston/tests/reference/internal-screenshot-00.png
> > Screenshot different from reference image
> > Clip: 50,50 101 x 101
> > Bytewise comparison inside clip
> > Mismatched image on row 50
> > 50,0:        0       7b  <---
> > 50,1:        0       7b  <---
> > ...
> > 50,374:        0       7b  <---
> > 50,375:        0       7b  internal-screenshot.weston: /home/pq/git/weston/tests/internal-screenshot-test.c:242: inter
> > nal_screenshot: Assertion `match' failed.
> > <---
> > 50,376:        0       7b  <---
> > 50,377:        0       7b  <---
> > ...
> > Screenshot doesn't match reference image in clipped area
> > Writing PNG to disk
> > Test complete
> > test "internal_screenshot":     signal 6, fail.
> > 1 tests, 0 pass, 0 skip, 1 fail
> > 
> > 
> > The written screenshot is completely black. I believe this is still due
> > to the race between weston-desktop-shell and the test client.
> 
> This afternoon I've tried to reproduce this behavior, but it seems to be
> working for me.  Derek also is unable to reproduce.
> 
> I did discover that if I ssh -X in to the box and run make check over
> ssh using the X11 backend, then I get a test failure.  But that's not
> due to the test; same failure occurs over ssh if I run
> 
>  ../weston-build/weston --use-pixman --backend=x11-backend.so

Maybe our X11 backend does not cope with SHM being unavailable?
And the GL-renderer probably fails because I don't think shaders can be
communicated over network or maybe EGL does not even have indirect
support. Or we manage to reject indirect rendering like we should.

But indeed, all that is irrelevant.

> The serverlog shows this causes an xcb "error 10" whatever that means.
> But this is completely unrelated to your problem (and probably
> irrelevant to anyone but me).
> 
> Anyway, I'll hold off on doing the release until you're happy.

Cool, thanks.

I just tried a simple 'make check' with the latest weston, with the
out-of-tree test fix commit as the top, and the screenshot test failed
4 times out of 6 runs. I didn't do anything more fancy than just 'make
check' manually one by one.

My old analysis of the problem is here:
http://lists.freedesktop.org/archives/wayland-devel/2015-May/021908.html
and I believe it still holds.

If you cannot reproduce, put a sleep(1) in clients/desktop-shell.c,
early in main(). That should make this failure happen every time by
simulating a slow startup of weston-desktop-shell.

A good way to fix it would be to not rely on weston-desktop-shell at
all:
- the test client has to draw an opaque image and post it on the test
  surface
- take a screenshot
- find out in which coordinates the test surface was mapped (I think
  weston_test protocol can provide that, not by telling you where the
  surface is but allowing you to position the surface absolutely)
- compare the sub-rectangle of the surface on the screenshot to the
  buffer that was posted (this also means you don't really need a
  reference image as a PNG file in this case because you can generate
  it procedurally by drawing to the buffer)

You could use simple-shm.c's paint_pixels() as an example of drawing,
but really just filling with an opaque color would be enough for now.

Looks like create_client_and_test_surface() already does the
wl_surface positioning, so you know it will be at the x,y given as
arguments. Now you have to make a new buffer with create_shm_buffer(),
draw something opaque in it, and post it (attach, damage, commit).

Taking the screenshot implicitly ensures the new surface content will
be composited.

Note that check_surfaces_match_in_clip() cannot be used as is for the
comparison, because now you have one buffer to be used as a whole (the
one you drew) and the other to clip (a sub-rectangle from the
screenshot). An easy solution to that is to pass in a separate clip for
each image.

If you think this takes more than couple of days, then I would
recommend just disabling this test for the 1.8.0 release, and enabling
it again once 1.9 development opens. Do not add a sleep(1) anywhere as
a workaround for the problem.


Thanks,
pq