From: Dongwon Kim <dongwon.kim@intel.com>

This partially reverts commit 77bf310084dad38b3a2badf01766c659056f1cf2
which causes some guest display corruption when gtk-gl-area
is used for GTK rendering (e.g. Wayland Compositor) possibly due to
simulataneous accesses on the guest frame buffer by host compositor
and the guest.

Fixes: 77bf310084 ("ui/gtk: Draw guest frame at refresh cycle")
Reported-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reported-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
Message-Id: <20250214170813.2234754-1-dongwon.kim@intel.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-ID: <20250603110204.838117-12-alex.bennee@linaro.org>
Cc: qemu-stable@nongnu.org
---
 ui/gtk-gl-area.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
index 05fc38096ec..9a11c9b4d18 100644
--- a/ui/gtk-gl-area.c
+++ b/ui/gtk-gl-area.c
@@ -165,7 +165,6 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
 
     if (vc->gfx.guest_fb.dmabuf &&
         qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
-        gd_gl_area_draw(vc);
         return;
     }
 
-- 
2.47.3

On 2025/10/14 20:12, Alex Bennée wrote:
> From: Dongwon Kim <dongwon.kim@intel.com>
> 
> This partially reverts commit 77bf310084dad38b3a2badf01766c659056f1cf2
> which causes some guest display corruption when gtk-gl-area
> is used for GTK rendering (e.g. Wayland Compositor) possibly due to
> simulataneous accesses on the guest frame buffer by host compositor
> and the guest.
> 
> Fixes: 77bf310084 ("ui/gtk: Draw guest frame at refresh cycle")
> Reported-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Reported-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
> Message-Id: <20250214170813.2234754-1-dongwon.kim@intel.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-ID: <20250603110204.838117-12-alex.bennee@linaro.org>
> Cc: qemu-stable@nongnu.org
> ---
>   ui/gtk-gl-area.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
> index 05fc38096ec..9a11c9b4d18 100644
> --- a/ui/gtk-gl-area.c
> +++ b/ui/gtk-gl-area.c
> @@ -165,7 +165,6 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
>   
>       if (vc->gfx.guest_fb.dmabuf &&
>           qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
> -        gd_gl_area_draw(vc);


I suggested adding code comment for the lack of gd_gl_area_draw() here a 
while ago but it seems it is missed since then:
https://lore.kernel.org/qemu-devel/63911dcc-482b-45c5-9468-120ae3df691b@daynix.com/

The removal of this function call itself looks good to me.

>           return;
>       }
>   

Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:

