[Mesa-dev] [PATCH 1/3] egl/android: Delete set_damage_region from egl dri vtbl

[Eric Anholt]

Harish Krupo <harish.krupo.kps at intel.com> writes:

> Eric Anholt <eric at anholt.net> writes:
>
>> Harish Krupo <harish.krupo.kps at intel.com> writes:
>>
>>> Hi Eric,
>>>
>>> Eric Anholt <eric at anholt.net> writes:
>>>
>>>> Harish Krupo <harish.krupo.kps at intel.com> writes:
>>>>
>>>>> The intension of the KHR_partial_update was not to send the damage back
>>>>> to the platform but to send the damage to the driver to ensure that the
>>>>> following rendering could be restricted to those regions.
>>>>> This patch removes the set_damage_region from the egl_dri vtbl and all
>>>>> the platfrom_*.c files.
>>>>> Then upcomming patches add a new dri2 interface for the drivers to
>>>>> implement
>>>>>
>>>>> Signed-off-by: Harish Krupo <harish.krupo.kps at intel.com>
>>>>
>>>> Why shouldn't the platform know about the damage region in a swap, if
>>>> it's available?  It looks like it was successfully used for Android, and
>>>> we should be using it for Present as well.
>>>
>>> From the spec [1], the damage region referred to by partial_update spec is
>>> the damaged part of the buffer when it is used again. The damage that the
>>> compositor/platform needs to know is the damage between the (n-1)th
>>> frame and the nth frame. Quoting from the spec:
>>> "   The surface damage for frame n is the difference between frame n and frame
>>>     (n-1), and represents the area that a compositor must recompose."
>>> This is the damage referred to by the swap_buffers_with_damage spec [2],
>>> whereas the partial_update damage region's objective is to restrict the subsequent
>>> rendering operations on the back buffer, to only those regions which have changed since
>>> that buffer was last used. This information is available as the buffer
>>> age. Some more information: [3].
>>
>> OK, let's document that in the new internal API you're adding then.
>> Things I'd want to know as an implementer of the hook:
>>
>> 1) Am I guaranteed that it's called before the frame is started?
>>
>
> No. When no damage region is set, the whole surface should be considered
> damaged. As a matter of fact, the damage region is set to full surface
> when the frame boundary is reached (i.e. swapbuffersXXX is called).

The spec citation I was looking for was:

    If any client API commands resulting in rendering to <surface> have been
    issued since eglSwapBuffers was last called with <surface>, or since the
    surface was created in case eglSwapBuffers has not yet been called on it,
    attempting to set the damage region will result in undefined framebuffer
    contents for the entire framebuffer.

So, the driver should expect the partial damage region to be set before
any rendering has happened, and doesn't need to worry about doing things
right if it shows up later.

>> 2) Is the behavior if the client draws outside of the partial update
>> damage region defined?  (is it "the driver must not change pixels
>> outside of the partial region" or "the driver might not change pixels
>> outside of the partial region")
>>
>
> If I have understood the spec correctly, then the damage regions set are
> a hint to the driver so that it can optimize the rendering by
> restricting the client's drawing commands to only the damaged region.
> In the current implementation, although the damage regions are sent back
> to the compositor instead of sending it to the driver, no issues are
> observed with the rendered output and it passes deqp tests. This
> supports the argument that the damages are only a hint.

I was looking for documentation of this part of the spec in the method's
comment:

    At all times, any client API rendering which falls outside of the damage
    region results in undefined framebuffer contents for the entire framebuffer.
    It is the client's responsibility to ensure that rendering is confined to
    the current damage area.

>> 3) Is the client guaranteed to fully initialize pixels in the partial
>> update region, or might it depend on previous contents?
>
> If the above argument is right then it means that the client would
> actually initialize the pixels of the full buffer but expect that the
> driver renders only the damaged regions.

The client can't initialize the full buffer, because of the spec quote
above.

The actual relevant spec quote for this is:

    If EGL_EXT_buffer_age is supported, the contents of the buffer inside the
    damage region may also be relied upon to contain the same content as the
    last time they were defined for the current back buffer.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180717/6b8a3157/attachment.sig>