[PATCH v1 weston 07/11] tests: Add a fadein test

[Derek Foreman]

On 25/11/14 04:11 AM, Pekka Paalanen wrote:
> On Mon, 24 Nov 2014 18:48:51 -0800
> Bryce Harrington <bryce at osg.samsung.com> wrote:
> 
>> On Mon, Nov 24, 2014 at 01:19:46PM +0200, Pekka Paalanen wrote:
>>> On Wed, 19 Nov 2014 15:06:22 -0800
>>> Bryce Harrington <bryce at osg.samsung.com> wrote:
>>>
>>>> This also serves as a proof of concept of the screen capture
>>>> functionality and as a demo for snapshot-based rendering verification.
>>>>
>>>> Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
>>>> ---
>>>>  Makefile.am         |  7 +++++-
>>>>  tests/fadein-test.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 70 insertions(+), 1 deletion(-)
> 
>>>> +TEST(fadein)
>>>> +{
>>>> +	struct client *client;
>>>> +	char basename[32];
>>>> +	char *out_path;
>>>> +	int i;
>>>> +
>>>> +	client = client_create(100, 100, 100, 100);
>>>> +	assert(client);
>>>> +
>>>> +	for (i = 0; i < 6; i++) {
>>>> +		snprintf(basename, sizeof basename, "fadein-%02d", i);
>>>> +		out_path = output_filename(basename);
>>>> +
>>>> +		wl_test_record_screenshot(client->test->wl_test, out_path);
>>>> +		client_roundtrip(client);
>>>> +		free (out_path);
>>>> +
>>>> +		usleep(250000);
>>>> +	}
>>>> +
>>>> +}
>>>
>>> Hi,
>>>
>>> where is the verification promised in the commit message? ;-)
>>>
>>> I think it is bad to rely on delays/timers. We need something
>>> more deterministic: a protocol to drive the (headless) repaint.
>>>
>>> We probably need the headless repaint to not run on its own, but
>>> strictly when requested by the test client. Additionally, we probably
>>> want to carry a "page flip completion" timestamp in that request, and
>>> have the compositor use the timestamp. That way the test client can
>>> actually drive the in-compositor animations, because it is in control
>>> of the clock and framerate.
>>
>> Yes, that would definitely be helpful.
>>
>> Can you elaborate on how this should be implemented?
> 
> I don't have a clear picture in my mind yet.
> 
> Client driven repaint would probably need to be a new global in the
> protocol, another test extension. Exposing it would be conditional to
> both the test module, and the current backend supporting it.
> 
> It is difficult to call module functions from another module, so you
> likely need some function pointers in struct weston_compositor. One
> module sets them, and if the other module finds them non-NULL, it knows
> the other one supports client driven repaint.
> 
> But, because for starters it would be supported only by the headless
> backend, which is not used for anything but testing, it's likely fine
> to make the first implementation in the headless backend and always
> expose it directly from there. No need to hook it up in the test module
> for now.
> 
> Once we see how it works for headless, we can plan how to let other
> backends support it, if needed.

I've been thinking about this a bit lately, here's some random musing.  :)

We could wrap any clock_gettime usage within weston (if we only care
about the headless compositor that's one call) - if we're using a test
clock it would return the time as controlled by the client, otherwise
it'd just call clock_gettime.  I need to look more into how the timerfd_
stuff works, but I think the fds will need to be manually messed with on
clock change.

The client would advance the clock by a specified number of
(nano?)seconds (monotonically, always increasing) via protocol.

headless renderer would need changes to fake a refresh rate - I don't
think we'd want every clock advance to trigger a repaint.  I think we're
better off with client driving the clock, and repaint being indirectly
controlled.  Right now it just waits 16ms after a repaint then repaints
again.

In any event we do need to control the clock (ie: not just gate repaint)
to get deterministic snapshots of animations/presentation - if that's
really our end goal.

Catching the server fade-in would be problematic - the clock frobbing
would have to be turned on at some point, I'm not sure we want it to
default to on even for the headless compositor (I guess this could be a
command line parameter).


I've wondered a bit about sharing this fake clock with clients -
wl_clock_gettime() or such, the first call would query the presentation
clock id, on every subsequent call it'd just call clock_gettime() on
that id if it's a normal clock.  If it's the test clock it would query
the time from the server.

Is this too invasive?  There's no weston client library, so I'm not
really sure exactly where this would have to be implemented.  I think
jamming this into libwayland-client might be frowned upon.

Similar effects could be achieved with LD_PRELOAD and replacing
clock_gettime when running a test, but that's pretty nasty.

If we're going to do anything for normal clients I guess it should be
sorted before the presentation extension is finalized, since it relies
on clients calling clock_gettime() directly.

(as a complete tangent, I'm a little confused that the presentation
protocol is part of weston - does this mean that video players will
target specific compositors and not generically "wayland"?)

>>> That should make the fade-in test reliable, and it will help with
>>> Presentation testing too.
>>>
>>> The client driven repaint likely needs to be enabled per-test. If a
>>> test attempts to enable it, but the compositor still does not advertise
>>> the support(!) in protocol, the test should skip. This way real
>>> hardware backends can skip these tests automatically, as they cannot
>>> (or not implemented yet) support client driven repaint.
>>>
>>> How would that sound?
>>
>> Sure, although I suspect all tests that care about the rendered output
>> will need to set this.  Unless they care only about the initial frame.
> 
> To the contrary, I would expect most tests to only care about the final
> stabilised image when all animations have ended (preferably never even
> started).
>
> I think that should be perfectly implementable without client driven
> repaint. Client driven repaint is only needed for timing sensitive
> tests, like for animations and testing Presentation extension.

Right - if we're not specifically testing animations we should probably
just disable them.

If we are testing animations it's possible we can write test apps that
just display the frame we're interested in for a really long time
anyway, which may be less direct but with care could produce valid results.

Some aspects of presentation can be tested by playing the same video
frame for a very long time as well...

Is client driven repaint too much work for too little gain?  I'm
inclined to think it's worthwhile, but don't want to go too far towards
implementation if the general concensus is no. :)

It gets a bit easier if we don't care at all about messing with the
clocks in non-test clients.

> You post damage to a surface and wait for the
> presentation_feedback.presented event. Then do the screenshot. Or
> actually you shouldn't even need to wait, because completing a
> screenshot likely requires one more repaint cycle, so shooting itself
> already guarantees a repaint.

Hmm, I'm not sure why the screenshot would require an extra repaint cycle?