> On 2025/10/14 20:12, Alex Bennée wrote:
>> From: Dongwon Kim <dongwon.kim@intel.com>
>> This partially reverts commit
>> 77bf310084dad38b3a2badf01766c659056f1cf2
>> which causes some guest display corruption when gtk-gl-area
>> is used for GTK rendering (e.g. Wayland Compositor) possibly due to
>> simulataneous accesses on the guest frame buffer by host compositor
>> and the guest.
>> Fixes: 77bf310084 ("ui/gtk: Draw guest frame at refresh cycle")
>> Reported-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> Reported-by: Alex Bennée <alex.bennee@linaro.org>
>> Tested-by: Alex Bennée <alex.bennee@linaro.org>
>> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
>> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
>> Message-Id: <20250214170813.2234754-1-dongwon.kim@intel.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-ID: <20250603110204.838117-12-alex.bennee@linaro.org>
>> Cc: qemu-stable@nongnu.org
>> ---
>>   ui/gtk-gl-area.c | 1 -
>>   1 file changed, 1 deletion(-)
>> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
>> index 05fc38096ec..9a11c9b4d18 100644
>> --- a/ui/gtk-gl-area.c
>> +++ b/ui/gtk-gl-area.c
>> @@ -165,7 +165,6 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
>>         if (vc->gfx.guest_fb.dmabuf &&
>>           qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
>> -        gd_gl_area_draw(vc);
>
>
> I suggested adding code comment for the lack of gd_gl_area_draw() here
> a while ago but it seems it is missed since then:
> https://lore.kernel.org/qemu-devel/63911dcc-482b-45c5-9468-120ae3df691b@daynix.com/
>
> The removal of this function call itself looks good to me.

What comment would you like - its not clear from the thread what I
should add.

>
>>           return;
>>       }
>>   

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 2025/10/15 19:45, Alex Bennée wrote:
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:
> 
>> On 2025/10/14 20:12, Alex Bennée wrote:
>>> From: Dongwon Kim <dongwon.kim@intel.com>
>>> This partially reverts commit
>>> 77bf310084dad38b3a2badf01766c659056f1cf2
>>> which causes some guest display corruption when gtk-gl-area
>>> is used for GTK rendering (e.g. Wayland Compositor) possibly due to
>>> simulataneous accesses on the guest frame buffer by host compositor
>>> and the guest.
>>> Fixes: 77bf310084 ("ui/gtk: Draw guest frame at refresh cycle")
>>> Reported-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> Reported-by: Alex Bennée <alex.bennee@linaro.org>
>>> Tested-by: Alex Bennée <alex.bennee@linaro.org>
>>> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
>>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Cc: Vivek Kasireddy <vivek.kasireddy@intel.com>
>>> Signed-off-by: Dongwon Kim <dongwon.kim@intel.com>
>>> Message-Id: <20250214170813.2234754-1-dongwon.kim@intel.com>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Message-ID: <20250603110204.838117-12-alex.bennee@linaro.org>
>>> Cc: qemu-stable@nongnu.org
>>> ---
>>>    ui/gtk-gl-area.c | 1 -
>>>    1 file changed, 1 deletion(-)
>>> diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c
>>> index 05fc38096ec..9a11c9b4d18 100644
>>> --- a/ui/gtk-gl-area.c
>>> +++ b/ui/gtk-gl-area.c
>>> @@ -165,7 +165,6 @@ void gd_gl_area_refresh(DisplayChangeListener *dcl)
>>>          if (vc->gfx.guest_fb.dmabuf &&
>>>            qemu_dmabuf_get_draw_submitted(vc->gfx.guest_fb.dmabuf)) {
>>> -        gd_gl_area_draw(vc);
>>
>>
>> I suggested adding code comment for the lack of gd_gl_area_draw() here
>> a while ago but it seems it is missed since then:
>> https://lore.kernel.org/qemu-devel/63911dcc-482b-45c5-9468-120ae3df691b@daynix.com/
>>
>> The removal of this function call itself looks good to me.
> 
> What comment would you like - its not clear from the thread what I
> should add.

Below is an idea of comment:

===

gd_egl_refresh() calls gd_egl_draw() if a DMA-BUF draw has already
been submitted, but this function does not call gd_gl_area_draw() in
such a case due to display corruption.

Calling gd_gl_area_draw() is necessary to prevent a situation where
there is a scheduled draw event but it won't happen bacause the window
is currently in inactive state (minimized or tabified). If draw is not
done for a long time, gl_block timeout and/or fence timeout (on the
guest) will happen eventually.

However, it is found that calling gd_gl_area_draw() here causes guest
display corruption on a Wayland Compositor. The display corruption is
more serious than the possible fence timeout so gd_gl_area_draw() is
omitted for now.

===

In the thread, it was discussed that the removal of gd_gl_area_draw() 
can cause a regression but it is necessary to fix a bigger problem 
(display corruption on Wayland), and I suggested to note the regression 
so that we won't lose track of it.

To remind of the regression, the comment first describes it omits 
gd_gl_area_draw(), and then explains why omitting gd_gl_area_draw() can 
cause a problem, which once fixed by the reverted change. Finally, it 
explains that omitting gd_gl_area_draw() is necessary to avoid display 
corruption on Wayland.

Regards,
Akihiko Odaki