[PATCH weston 4/7] Calculate next-frame target time in absolute space

[Daniel Stone]

Hi,

On 16 February 2017 at 10:23, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Tue, 14 Feb 2017 13:18:04 +0000
> Daniel Stone <daniels at collabora.com> wrote:
>> @@ -2389,34 +2390,35 @@ weston_output_finish_frame(struct weston_output *output,
>>                                                 presented_flags);
>>
>>       output->frame_time = timespec_to_msec(stamp);
>> -
>>       weston_compositor_read_presentation_clock(compositor, &now);
>> -     timespec_sub(&gone, &now, stamp);
>> -     msec = (refresh_nsec - timespec_to_nsec(&gone)) / 1000000; /* floor */
>> -     msec -= compositor->repaint_msec;
>>
>> -     if (msec < -1000 || msec > 1000) {
>> +     timespec_add_nsec(&next, stamp, refresh_nsec);
>> +     timespec_add_msec(&next, &next, -compositor->repaint_msec);
>> +     timespec_sub(&remain, &next, &now);
>> +     msec_rel = timespec_to_msec(&remain);
>> +
>> +     if (msec_rel < -1000 || msec_rel > 1000) {
>>               static bool warned;
>>
>>               if (!warned)
>>                       weston_log("Warning: computed repaint delay is "
>> -                                "insane: %d msec\n", msec);
>> +                                "insane: %d msec\n", msec_rel);
>>               warned = true;
>>
>> -             msec = 0;
>> +             msec_rel = 0;
>>       }
>>
>>       /* Called from restart_repaint_loop and restart happens already after
>>        * the deadline given by repaint_msec? In that case we delay until
>>        * the deadline of the next frame, to give clients a more predictable
>>        * timing of the repaint cycle to lock on. */
>> -     if (presented_flags == WP_PRESENTATION_FEEDBACK_INVALID && msec < 0)
>> -             msec += refresh_nsec / 1000000;
>> +     if (presented_flags == WP_PRESENTATION_FEEDBACK_INVALID && msec_rel < 0)
>> +             msec_rel += refresh_nsec / 1000000;
>
> This one is postponing the deadline, but should it be reflected
> also in the absolute timestamp?

Mm, maybe. But that would involve keeping the msec_rel 'is insane'
clamp in absolute space, which seemed pretty painful, at least without
new helpers which would probably be single-use forever. So I decided
to just leave it like that.

> If that does not matter, then:
> Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Thanks!
-d