Eric Anholt <e...@anholt.net> writes: > Harish Krupo <harish.krupo....@intel.com> writes: > >> Eric Anholt <e...@anholt.net> writes: >> >>> Harish Krupo <harish.krupo....@intel.com> writes: >>> >>>> Hi Eric, >>>> >>>> Eric Anholt <e...@anholt.net> writes: >>>> >>>>> Harish Krupo <harish.krupo....@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....@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.
Thank you, understood it. I should have read the spec better :(. Also, generalizing Android/deqp's usage seems to be wrong. Android's deqp passed previously even when the driver wasn't restricting the rendering to only the damaged regions. Should I update these in the comments section of the extension? > It is the client's responsibility to ensure that rendering is confined to > the current damage area. quick question: How can the client ensure this? Thank you Regards Harish Krupo _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
