[PATCH v1 weston 06/11] tests: Add screenshot recording to weston-test

[Derek Foreman]

On 25/11/14 03:53 AM, Pekka Paalanen wrote:
> On Mon, 24 Nov 2014 15:20:35 -0800
> Bryce Harrington <bryce at osg.samsung.com> wrote:
> 
>> On Mon, Nov 24, 2014 at 04:31:01PM -0600, Derek Foreman wrote:
>>> On 24/11/14 05:01 AM, Pekka Paalanen wrote:
>>>> On Wed, 19 Nov 2014 15:06:21 -0800
>>>> Bryce Harrington <bryce at osg.samsung.com> wrote:
>>>>
>>>>> From: Derek Foreman <derekf at osg.samsung.com>
>>>>>
>>>>> Adds wl_test_record_screenshot() to weston test.  This commit also
>>>>> adds a dependency on cairo to weston-test to use it for writing PNG
>>>>> files.
>>>>>
>>>>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=83981
>>>>> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
>>>>
>>>> Hi,
>>>>
>>>> could we use a wl_shm-based wl_buffer instead of a file, please?
>>>>
>>>> That way we can simply relay the pixels as is to the test client, which
>>>> can then compare without compressing and decompressing from PNG first.
>>>
>>> Ok - it (the client) will still need to be able to write and read files
>>> though, so it can create or read the reference images.
>>
>> I agree it will run faster and save on disk space if we can skip writing
>> the images to files.  Yes, we'll still need to read the reference
>> images, but reading is faster than writing so we'll be ahead.  Most of
>> the time we don't care what the images look like anyway.
>>
>> That said, it can be beneficial to write the output out to files for
>> troubleshooting purposes.  So we still need a write mode.  But perhaps
>> it should be off by default with some env var or switch to make it show
>> its work.
> 
> Agreed on all accounts. One will be interested on the output image only
> when the test fails (hey, you could write it automatically in that
> case) or when bootstrapping the test (creating the reference image to
> begin with).

Easy enough, I think missing reference image should be a failure so
these become the same case.

> And if a missing reference image automatically causes a test to fail,
> then all one needs is to 'mv && git add' after checking the result is
> as expected.
>
> Btw. I would imagine that it would be useful to have a check function
> like:
> 	bool check_solid_color(image, rectangle, color, fuzz?)
> which would check that the rectangle sub-region of the image is of that
> solid color (with appropriate fuzz). That way we don't need reference
> images in git for everything.

What's fuzz exactly?  Each pixel can be within +-fuzz/2 on each color
component and still be a match?  a fuzz of 256 would match everything?

Or would fuzz be to allow a certain percentage of pixels to deviate (ie:
the bar with the clock and the icons on it being tested against red...)

>>>> Synchronization cannot be done with wl_display_roundtrip, so you need
>>>> an explicit event for that. The best would be to create a new protocol
>>>> object on record_screenshot request, which then delivers the event and
>>>> self-destructs, like wl_callback. You could just use wl_callback, but
>>>> in principle we should avoid new use cases for wl_callback due to
>>>> the design issues.
>>>>
>>>> Please also document carefully in which orientation the screenshot is
>>>> taken and how the clip coords are interpreted, in case some
>>>> output_scale/transform are in effect. Or if you rely on the renderer's
>>>> read_pixels() convention, I hope that is documented somewhere...
>>>
>>> Not yet ;)
>>>
>>> (fwiw it's done so an unrotated display's screenshot looks normal in an
>>> image viewer - so the opposite of gl convention :)
>>>
>>>> Btw. there is a small danger in linking Cairo to the compositor.
>>>> GL-renderer uses GLESv2, but if CAIRO_LIBS contains cairo-gl.so linking
>>>> to libGL, it might explode... or maybe not because of no RTLD_GLOBAL...
>>>> or maybe it could, I don't know. So I'd just avoid that. :-)
>>>
>>> Siiiigh, I was afraid someone would come up with a reason to actually
>>> deal with libpng directly.  Oh, wait, only the client has to deal with
>>> png files - I think it's safe to link cairo there? :)
>>
>> Most distros don't ship cairo with gl enabled anyway.  You'd have to do
>> that intentionally.
> 
> Yeah. Also GL vs. GLESv2 must be chosen during Cairo build time and
> they are mutually exclusive.
> 
> Relying on cairo-image is ok, cairo-gl not so much.

I'll rewrite this patch so it makes no use of cairo in weston-test, or
in any server code.

>> Even with it enabled, if we keep png generation client-side then we're
>> not going to risk interfering with the compositor.  Although, maybe some
>> tests link in some compositor code and could hit the issue... not sure.
>> At least here the scope of the problem will be limited to the one test.
>> And there's probably a few paths to solve it.
> 
> Linking to Cairo in the test clients is fine. By definition, that is
> client code.
> 
> We have some tests that include server code IIRC, but those are not
> test clients, and do not run with an actual server, so you wouldn't be
> screenshooting there anyway.

As eager as I am to not use libpng directly it's not that big deal, and
I have done it before.  I think I can see wanting to have a test client
that uses GL for its own rendering, and if using cairo for screenshots
makes that unworkable I'll look silly later for being lazy now.

We're probably better off just ditching cairo entirely here (like Bill
suggested in the first place.  yeah, hi, sorry.  ;)

Unfortunately, I think that necessitates changes to some of the later
patches too that use cairo for masking regions.