[vlc-devel] [PATCH] qt(wayland): patches to fix video display issues
erwan.tulou at gmail.com
erwan.tulou at gmail.com
Fri Dec 15 19:55:48 CET 2017
Hello,
On 13/12/2017 14:21, Rémi Denis-Courmont wrote:
>
> ://www.avast.com/antivirus
> 1) This needs a clarifying comment in code.
> 2) Grammar. Also not clear how this actually works (if it works).
Since Jean-Baptiste committed the main patch regarding this qt and
Wayland story, I am resubmitting an updated version of the other patches
in the hope that I answer the comments that were issued.
These patches have no dependencies whatsoever. So, they can be
partially applied if need be.
patch1: videowidget management when undocking the playlist
This patch fixes a crash for Wayland and incidentally also benefits
other platforms. The patch should not pose a problem.
patch2: fix qt crash when toggling between playlist and full screen
in the main window
This patch is a not-so-clean hack, but the only way to solve a
current qt limitation without major redesign. The only good reason to
apply the patch is that it fixes a very conspicuous crash within the
main window.
patch3: EGL/wayland issue
I know this patch is badly regarded, but since Opengl is the default
vout display, something has to be done to fix it. I fail to see any
problem, but I leave it up to you to apply it or come up with a better
solution.
The patch has been slightly modified to also fix a null dereference.
patch4: width/height with null value
This patch is an easy way to sweep the problem under the carpet ! It
does fix crashes, but may hide some other issues. The patch is to be
applied if fixing another conspicuous crash right away is important, or
wait till a better long term solution is found.
Rgds
Erwan Tulou
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
-------------- next part --------------
From 2444100f242a0d660641880a8ee0dee4b3f612d2 Mon Sep 17 00:00:00 2001
From: Erwan Tulou <erwan10 at videolan.org>
Date: Tue, 12 Dec 2017 14:06:10 +0100
Subject: [PATCH 1/4] qt: fix crash when undocking playlist on Wayland +
optimization elsewhere
Since the video widget is always kept in the main interface, just making
sure it is relocated __before__ moving the playlistwidget to the playlist
dialog saves the two calls to the display server needed to reparent the
native window back and forth at the qt level.
For Wayland, this fixes a crash, since qt is unable to reparent due to
Wayland limitations, anyway.
For other platforms, this is an optimization, since we avoid two useless
often inter-process communications (reparenting back and forth)
---
modules/gui/qt/main_interface.cpp | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/modules/gui/qt/main_interface.cpp b/modules/gui/qt/main_interface.cpp
index 3b576ac23d..e8cdce51e0 100644
--- a/modules/gui/qt/main_interface.cpp
+++ b/modules/gui/qt/main_interface.cpp
@@ -1063,12 +1063,21 @@ void MainInterface::dockPlaylist( bool p_docked )
if( !p_docked ) /* Previously docked */
{
playlistVisible = playlistWidget->isVisible();
- stackCentralW->removeWidget( playlistWidget );
- dialog->importPlaylistWidget( playlistWidget );
+
+ /* repositioning the videowidget __before__ exporting the
+ playlistwidget into the playlist dialog avoids two unneeded
+ calls to the server in the qt library to reparent the underlying
+ native window back and forth.
+ For Wayland, this is mandatory since reparenting is not implemented.
+ For X11 or Windows, this is just an optimization. */
if ( videoWidget && THEMIM->getIM()->hasVideo() )
showTab(videoWidget);
else
showTab(bgWidget);
+
+ /* playlistwidget exported into the playlist dialog */
+ stackCentralW->removeWidget( playlistWidget );
+ dialog->importPlaylistWidget( playlistWidget );
if ( playlistVisible ) dialog->show();
}
else /* Previously undocked */
--
2.14.1
-------------- next part --------------
From da13bdf5af254a5cf56bdede1a234f748c85130f Mon Sep 17 00:00:00 2001
From: Erwan Tulou <erwan10 at videolan.org>
Date: Tue, 12 Dec 2017 14:14:02 +0100
Subject: [PATCH 2/4] qt(wayland): fix crash when toggling between playlist and
full video
Due to Wayland limitations, qt opts for destroying the native window rather
than just hiding it. This leads vlc to crash in the vout display.
This patch reimplements the setVisible() method to avoid hiding the video
widget while a video is currently running.
This is clearly a hack for Wayland, and should be removed as soon as qt
comes up with a less restrictive implementation. The hack is coded in such
a way that it is confined to the use of Wayland.
Some tests have reported possible UI artefacts depending on the Wayland
server. But no more crashes.
---
modules/gui/qt/components/interface_widgets.cpp | 19 +++++++++++++++++++
modules/gui/qt/components/interface_widgets.hpp | 1 +
2 files changed, 20 insertions(+)
diff --git a/modules/gui/qt/components/interface_widgets.cpp b/modules/gui/qt/components/interface_widgets.cpp
index 98dfdebfbe..101b1b2851 100644
--- a/modules/gui/qt/components/interface_widgets.cpp
+++ b/modules/gui/qt/components/interface_widgets.cpp
@@ -98,6 +98,25 @@ VideoWidget::~VideoWidget()
assert( !p_window );
}
+void VideoWidget::setVisible( bool visible )
+{
+#ifdef QT5_HAS_WAYLAND
+
+ /* On Wayland, qt destroys the stable widget native
+ window when setVisible(false) or hide() is called.
+ So, just never hide if a video is running */
+
+ if( p_window && p_window->type == VOUT_WINDOW_TYPE_WAYLAND)
+ if( !visible )
+ {
+ msg_Dbg(p_intf,"VideoWidget doesn't hide on Wayland");
+ return;
+ }
+#endif
+
+ QFrame::setVisible( visible );
+}
+
void VideoWidget::sync( void )
{
/* Make sure the X server has processed all requests.
diff --git a/modules/gui/qt/components/interface_widgets.hpp b/modules/gui/qt/components/interface_widgets.hpp
index e56b828021..dc2cc5d36f 100644
--- a/modules/gui/qt/components/interface_widgets.hpp
+++ b/modules/gui/qt/components/interface_widgets.hpp
@@ -75,6 +75,7 @@ protected:
void mouseMoveEvent(QMouseEvent *) Q_DECL_OVERRIDE;
void mouseReleaseEvent(QMouseEvent *) Q_DECL_OVERRIDE;
void mouseDoubleClickEvent(QMouseEvent *) Q_DECL_OVERRIDE;
+ void setVisible(bool) Q_DECL_OVERRIDE;
QSize physicalSize() const;
private:
--
2.14.1
-------------- next part --------------
From 01c982e0043a6b538253c24e4c6a27855acba264 Mon Sep 17 00:00:00 2001
From: Erwan Tulou <erwan10 at videolan.org>
Date: Sun, 10 Dec 2017 17:24:35 +0100
Subject: [PATCH 3/4] egl(wayland): fix vlc_gl_Resize()
This function should report changes in size only about the original display.
All subsequent video placements are managed in vout_display_opengl_Viewport().
This fixes video misplacements found when running vlc on Wayland with opengl.
---
modules/video_output/opengl/display.c | 2 +-
modules/video_output/opengl/egl.c | 8 ++------
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/modules/video_output/opengl/display.c b/modules/video_output/opengl/display.c
index 2ecdd0caaa..2ace457a7d 100644
--- a/modules/video_output/opengl/display.c
+++ b/modules/video_output/opengl/display.c
@@ -253,7 +253,7 @@ static int Control (vout_display_t *vd, int query, va_list ap)
c.align.vertical = VOUT_DISPLAY_ALIGN_TOP;
vout_display_PlacePicture (&place, src, &c, false);
- vlc_gl_Resize (sys->gl, place.width, place.height);
+ vlc_gl_Resize (sys->gl, c.display.width, c.display.height);
if (vlc_gl_MakeCurrent (sys->gl) != VLC_SUCCESS)
return VLC_EGENERIC;
vout_display_opengl_SetWindowAspectRatio(sys->vgl, (float)place.width / place.height);
diff --git a/modules/video_output/opengl/egl.c b/modules/video_output/opengl/egl.c
index 9162675447..cdcb4a41f5 100644
--- a/modules/video_output/opengl/egl.c
+++ b/modules/video_output/opengl/egl.c
@@ -53,7 +53,6 @@ typedef struct vlc_gl_sys_t
#endif
#if defined (USE_PLATFORM_WAYLAND)
struct wl_egl_window *window;
- unsigned width, height;
#endif
PFNEGLCREATEIMAGEKHRPROC eglCreateImageKHR;
PFNEGLDESTROYIMAGEKHRPROC eglDestroyImageKHR;
@@ -82,11 +81,8 @@ static void Resize (vlc_gl_t *gl, unsigned width, unsigned height)
{
vlc_gl_sys_t *sys = gl->sys;
- wl_egl_window_resize(sys->window, width, height,
- (sys->width - width) / 2,
- (sys->height - height) / 2);
- sys->width = width;
- sys->height = height;
+ if (sys->window != NULL)
+ wl_egl_window_resize(sys->window, width, height, 0, 0);
}
#else
# define Resize (NULL)
--
2.14.1
-------------- next part --------------
From f59ff93372defcf8beabe8f5ced5ccb44d7b3a37 Mon Sep 17 00:00:00 2001
From: Erwan Tulou <erwan10 at videolan.org>
Date: Wed, 8 Nov 2017 01:37:20 +0100
Subject: [PATCH 4/4] qt: fix width/height with zero value
In a full wayland context, these width/height may come up as equal to 0, and
this leads to vlc crashing in both the wl and the gl vout display.
Not sure though if these zero can be valid values in this context ?
The patch seems to solve the issue without any obvious regression.
---
modules/gui/qt/main_interface.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/modules/gui/qt/main_interface.cpp b/modules/gui/qt/main_interface.cpp
index e8cdce51e0..419fbc8d6a 100644
--- a/modules/gui/qt/main_interface.cpp
+++ b/modules/gui/qt/main_interface.cpp
@@ -1331,7 +1331,8 @@ void MainInterface::resizeWindow(int w, int h)
return;
}
#endif
- resize(w, h);
+ if( w > 0 && h > 0 )
+ resize(w, h);
}
/**
--
2.14.1
More information about the vlc-devel
mailing list