On 05.06.2018 11:51, Juan A. Suarez Romero wrote:
On Mon, 2018-06-04 at 13:22 +0100, Daniel Stone wrote:
Hi Juan,

On 4 June 2018 at 12:43, Juan A. Suarez Romero <jasua...@igalia.com> wrote:
On Fri, 2018-06-01 at 16:32 +0100, Daniel Stone wrote:
I think you're right, and this needs more rework to be consistent.

wl_egl_window_get_attached_size() always returns the size of the last
attached buffer (if there have been any); that is good and consistent.

Let's focus now on this, as I already sent a V2 patch to properly initialize
surface Width/Height values.

Right, but in doing so it introduces an inconsistency. As below:
wl_egl_create_window(w1, h1);
eglCreateSurface();
eglQuerySurface(EGL_{WIDTH,HEIGHT});
wl_egl_resize_window(w2, h2);
eglQuerySurface(EGL_{WIDTH,HEIGHT});
glClear();
eglSwapBuffers();
wl_egl_resize_window(w3, h3);
eglQuerySurface(EGL_{WIDTH,HEIGHT});
glClear();
eglQuerySurface(EGL_{WIDTH,HEIGHT});
eglSwapBuffers();

The first query will correctly return (w1,h1). The second query will
incorrectly also return (w1,h1), even though the surface will never
have that size. The third query will return (w2,h2) as the
last-attached size, even though the surface will be (w3,h3) on next
draw. The fourth query will correctly return (w3,h3).

I would like this to be consistent: my suggestion is that a query for
the surface size always returns the size that the surface will become
on the next eglSwapBuffers.



I've been re-reading again EGL 1.5 spec, and found this interesting parts that
can bring some clarification to this mess:

- Section 3.10.1.1 ("Native Window Resizing"): if the native window
corresponding to _surface_ has been resized prior to the swap, _surface_ must be
resized to match. _surface_ will normally be resized by the EGL implementation
at the time the native window is resized. If the implementation cannot do this
transparently to the client, then *eglSwapBuffers* must detect the change and
resize surface prior to copying its pixels to the native window.

- Section 3.5.6 ("Surface Attributes"): querying `EGL_WIDTH` and `EGL_HEIGHT`
returns respectively the width and height, in pixels, of the surface. For a
window or pixmap surface, these values are initially equal to the width and
height of the native window or pixmap with respect to which surface was created.
If a native window is resized, the corresponding window surface will eventually
be resized by the implementation to match (as discussed in section 3.10.1). If
there is a discrepancy because EGL has not yet resized the window surface, the
size returned by *eglQuerySurface* will always be that of the EGL surface, not
the corresponding native window.


 From the previous parts, I extract the following conclusions:

- Surface size value is indeed initialized with the native window size, as we
are doing in our patch.

- In Wayland/Mesa, the surface is not resized at the time the native window is
resized; this is done in *eglSwapBuffers*, following 3.10.1.1.

- If our EGL implementation has not resized the window surface, then
*eglQuerySurface* should return the current size of the surface, not the future
size, as the future size is the native window size. This is the main difference:
*eqlQuerySurface* does not return the size the surface will become on next
eglSwapBuffers, but the current size.

This sounds correct to me and is exactly what x11 implementation does, during query hook we ask X server the current size of the surface to make sure we are in sync with what window system. Mismatch is what the resize tests are trying to catch.



If the above conclusions are correct, then the calls to eglQuerySurface() in the
  example code should return:

- (w1,h1): this is the initial surface window size.
- (w1,h1): even when the window was resized to (w2,h2), in Mesa this does not
take effect until calling eglSwapBuffers(). So current size is still (w1,h1)
- (w2,h2): like the previous call, the native window was resized to (w3,h3), but
  eglSwapBuffers() was not called yet.
- (w2,h2) or (w3,h3)?: I have doubts here. On one side, we should return
(w2,h2), as eglSwapBuffers() was not called yet. On the other hand, not sure if
glClear() should update the size. Maybe this is something dependent on the
implementation.

If NVidia is resizing the surface at the time the native window is resized,
without waiting for eglSwapBuffers, then the returned values should be (w1,h1),
(w2,h2), (w3,h3) and (w3,h3), respectively.


In dEQP-EGL.functional.resize.surface_size.* , this is what the test does
initially:

wl_egl_window_create(w1, h1);
eglCreateWindowSurface();
egl.makeCurrent()
wl_egl_window_get_attached_size();

The failing part is the results for wl_egl_window_get_attached_size(): test
assumes it will return (w1, h1) (and this is what Nvidia implementation seems to
return), but Mesa returns (0, 0).

If I understand correctly, Mesa returns (0, 0) because there is no buffer
attached. If we call before eglSwapBuffers() (which internally calls
wl_surface_attach()) and afterwards wl_egl_window_get_attached_size(), then Mesa
would return (w1, h1) because now the buffer is attached to the window.

Am I correct?

Yep, that is correct.

Because it seems like NVidia implementation is returning (w1,h1) because at that
moment it has an attached buffer. Wondering if eglMakeCurrent() is performing
such attachment.

Yes, that's correct, and I believe eglMakeCurrent() does perform that
attachment. The NVIDIA/EGLStreams implementation is extremely broken
in that regard, in a way which does break genuine real-world
applications. We had a long discussion on wayland-devel@ when the
EGLStreams implementation was first posted, detailing the ways in
which their implementation causes genuine problems for real-world
users, and can provoke client deadlocks. I am very opposed to
supporting or codifying that behaviour.


So I understand eglSwapBuffers() is required to do the attachment.

In the example above, if we replace eglQuerySurface() by
wl_egl_window_get_attached_size(), what would be the expected values?


BR,

        J.A.

Cheers,
Daniel

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to