Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Mon, 2018-08-06 at 16:02 +0100, Daniel Stone wrote:
> Hi Juan,
>
> On Tue, 5 Jun 2018 at 18:23, Juan A. Suarez Romero
> wrote:
> > On Tue, 2018-06-05 at 12:41 +0100, Daniel Stone wrote:
> > > > - 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.
> > >
> > > Disagree, given that we are in charge of exactly when the native
> > > window is 'resized'. It's unclear to me what 'resize the surface prior
> > > to copying its pixels to the native window' even means: does this
> > > require we do a scale blit, or a crop blit, or?
> > >
> > > The semantics Mesa has interpreted until recently is that
> > > wl_egl_resize_window() constitutes a resize _request_, and that the
> > > last submitted request is acted upon and latched at the first draw
> > > call. This then constitutes the native surface size up until the next
> > > first draw call. I think these are good semantics, and we have to be
> > > quite picky about what exactly we implement, since clients rely on
> > > them. Not just cosmetically (atomic resizing of subsurface chains),
> > > but also because if you get your window sizing wrong on the desktop,
> > > you'll catch a fatal error from the compositor (X11 just shrugs and
> > > does nothing).
> > >
> > > I think these are good semantics (as to exactly when resize takes
> > > effect), but for the moment we only update the EGL_{WIDTH,HEIGHT}
> > > surface query results after the first draw call. I would suggest that
> > > in the period after eglSwapBuffers but before the first draw call, we
> > > also update those values, and that we update them after eglSwapBuffers
> > > has executed as well. This makes the semantics that any native window
> > > resize requests take effect immediately after eglSwapBuffers (or just
> > > immediately, if no swap has ever been executed) and before the first
> > > draw call (or buffer_age query ... cf. 9ca6711faa03) for a given
> > > frame; in between the first draw call and eglSwapBuffers, the native
> > > resize requests will be queued until after eglSwapBuffers.
> >
> >
> > I see. The surface size should be updated inmediately, after a window
> > resize is
> > done, as you said.
> >
> > I'll send then a new patch version. Actually I'll split this patch in two:
> > one
> > to correctly initialize the surface size (there is no doubt on this, and
> > this
> > patch alone will fix the test), and another patch to correctly update the
> > surface size when the window is resized.
>
> I think the v3 2/2 patch doesn't actually conform to this behaviour,
> as it unconditionally sets the surface width/height from
> wl_egl_window_resize() rather than waiting until the resized buffers
> are actually going to take effect.
>
Yes, the idea is that any window resize take effect immediately, without
requiring to run eglSwapBuffers() (that is, if window is resized and the surface
size is immediately queried, the new size is returned; only when the buffers are
updated a new buffer with is created).
J.A.
> Cheers,
> Daniel
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
Hi Juan,
On Tue, 5 Jun 2018 at 18:23, Juan A. Suarez Romero wrote:
> On Tue, 2018-06-05 at 12:41 +0100, Daniel Stone wrote:
> > > - 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.
> >
> > Disagree, given that we are in charge of exactly when the native
> > window is 'resized'. It's unclear to me what 'resize the surface prior
> > to copying its pixels to the native window' even means: does this
> > require we do a scale blit, or a crop blit, or?
> >
> > The semantics Mesa has interpreted until recently is that
> > wl_egl_resize_window() constitutes a resize _request_, and that the
> > last submitted request is acted upon and latched at the first draw
> > call. This then constitutes the native surface size up until the next
> > first draw call. I think these are good semantics, and we have to be
> > quite picky about what exactly we implement, since clients rely on
> > them. Not just cosmetically (atomic resizing of subsurface chains),
> > but also because if you get your window sizing wrong on the desktop,
> > you'll catch a fatal error from the compositor (X11 just shrugs and
> > does nothing).
> >
> > I think these are good semantics (as to exactly when resize takes
> > effect), but for the moment we only update the EGL_{WIDTH,HEIGHT}
> > surface query results after the first draw call. I would suggest that
> > in the period after eglSwapBuffers but before the first draw call, we
> > also update those values, and that we update them after eglSwapBuffers
> > has executed as well. This makes the semantics that any native window
> > resize requests take effect immediately after eglSwapBuffers (or just
> > immediately, if no swap has ever been executed) and before the first
> > draw call (or buffer_age query ... cf. 9ca6711faa03) for a given
> > frame; in between the first draw call and eglSwapBuffers, the native
> > resize requests will be queued until after eglSwapBuffers.
>
>
> I see. The surface size should be updated inmediately, after a window resize
> is
> done, as you said.
>
> I'll send then a new patch version. Actually I'll split this patch in two: one
> to correctly initialize the surface size (there is no doubt on this, and this
> patch alone will fix the test), and another patch to correctly update the
> surface size when the window is resized.
I think the v3 2/2 patch doesn't actually conform to this behaviour,
as it unconditionally sets the surface width/height from
wl_egl_window_resize() rather than waiting until the resized buffers
are actually going to take effect.
Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Tue, 05 Jun 2018 19:22:47 +0200 "Juan A. Suarez Romero" wrote: > On Tue, 2018-06-05 at 12:41 +0100, Daniel Stone wrote: > > Hi Juan, > > > > On 5 June 2018 at 09:51, Juan A. Suarez Romero wrote: > > > > > On Mon, 2018-06-04 at 13:22 +0100, Daniel Stone wrote: > > > > 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. > > > > It is required to do an attachment, yeah. > > > > Nice. The other failing test in CTS is not calling eglSwapBuffers() > before doing the first wl_gl_window_get_attached_size(), and hence > when we call it we get null values. > > So it seems the fix must be done in CTS, bycalling eglSwapBuffers() > to ensure the window has an attached buffer, before getting the size. Hi, I would like to try to clarify the mental model here. The term "attached" is somewhat of a misnomer. But this may also be confusing, because I am bringing in underlying details of Wayland which no other window system has AFAIK. It is true the Wayland request is called wl_surface_attach(wl_buffer). Alone it does essentially nothing, it requires a wl_surface_commit() to become effective. eglSwapBuffers() always does both before returning, so that is fine. Intuitively "attach" implies that one creates an association between two objects and then one can continue working on the attached object (buffer). This is where the misnomer becomes apparent: it would be better to talk about submitting, not attaching. eglSwapBuffers is submitting the current WIP buffer to the compositor to replace the contents of the Wayland wl_surface (window). One cannot resize or draw into the submitted buffer as long as it is held reserved by the Wayland compositor, to be released with wl_buffer.release event. A Wayland window (wl_surface) has no initial size. Only submitting (attach+commit) a wl_buffer will give the wl_surface a size, that is, calling eglSwapBuffers resizes the window. Only the client itself can resize the window e.g. by calling eglSwapBuffers, the compositor or any third party cannot resize the window, so there is no way to query the window size via Wayland. From an EGL application point of view, a window has two different concepts of size: the size that is currently being used (with e.g. input events) which is the size of the wl_surface and returned by wl_egl_window_get_attached_size(); and the size of your current unfinished drawing which will become the wl_surface size on the next call to eglSwapBuffers. wl_egl_window_get_attached_size() would more appropriately be described as returning the size of the last submitted buffer. When I talked about wl_surface size in the above, I was actually using a generally incorrect simplification. The wl_buffer size determines the wl_surface size (now I am using the specific Wayland terms here), but they may not be equal. Buffer scale and transform and wl_viewport are Wayland protocol features that change how the wl_surface size is computed from the wl_buffer size. However, all these are outside of EGL, in the EGL user instead, and EGL itself will be unaware of these. Therefore EGL Wayland can only concern itself with buffer dimensions, the actual wl_surface size will remain unknown to it. Therefore, when interpreting the EGL specifications, when it talks about "window size", on Wayland you can really only think about the WIP or submitted buffer size. I would say that for EGL purposes, the current native window size is the last submitted buffer size and it is zero initially. For the CTS, one may want to test wl_egl_window_get_attached_size and the WIDTH/HEIGHT queries separately to ensure both are as expected (I agree to Daniel's interpretation here). Thanks, pq pgpm3_heJi0F4.pgp Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Tue, 2018-06-05 at 19:36 +0100, Emil Velikov wrote: > Hi everyone, > > Throwing a small suggestion here: > > Once a consensus is reached, do add some beefy comments inline. Mostly > of any parts that are different from the X11/GBM/other backends. > A reference to this thread might also be a good idea. > Sure, I'll add proper comments. Thanks for the suggestion. J.A. > Thanks > Emil > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
Hi everyone, Throwing a small suggestion here: Once a consensus is reached, do add some beefy comments inline. Mostly of any parts that are different from the X11/GBM/other backends. A reference to this thread might also be a good idea. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Tue, 2018-06-05 at 12:41 +0100, Daniel Stone wrote:
> Hi Juan,
>
> On 5 June 2018 at 09:51, Juan A. Suarez Romero wrote:
> > On Mon, 2018-06-04 at 13:22 +0100, Daniel Stone wrote:
> > > 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:
>
> 'Mess' is right. :) Probably because the spec is clearly written from
> platforms which actually have a native window size outside of client
> control. e.g. on X11, the window manager can resize a window and from
> the client's point of view this is something which just happens to it:
> neither the application code nor EGL has any say at all on it. Wayland
> doesn't have this constraint, and puts the client (in this case, the
> EGL implementation) in full control of the size at all times.
>
:)
> > - 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.
>
> Agree.
>
Nice.
> > - 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.
>
> Disagree, given that we are in charge of exactly when the native
> window is 'resized'. It's unclear to me what 'resize the surface prior
> to copying its pixels to the native window' even means: does this
> require we do a scale blit, or a crop blit, or?
>
> The semantics Mesa has interpreted until recently is that
> wl_egl_resize_window() constitutes a resize _request_, and that the
> last submitted request is acted upon and latched at the first draw
> call. This then constitutes the native surface size up until the next
> first draw call. I think these are good semantics, and we have to be
> quite picky about what exactly we implement, since clients rely on
> them. Not just cosmetically (atomic resizing of subsurface chains),
> but also because if you get your window sizing wrong on the desktop,
> you'll catch a fatal error from the compositor (X11 just shrugs and
> does nothing).
>
> I think these are good semantics (as to exactly when resize takes
> effect), but for the moment we only update the EGL_{WIDTH,HEIGHT}
> surface query results after the first draw call. I would suggest that
> in the period after eglSwapBuffers but before the first draw call, we
> also update those values, and that we update them after eglSwapBuffers
> has executed as well. This makes the semantics that any native window
> resize requests take effect immediately after eglSwapBuffers (or just
> immediately, if no swap has ever been executed) and before the first
> draw call (or buffer_age query ... cf. 9ca6711faa03) for a given
> frame; in between the first draw call and eglSwapBuffers, the native
> resize requests will be queued until after eglSwapBuffers.
>
I see. The surface size should be updated inmediately, after a window resize is
done, as you said.
I'll send then a new patch version. Actually I'll split this patch in two: one
to correctly initialize the surface size (there is no doubt on this, and this
patch alone will fix the test), and another patch to correctly update the
surface size
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
Hi Juan,
On 5 June 2018 at 09:51, Juan A. Suarez Romero wrote:
> On Mon, 2018-06-04 at 13:22 +0100, Daniel Stone wrote:
>> 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:
'Mess' is right. :) Probably because the spec is clearly written from
platforms which actually have a native window size outside of client
control. e.g. on X11, the window manager can resize a window and from
the client's point of view this is something which just happens to it:
neither the application code nor EGL has any say at all on it. Wayland
doesn't have this constraint, and puts the client (in this case, the
EGL implementation) in full control of the size at all times.
> - 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.
Agree.
> - 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.
Disagree, given that we are in charge of exactly when the native
window is 'resized'. It's unclear to me what 'resize the surface prior
to copying its pixels to the native window' even means: does this
require we do a scale blit, or a crop blit, or?
The semantics Mesa has interpreted until recently is that
wl_egl_resize_window() constitutes a resize _request_, and that the
last submitted request is acted upon and latched at the first draw
call. This then constitutes the native surface size up until the next
first draw call. I think these are good semantics, and we have to be
quite picky about what exactly we implement, since clients rely on
them. Not just cosmetically (atomic resizing of subsurface chains),
but also because if you get your window sizing wrong on the desktop,
you'll catch a fatal error from the compositor (X11 just shrugs and
does nothing).
I think these are good semantics (as to exactly when resize takes
effect), but for the moment we only update the EGL_{WIDTH,HEIGHT}
surface query results after the first draw call. I would suggest that
in the period after eglSwapBuffers but before the first draw call, we
also update those values, and that we update them after eglSwapBuffers
has executed as well. This makes the semantics that any native window
resize requests take effect immediately after eglSwapBuffers (or just
immediately, if no swap has ever been executed) and before the first
draw call (or buffer_age query ... cf. 9ca6711faa03) for a given
frame; in between the first draw call and eglSwapBuffers, the native
resize requests will be queued until after eglSwapBuffers.
> - 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.
3.5.6 ('EGL has not yet resized the window surface') and 3.10.1.1
('will normally be resized by the EGL implementation at the time the
native window is resized') seem to contradict each other here. But the
reason for 3.5.6 wording is almost certainly coming from X11, where
the native
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
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 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
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
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 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.
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.
> >
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
Hi Juan,
On 4 June 2018 at 12:43, Juan A. Suarez Romero 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.
> 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.
Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Fri, 2018-06-01 at 16:32 +0100, Daniel Stone wrote: > Hi, > > On 1 June 2018 at 15:35, Brendan King wrote: > > IMG has a similar patch to the one you describe, but without the changes to > > the resize_callback, so just this bit: > > > > @@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv, > > _EGLDisplay *disp, > >goto cleanup_surf; > > } > > > > + dri2_surf->base.Width = window->width; > > + dri2_surf->base.Height = window->height; > > + > > > > I don't think the resize_callback changes are necessary. > > 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. 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? 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. J.A. > It seems like the desired semantic for handling EGL surface-size > queries, is that the query returns the size of the buffer that will be > attached at the next eglSwapBuffers. For example, if you issue > wl_egl_window_resize() in between eglSwapBuffers() and the first draw > call, the surface-size query should reflect this new size immediately > after the function returns. However, if you issue > wl_egl_window_resize() after a frame's first draw call has been made, > querying the surface size should return the old size until > eglSwapBuffers() has been called, since the resize call has no effect > on the buffer which will actually be attached when calling SwapBuffers > in that frame. > > Cheers, > Daniel > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
When creating a windows surface with eglCreateWindowSurface(), the
width and height returned by eglQuerySurface(EGL_{WIDTH,HEIGHT}) is
invalid until buffers are updated (like calling glClear()).
But according to EGL 1.5 spec, 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 the surface was
created"
This fixes dEQP-EGL.functional.color_clears.* CTS tests
v2:
- Do not modify attached_{width,height} (Daniel)
- Do not update size on resizing window (Brendan)
CC: Daniel Stone
CC: Brendan King
---
src/egl/drivers/dri2/platform_wayland.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 63da21cdf55..f62cbbc5c02 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -255,6 +255,9 @@ dri2_wl_create_window_surface(_EGLDriver *drv, _EGLDisplay
*disp,
goto cleanup_surf;
}
+ dri2_surf->base.Width = window->width;
+ dri2_surf->base.Height = window->height;
+
visual_idx = dri2_wl_visual_idx_from_config(dri2_dpy, config);
assert(visual_idx != -1);
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
Hi, On 1 June 2018 at 15:35, Brendan King wrote: > IMG has a similar patch to the one you describe, but without the changes to > the resize_callback, so just this bit: > > @@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv, > _EGLDisplay *disp, >goto cleanup_surf; > } > > + dri2_surf->base.Width = window->width; > + dri2_surf->base.Height = window->height; > + > > I don't think the resize_callback changes are necessary. 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. It seems like the desired semantic for handling EGL surface-size queries, is that the query returns the size of the buffer that will be attached at the next eglSwapBuffers. For example, if you issue wl_egl_window_resize() in between eglSwapBuffers() and the first draw call, the surface-size query should reflect this new size immediately after the function returns. However, if you issue wl_egl_window_resize() after a frame's first draw call has been made, querying the surface size should return the old size until eglSwapBuffers() has been called, since the resize call has no effect on the buffer which will actually be attached when calling SwapBuffers in that frame. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On 01/06/18 12:09, Juan A. Suarez Romero wrote:
On Fri, 2018-06-01 at 11:23 +0100, Daniel Stone wrote:
Hi,
On 1 June 2018 at 09:47, Juan A. Suarez Romero wrote:
My question then: is mandatory to call eglSwapBuffers() to ensure
wl_egl_window_get_attached_size() returns the right window size? Because I
found nothing related about this. If it is mandatory, then calling
eglSwapBuffers() in the test should fix the problem (including the patch
I've summited with Daniel's changes).
In Wayland, surfaces do _not_ have a native size, unlike say GBM
surfaces or X11 surfaces. Buffers obviously have a size, and buffers
are 'attached' to a surface inside eglSwapBuffers by calling
wl_surface_attach(). wl_egl_window_get_attached_size() returns the
size of the last buffer which was actually attached, which is required
for things like correct input handling when resizing.
I see. So initialy the wayland window does not have a buffer attached, and hence
why calling wl_egl_window_get_attached_size() returns an invalid value.
Only when wl_surface_attach() is called (that happens when invoking
eglSwapBuffers()) wl_egl_window_get_attached_size() returns the proper value.
In this case, I agree the error is in the dEQP test.
I'm going to get rid of attached_{width,height} in the original patch and push
it. And send a patch to CTS to fix the dEQP test.
Thanks!
J.A.
IMG has a similar patch to the one you describe, but without the changes
to the resize_callback, so just this bit:
@@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
_EGLDisplay *disp,
goto cleanup_surf;
}
+ dri2_surf->base.Width = window->width;
+ dri2_surf->base.Height = window->height;
+
I don't think the resize_callback changes are necessary.
Brendan.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On 01/06/18 12:09, Juan A. Suarez Romero wrote:
On Fri, 2018-06-01 at 11:23 +0100, Daniel Stone wrote:
Hi,
On 1 June 2018 at 09:47, Juan A. Suarez Romero wrote:
My question then: is mandatory to call eglSwapBuffers() to ensure
wl_egl_window_get_attached_size() returns the right window size? Because I
found nothing related about this. If it is mandatory, then calling
eglSwapBuffers() in the test should fix the problem (including the patch
I've summited with Daniel's changes).
In Wayland, surfaces do _not_ have a native size, unlike say GBM
surfaces or X11 surfaces. Buffers obviously have a size, and buffers
are 'attached' to a surface inside eglSwapBuffers by calling
wl_surface_attach(). wl_egl_window_get_attached_size() returns the
size of the last buffer which was actually attached, which is required
for things like correct input handling when resizing.
I see. So initialy the wayland window does not have a buffer attached, and hence
why calling wl_egl_window_get_attached_size() returns an invalid value.
Only when wl_surface_attach() is called (that happens when invoking
eglSwapBuffers()) wl_egl_window_get_attached_size() returns the proper value.
In this case, I agree the error is in the dEQP test.
I'm going to get rid of attached_{width,height} in the original patch and push
it. And send a patch to CTS to fix the dEQP test.
Thanks!
J.A.
IMG has a similar patch to the modified one you describe, but without
the changes to resize_callback, so just this bit:
@@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
_EGLDisplay *disp,
goto cleanup_surf;
}
+ dri2_surf->base.Width = window->width;
+ dri2_surf->base.Height = window->height;
+
Brendan.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Fri, 2018-06-01 at 11:23 +0100, Daniel Stone wrote:
> Hi,
>
> On 1 June 2018 at 09:47, Juan A. Suarez Romero wrote:
> > My question then: is mandatory to call eglSwapBuffers() to ensure
> > wl_egl_window_get_attached_size() returns the right window size? Because I
> > found nothing related about this. If it is mandatory, then calling
> > eglSwapBuffers() in the test should fix the problem (including the patch
> > I've summited with Daniel's changes).
>
> In Wayland, surfaces do _not_ have a native size, unlike say GBM
> surfaces or X11 surfaces. Buffers obviously have a size, and buffers
> are 'attached' to a surface inside eglSwapBuffers by calling
> wl_surface_attach(). wl_egl_window_get_attached_size() returns the
> size of the last buffer which was actually attached, which is required
> for things like correct input handling when resizing.
>
I see. So initialy the wayland window does not have a buffer attached, and hence
why calling wl_egl_window_get_attached_size() returns an invalid value.
Only when wl_surface_attach() is called (that happens when invoking
eglSwapBuffers()) wl_egl_window_get_attached_size() returns the proper value.
In this case, I agree the error is in the dEQP test.
I'm going to get rid of attached_{width,height} in the original patch and push
it. And send a patch to CTS to fix the dEQP test.
Thanks!
J.A.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
Hi, On 1 June 2018 at 09:47, Juan A. Suarez Romero wrote: > My question then: is mandatory to call eglSwapBuffers() to ensure > wl_egl_window_get_attached_size() returns the right window size? Because I > found nothing related about this. If it is mandatory, then calling > eglSwapBuffers() in the test should fix the problem (including the patch > I've summited with Daniel's changes). In Wayland, surfaces do _not_ have a native size, unlike say GBM surfaces or X11 surfaces. Buffers obviously have a size, and buffers are 'attached' to a surface inside eglSwapBuffers by calling wl_surface_attach(). wl_egl_window_get_attached_size() returns the size of the last buffer which was actually attached, which is required for things like correct input handling when resizing. I'd prefer to fix dEQP to not ask nonsensical questions than try to fix the answer. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
The IMG patch Eric was referring to was to dEQP. I submitted bug reports
to Google and Khronos:
https://issuetracker.google.com/issues/64059452
https://gitlab.khronos.org/Tracker/vk-gl-cts/issues/594
I've attached the patch to this email. The problem is that the dEQP
*resize.surface_size* tests check the native window dimensions after
creating the window, without having called eglSwapBuffers, and hence get
back a width and height of zero, resulting in the tests failing.
Here is the patch description:
The Wayland version of deqp-egl calls wl_egl_window_get_attached_size
to get window dimensions. On Mesa at least, this initially returns a
width and height of zero for a new window. The surface resize tests
check the initial surface size, and fail as a result.
Add a new native window capability, CAPABILITY_INITIAL_SURFACE_SIZE_QUERY,
which is only set if the initial window size can be queried after
creation. If native windows don't have the capability, return the
required size from getNativeSurfaceSize, rather than the size obtained
from the window system.
A parameter has been added to getNativeSurfaceSize, to indicate if
the initial size of a surface is being queried.
Brendan.
On 01/06/18 09:12, Juan A. Suarez Romero wrote:
On Thu, 2018-05-31 at 16:57 +0100, Daniel Stone wrote:
Hi Juan,
Thanks for picking this up!
On 31 May 2018 at 16:44, Juan A. Suarez Romero wrote:
@@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv, _EGLDisplay
*disp,
goto cleanup_surf;
}
+ dri2_surf->base.Width = window->width;
+ dri2_surf->base.Height = window->height;
+
+ window->attached_width = dri2_surf->base.Width;
+ window->attached_height = dri2_surf->base.Height;
We should definitely not initialise attached_{width,height} here,
because no buffer has ever been attached.
@@ -574,8 +582,8 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
struct dri2_egl_display *dri2_dpy =
dri2_egl_display(dri2_surf->base.Resource.Display);
- if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
- dri2_surf->base.Height != dri2_surf->wl_win->height) {
+ if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
+ dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
dri2_wl_release_buffers(dri2_surf);
@@ -1629,8 +1637,8 @@ swrast_update_buffers(struct dri2_egl_surface *dri2_surf)
if (dri2_surf->back)
return 0;
- if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
- dri2_surf->base.Height != dri2_surf->wl_win->height) {
+ if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
+ dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
Not initialising attached_{width,height} should not cause any problems
with these checks. By definition there cannot have been any buffers
allocated or attached between creation and the first draw call, so
there are no old buffers to release.
Eric explained pretty well the reason to touch attached_{width,height};
otherwise dEQP-EGL.functional.resize.surface_size.* tests continue to fail.
J.A.
So with that initialisation removed, this is (assuming dEQP still passes):
Reviewed-by: Daniel Stone
Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>From 0e82ac55c7bba15b2f17eda74e492c0d6040e7e0 Mon Sep 17 00:00:00 2001
From: Brendan King
Date: Tue, 25 Jul 2017 16:01:22 +0100
Subject: [PATCH] Fix the deqp-egl *resize.surface_size* for Wayland
The Wayland version of deqp-egl calls wl_egl_window_get_attached_size
to get window dimensions. On Mesa at least, this initially returns a
width and height of zero for a new window. The surface resize tests
check the initial surface size, and fail as a result.
Add a new native window capability, CAPABILITY_INITIAL_SURFACE_SIZE_QUERY,
which is only set if the initial window size can be queried after
creation. If native windows don't have the capability, return the
required size from getNativeSurfaceSize, rather than the size obtained
from the window system.
A parameter has been added to getNativeSurfaceSize, to indicate if
the initial size of a surface is being queried.
---
framework/egl/egluNativeWindow.hpp | 3 ++-
framework/platform/X11/tcuX11EglPlatform.cpp | 3 ++-
framework/platform/win32/tcuWin32EGLNativeDisplayFactory.cpp | 3 ++-
modules/egl/teglResizeTests.cpp | 9 ++---
4 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/framework/egl/egluNativeWindow.hpp b/framework/egl/egluNativeWindow.hpp
index e468a23..1a4054d 100644
--- a/framework/egl/egluNativeWindow.hpp
+++ b/framework/egl/egluNativeWindow.hpp
@@ -81,7 +81,8 @@ public:
CAPABILITY_SET_SURFACE_SIZE = (1<<3),
CAPABILITY_GET_SCREEN_SIZE = (1<<4),
CAPABILITY_READ_SCREEN_PIXELS = (1<<5),
-
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
The IMG patch Eric was referring to was to dEQP. I submitted bug reports
to both Google and Khronos:
https://issuetracker.google.com/issues/64059452
https://gitlab.khronos.org/Tracker/vk-gl-cts/issues/594
I've attached the patch to this email. The problem is that the dEQP
*resize.surface_size* tests check the native window dimensions after
creating the window, without having called eglSwapBuffers, and hence get
back a native window width and height of zero.
Here is the patch description:
"The Wayland version of deqp-egl calls wl_egl_window_get_attached_size
to get window dimensions. On Mesa at least, this initially returns a
width and height of zero for a new window. The surface resize tests
check the initial surface size, and fail as a result.
Add a new native window capability, CAPABILITY_INITIAL_SURFACE_SIZE_QUERY,
which is only set if the initial window size can be queried after
creation. If native windows don't have the capability, return the
required size from getNativeSurfaceSize, rather than the size obtained
from the window system.
A parameter has been added to getNativeSurfaceSize, to indicate if
the initial size of a surface is being queried".
Brendan.
On 01/06/18 09:36, Juan A. Suarez Romero wrote:
On Thu, 2018-05-31 at 17:24 +0100, Daniel Stone wrote:
Hi Eric,
On 31 May 2018 at 17:13, Eric Engestrom wrote:
On Thursday, 2018-05-31 16:57:17 +0100, Daniel Stone wrote:
Not initialising attached_{width,height} should not cause any problems
with these checks. By definition there cannot have been any buffers
allocated or attached between creation and the first draw call, so
there are no old buffers to release.
So with that initialisation removed, this is (assuming dEQP still passes):
IMG has a similar patch locally (Cc'ed Frank), because (IIRC) dEQP
creates a window, and before attaching any surface to it, it reads back
the size and asserts out of most tests because it's 0 or -1 or something
like that, and I think this attached_width/height hack was needed
because of that.
That's odd. As I understand it, eglQuerySurface(EGL_{WIDTH,HEIGHT})
should be returning the base surface values. attached_{width,height}
should only be returned to the user when calling
wl_egl_window_get_attached_size(). I'm happy to fix the former for
consistency with other platforms, but anyone calling the latter
without ever having attached anything deserves whatever they get, to
be honest.
What dEQP-EGL.functional.resize.surface_size.* is doing:
- Initialize:
- Create a wayland window with specific size (128x128)
- Create the surface with eglQuerySurface()
- Perform the proper test
- Check if current surface size is the same as window size
- Resize the window
- Swap buffers
- Check again if the current surface size is the same as the window size
The first check is the one that it is failing.
I'm not sure if the test should swap buffers before the first check, as I didn't
find any specification or documentation stating it. But without such call,
seems to work in other platforms.
J.A.
Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>From 0e82ac55c7bba15b2f17eda74e492c0d6040e7e0 Mon Sep 17 00:00:00 2001
From: Brendan King
Date: Tue, 25 Jul 2017 16:01:22 +0100
Subject: [PATCH] Fix the deqp-egl *resize.surface_size* for Wayland
The Wayland version of deqp-egl calls wl_egl_window_get_attached_size
to get window dimensions. On Mesa at least, this initially returns a
width and height of zero for a new window. The surface resize tests
check the initial surface size, and fail as a result.
Add a new native window capability, CAPABILITY_INITIAL_SURFACE_SIZE_QUERY,
which is only set if the initial window size can be queried after
creation. If native windows don't have the capability, return the
required size from getNativeSurfaceSize, rather than the size obtained
from the window system.
A parameter has been added to getNativeSurfaceSize, to indicate if
the initial size of a surface is being queried.
---
framework/egl/egluNativeWindow.hpp | 3 ++-
framework/platform/X11/tcuX11EglPlatform.cpp | 3 ++-
framework/platform/win32/tcuWin32EGLNativeDisplayFactory.cpp | 3 ++-
modules/egl/teglResizeTests.cpp | 9 ++---
4 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/framework/egl/egluNativeWindow.hpp b/framework/egl/egluNativeWindow.hpp
index e468a23..1a4054d 100644
--- a/framework/egl/egluNativeWindow.hpp
+++ b/framework/egl/egluNativeWindow.hpp
@@ -81,7 +81,8 @@ public:
CAPABILITY_SET_SURFACE_SIZE = (1<<3),
CAPABILITY_GET_SCREEN_SIZE = (1<<4),
CAPABILITY_READ_SCREEN_PIXELS = (1<<5),
- CAPABILITY_CHANGE_VISIBILITY = (1<<6)
+ CAPABILITY_CHANGE_VISIBILITY = (1<<6),
+ CAPABILITY_INITIAL_SURFACE_SIZE_QUERY = (1<<7)
};
virtual
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Fri, 2018-06-01 at 09:39 +0100, Brendan King wrote:
> The IMG patch Eric was referring to was to dEQP. I submitted bug
> reports to Google and Khronos:
>
> https://issuetracker.google.com/issues/64059452
>
> https://gitlab.khronos.org/Tracker/vk-gl-cts/issues/594
>
> I've attached the patch to this email. The problem is that the dEQP
> *resize.surface_size* tests check the native window
> dimensions after creating the window, without having called
> eglSwapBuffers, and hence get back a width and height of zero,
> resulting in the tests failing.
Thanks for the reference.
My question then: is mandatory to call eglSwapBuffers() to ensure
wl_egl_window_get_attached_size() returns the right window size? Because I
found nothing related about this. If it is mandatory, then calling
eglSwapBuffers() in the test should fix the problem (including the patch I've
summited with Daniel's changes).
J.A.
>
>
> Here is the patch description:
>
>
> The Wayland version of deqp-egl calls wl_egl_window_get_attached_sizeto
> get window dimensions. On Mesa at least, this initially returns awidth and
> height of zero for a new window. The surface resize testscheck the initial
> surface size, and fail as a result.
> Add a new native window capability,
> CAPABILITY_INITIAL_SURFACE_SIZE_QUERY,which is only set if the initial window
> size can be queried aftercreation. If native windows don't have the
> capability, return therequired size from getNativeSurfaceSize, rather than
> the size obtainedfrom the window system.
> A parameter has been added to getNativeSurfaceSize, to indicate ifthe initial
> size of a surface is being queried.
> Brendan.
>
> On 01/06/18 09:12, Juan A. Suarez
> Romero wrote:
>
>
>
> > On Thu, 2018-05-31 at 16:57 +0100, Daniel Stone wrote:
> >
> >
> > > Hi Juan,Thanks for picking this up!
> > > On 31 May 2018 at 16:44, Juan A. Suarez Romero
> > > wrote:
> > >
> > >
> > > > @@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver
> > > > *drv, _EGLDisplay *disp, goto cleanup_surf;}
> > > > + dri2_surf->base.Width = window->width;+ dri2_surf->base.Height =
> > > > window->height;++ window->attached_width = dri2_surf->base.Width;+
> > > > window->attached_height = dri2_surf->base.Height;
> > > >
> > > >
> > >
> > > We should definitely not initialise attached_{width,height}
> > > here,because no buffer has ever been attached.
> > >
> > >
> > >
> > > > @@ -574,8 +582,8 @@ update_buffers(struct dri2_egl_surface
> > > > *dri2_surf)struct dri2_egl_display *dri2_dpy =
> > > > dri2_egl_display(dri2_surf->base.Resource.Display);
> > > > - if (dri2_surf->base.Width != dri2_surf->wl_win->width ||-
> > > > dri2_surf->base.Height != dri2_surf->wl_win->height) {+ if
> > > > (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||+
> > > > dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
> > > >dri2_wl_release_buffers(dri2_surf);
> > > > @@ -1629,8 +1637,8 @@ swrast_update_buffers(struct dri2_egl_surface
> > > > *dri2_surf)if (dri2_surf->back) return 0;
> > > > - if (dri2_surf->base.Width != dri2_surf->wl_win->width ||-
> > > > dri2_surf->base.Height != dri2_surf->wl_win->height) {+ if
> > > > (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||+
> > > > dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
> > > >
> > > >
> > >
> > > Not initialising attached_{width,height} should not cause any
> > > problemswith these checks. By definition there cannot have been any
> > > buffersallocated or attached between creation and the first draw call,
> > > sothere are no old buffers to release.
> > >
> > >
> >
> > Eric explained pretty well the reason to touch
> > attached_{width,height};otherwise dEQP-EGL.functional.resize.surface_size.*
> > tests continue to fail.
> >
> > J.A.
> >
> >
> >
> >
> > > So with that initialisation removed, this is (assuming dEQP still
> > > passes):Reviewed-by: Daniel Stone
> > > Cheers,Daniel
> > >
> > >
> > >
> >
> > ___mesa-dev mailing
> > listmesa-dev@lists.freedesktop.orghttps://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
>
>
>
>
> ___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Thu, 2018-05-31 at 17:24 +0100, Daniel Stone wrote:
> Hi Eric,
>
> On 31 May 2018 at 17:13, Eric Engestrom wrote:
> > On Thursday, 2018-05-31 16:57:17 +0100, Daniel Stone wrote:
> > > Not initialising attached_{width,height} should not cause any problems
> > > with these checks. By definition there cannot have been any buffers
> > > allocated or attached between creation and the first draw call, so
> > > there are no old buffers to release.
> > >
> > > So with that initialisation removed, this is (assuming dEQP still passes):
> >
> > IMG has a similar patch locally (Cc'ed Frank), because (IIRC) dEQP
> > creates a window, and before attaching any surface to it, it reads back
> > the size and asserts out of most tests because it's 0 or -1 or something
> > like that, and I think this attached_width/height hack was needed
> > because of that.
>
> That's odd. As I understand it, eglQuerySurface(EGL_{WIDTH,HEIGHT})
> should be returning the base surface values. attached_{width,height}
> should only be returned to the user when calling
> wl_egl_window_get_attached_size(). I'm happy to fix the former for
> consistency with other platforms, but anyone calling the latter
> without ever having attached anything deserves whatever they get, to
> be honest.
What dEQP-EGL.functional.resize.surface_size.* is doing:
- Initialize:
- Create a wayland window with specific size (128x128)
- Create the surface with eglQuerySurface()
- Perform the proper test
- Check if current surface size is the same as window size
- Resize the window
- Swap buffers
- Check again if the current surface size is the same as the window size
The first check is the one that it is failing.
I'm not sure if the test should swap buffers before the first check, as I didn't
find any specification or documentation stating it. But without such call,
seems to work in other platforms.
J.A.
>
> Cheers,
> Daniel
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Thu, 2018-05-31 at 16:57 +0100, Daniel Stone wrote:
> Hi Juan,
> Thanks for picking this up!
>
> On 31 May 2018 at 16:44, Juan A. Suarez Romero wrote:
> > @@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> > _EGLDisplay *disp,
> >goto cleanup_surf;
> > }
> >
> > + dri2_surf->base.Width = window->width;
> > + dri2_surf->base.Height = window->height;
> > +
> > + window->attached_width = dri2_surf->base.Width;
> > + window->attached_height = dri2_surf->base.Height;
>
> We should definitely not initialise attached_{width,height} here,
> because no buffer has ever been attached.
>
> > @@ -574,8 +582,8 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
> > struct dri2_egl_display *dri2_dpy =
> >dri2_egl_display(dri2_surf->base.Resource.Display);
> >
> > - if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> > - dri2_surf->base.Height != dri2_surf->wl_win->height) {
> > + if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
> > + dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
> >
> >dri2_wl_release_buffers(dri2_surf);
> >
> > @@ -1629,8 +1637,8 @@ swrast_update_buffers(struct dri2_egl_surface
> > *dri2_surf)
> > if (dri2_surf->back)
> >return 0;
> >
> > - if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> > - dri2_surf->base.Height != dri2_surf->wl_win->height) {
> > + if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
> > + dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
>
> Not initialising attached_{width,height} should not cause any problems
> with these checks. By definition there cannot have been any buffers
> allocated or attached between creation and the first draw call, so
> there are no old buffers to release.
Eric explained pretty well the reason to touch attached_{width,height};
otherwise dEQP-EGL.functional.resize.surface_size.* tests continue to fail.
J.A.
>
> So with that initialisation removed, this is (assuming dEQP still passes):
> Reviewed-by: Daniel Stone
>
> Cheers,
> Daniel
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
Hi Eric,
On 31 May 2018 at 17:13, Eric Engestrom wrote:
> On Thursday, 2018-05-31 16:57:17 +0100, Daniel Stone wrote:
>> Not initialising attached_{width,height} should not cause any problems
>> with these checks. By definition there cannot have been any buffers
>> allocated or attached between creation and the first draw call, so
>> there are no old buffers to release.
>>
>> So with that initialisation removed, this is (assuming dEQP still passes):
>
> IMG has a similar patch locally (Cc'ed Frank), because (IIRC) dEQP
> creates a window, and before attaching any surface to it, it reads back
> the size and asserts out of most tests because it's 0 or -1 or something
> like that, and I think this attached_width/height hack was needed
> because of that.
That's odd. As I understand it, eglQuerySurface(EGL_{WIDTH,HEIGHT})
should be returning the base surface values. attached_{width,height}
should only be returned to the user when calling
wl_egl_window_get_attached_size(). I'm happy to fix the former for
consistency with other platforms, but anyone calling the latter
without ever having attached anything deserves whatever they get, to
be honest.
Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
On Thursday, 2018-05-31 16:57:17 +0100, Daniel Stone wrote:
> Hi Juan,
> Thanks for picking this up!
>
> On 31 May 2018 at 16:44, Juan A. Suarez Romero wrote:
> > @@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> > _EGLDisplay *disp,
> >goto cleanup_surf;
> > }
> >
> > + dri2_surf->base.Width = window->width;
> > + dri2_surf->base.Height = window->height;
> > +
> > + window->attached_width = dri2_surf->base.Width;
> > + window->attached_height = dri2_surf->base.Height;
>
> We should definitely not initialise attached_{width,height} here,
> because no buffer has ever been attached.
>
> > @@ -574,8 +582,8 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
> > struct dri2_egl_display *dri2_dpy =
> >dri2_egl_display(dri2_surf->base.Resource.Display);
> >
> > - if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> > - dri2_surf->base.Height != dri2_surf->wl_win->height) {
> > + if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
> > + dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
> >
> >dri2_wl_release_buffers(dri2_surf);
> >
> > @@ -1629,8 +1637,8 @@ swrast_update_buffers(struct dri2_egl_surface
> > *dri2_surf)
> > if (dri2_surf->back)
> >return 0;
> >
> > - if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> > - dri2_surf->base.Height != dri2_surf->wl_win->height) {
> > + if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
> > + dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
>
> Not initialising attached_{width,height} should not cause any problems
> with these checks. By definition there cannot have been any buffers
> allocated or attached between creation and the first draw call, so
> there are no old buffers to release.
>
> So with that initialisation removed, this is (assuming dEQP still passes):
IMG has a similar patch locally (Cc'ed Frank), because (IIRC) dEQP
creates a window, and before attaching any surface to it, it reads back
the size and asserts out of most tests because it's 0 or -1 or something
like that, and I think this attached_width/height hack was needed
because of that.
> Reviewed-by: Daniel Stone
>
> 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
Re: [Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
Hi Juan,
Thanks for picking this up!
On 31 May 2018 at 16:44, Juan A. Suarez Romero wrote:
> @@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv,
> _EGLDisplay *disp,
>goto cleanup_surf;
> }
>
> + dri2_surf->base.Width = window->width;
> + dri2_surf->base.Height = window->height;
> +
> + window->attached_width = dri2_surf->base.Width;
> + window->attached_height = dri2_surf->base.Height;
We should definitely not initialise attached_{width,height} here,
because no buffer has ever been attached.
> @@ -574,8 +582,8 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
> struct dri2_egl_display *dri2_dpy =
>dri2_egl_display(dri2_surf->base.Resource.Display);
>
> - if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> - dri2_surf->base.Height != dri2_surf->wl_win->height) {
> + if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
> + dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
>
>dri2_wl_release_buffers(dri2_surf);
>
> @@ -1629,8 +1637,8 @@ swrast_update_buffers(struct dri2_egl_surface
> *dri2_surf)
> if (dri2_surf->back)
>return 0;
>
> - if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
> - dri2_surf->base.Height != dri2_surf->wl_win->height) {
> + if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
> + dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
Not initialising attached_{width,height} should not cause any problems
with these checks. By definition there cannot have been any buffers
allocated or attached between creation and the first draw call, so
there are no old buffers to release.
So with that initialisation removed, this is (assuming dEQP still passes):
Reviewed-by: Daniel Stone
Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] wayland/egl: initialize window surface size to window size
When creating a windows surface with eglCreatesWindowSurface(), the
width and height returned by eglQuerySurface(EGL_{WIDTH,HEIGHT}) is
invalid until buffers are updated (like calling glClear()).
Likewise, on top of above resizing the Wayland window and quering again
the surface size, returns the old size, until buffers are updated again.
This commit fixes this situation, returning in both cases the right
surface size, without requiring updating the buffers.
This fixes the following OpenGL CTS tests when runnig in Wayland:
- dEQP-EGL.functional.color_clears.*
- dEQP-EGL.functional.resize.surface_size.*
CC: Daniel Stone
---
src/egl/drivers/dri2/platform_wayland.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/egl/drivers/dri2/platform_wayland.c
b/src/egl/drivers/dri2/platform_wayland.c
index 63da21cdf55..f86c6dd1e52 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -198,6 +198,8 @@ resize_callback(struct wl_egl_window *wl_win, void *data)
struct dri2_egl_display *dri2_dpy =
dri2_egl_display(dri2_surf->base.Resource.Display);
+ dri2_surf->base.Width = wl_win->width;
+ dri2_surf->base.Height = wl_win->height;
dri2_dpy->flush->invalidate(dri2_surf->dri_drawable);
}
@@ -255,6 +257,12 @@ dri2_wl_create_window_surface(_EGLDriver *drv, _EGLDisplay
*disp,
goto cleanup_surf;
}
+ dri2_surf->base.Width = window->width;
+ dri2_surf->base.Height = window->height;
+
+ window->attached_width = dri2_surf->base.Width;
+ window->attached_height = dri2_surf->base.Height;
+
visual_idx = dri2_wl_visual_idx_from_config(dri2_dpy, config);
assert(visual_idx != -1);
@@ -574,8 +582,8 @@ update_buffers(struct dri2_egl_surface *dri2_surf)
struct dri2_egl_display *dri2_dpy =
dri2_egl_display(dri2_surf->base.Resource.Display);
- if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
- dri2_surf->base.Height != dri2_surf->wl_win->height) {
+ if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
+ dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
dri2_wl_release_buffers(dri2_surf);
@@ -1629,8 +1637,8 @@ swrast_update_buffers(struct dri2_egl_surface *dri2_surf)
if (dri2_surf->back)
return 0;
- if (dri2_surf->base.Width != dri2_surf->wl_win->width ||
- dri2_surf->base.Height != dri2_surf->wl_win->height) {
+ if (dri2_surf->wl_win->attached_width != dri2_surf->wl_win->width ||
+ dri2_surf->wl_win->attached_height != dri2_surf->wl_win->height) {
dri2_wl_release_buffers(dri2_surf);
--
2.17.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
