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

[Pekka Paalanen]

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).

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.

> > > 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.

> 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.


Thanks,
pq