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

[Bryce Harrington]

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

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.

Bryce