Avoid crash with EGLFS by daschuer · Pull Request #15874 · mixxxdj/mixxx

daschuer · 2026-01-18T12:16:47Z

With EGLFS we can use either QWidgets or OpenGL, not both at the same time.
This is a band aid that shall avoid the fatal abort reported in #15863.

A better fix would be to use only GELS.
This type of refactoring is probably not beneficial because we are already working in a QML GUI which likely works with EGLFS. We need to investigate with this.

JoergAtGithub · 2026-01-18T13:07:16Z

As we've two implementations for the waveforms (OpenGL and QQuick) would it help to switch from QOpenGLWindow to doc.qt.io/qt-6/qquickwidget.html using the QQuick waveform implementation we use for QML already?
This would also solve the problem that Apple deprecated OpenGL at all and support might be removed with an upcoming macOS release.

daschuer · 2026-01-18T13:51:00Z

I don't think so. EGLFS is used with a frame buffer device, where no windows manager is available. See https://doc.qt.io/qt-6/embedded-linux.html.
We either use QWidget, where Qt handles the off-screen rendering and screen updates or render everything with GLES. Since we currently use both only QWidget waveforms are available.

I can't test it though. Hope @mirko can confirm.

mirko · 2026-01-18T18:23:15Z

fb(dev) is legacy from kernel side by now and AFAIK is - if still used - routed via DRM/KMS.
I'm going the direct way via DRM/KMS. I only ever dealt with OpenGLES2 and Qt, so unfortunately I don't know about the OpenGL stuff. But as far as I remember, for Qt5 you could compile qt5gui either with OpenGL or OpenGLES support (see e.g. https://packages.debian.org/trixie/libqt5gui5-gles), whereas with Qt6 that's interchangeable at runtime. Probably not answering any of your questions, though :)

JoergAtGithub · 2026-01-18T18:25:05Z

See: https://doc.qt.io/qt-6/embedded-linux.html

mirko · 2026-01-18T18:27:38Z

The environment I use for reference:

export QT_QPA_PLATFORM=eglfs
export QT_QPA_EGLFS_INTEGRATION=eglfs_kms
export QT_QPA_EGLFS_KMS_CONFIG=/etc/qt_eglfs_kms.json
mixxx --qml --developer

whereas /etc/qt_eglfs_kms.json contains:

{
  "hwcursor": false,
  "outputs": [
    {
      "name": "HDMI1",
      "mode": "1920x1080"
    }
  ]
}

Copilot

Pull request overview

This pull request addresses issue #15863 where Mixxx crashes on OpenGLES2-only systems using the EGLFS (EGL Full Screen) platform with DRM/KMS backend. The root cause is that EGLFS can only support either QWidgets or OpenGL windows, not both simultaneously. When Mixxx tries to embed OpenGL windows into its QWidgets-based main window on EGLFS, it causes a fatal crash.

Changes:

Added EGLFS platform detection in both the waveform factory initialization and main window OpenGL initialization
Modified OpenGL availability flag logic to prevent setting OpenGL as available when running on EGLFS
Enhanced getSurfaceFormat() to safely handle nullptr config parameter

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/waveform/waveformwidgetfactory.h	Renamed parameter from `config` to `pConfig` to follow pointer naming convention
src/waveform/waveformwidgetfactory.cpp	Added EGLFS detection logic, prevented OpenGL availability flags from being set on EGLFS, and made getSurfaceFormat() handle nullptr safely
src/mixxxmainwindow.cpp	Added EGLFS detection to skip OpenGL window creation, added debug logging for OpenGL context creation, and renamed variables to follow pointer naming convention

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

daschuer · 2026-02-16T09:44:17Z

Done

ronso0 · 2026-02-16T10:59:03Z

Just came here as I was curious about the Copilot review. Shouldn't the include commit be squashed into the one with the related code change, so every commit can be built and run?
Or was that a false positive?

daschuer · 2026-02-17T22:42:55Z

This was on one hand a false positive. It was already included by vinylcontrolmanager.h however that is guarded by __VINYLCONTROL__ , so it is a good idea to add the include explicit.

daschuer · 2026-02-18T10:30:48Z

The precommit issue is unrelated, it will be fixed with #16004

ywwg · 2026-02-24T20:54:48Z

from the dev discussion: we should really get this in

daschuer · 2026-02-28T22:20:29Z

pre-commit fails because of the metafile issue in 2.6 when I have pushed. I have rebased and pushed again. I hope that was OK.

ywwg · 2026-03-01T20:31:28Z

code looks fine to me, do we need someone to test? (and how?)

acolombier · 2026-03-28T23:57:22Z

+            } else if (pContext->isOpenGLES()) {
+                m_openGLVersion = "ES ";
+            }
+


This used to be init-ed to an empty string if not isOpenGLES. Not sure if there is a risk this strings need to be cleared? Perhaps a DEBUG_ASSERT is enough.

Suggested change

} else if (pContext->isOpenGLES()) {

m_openGLVersion = "ES ";

}

} else if (pContext->isOpenGLES()) {

m_openGLVersion = "ES ";

} else {

m_openGLVersion = "";

}

We are here in the constructor, the constructor initializes the QString as empty. So nothing to do IMHO, except using QStringLiteral. I will also add a comment.

acolombier · 2026-03-29T00:02:01Z

+                if (pContext->isOpenGLES()) {
+                    if (majorVersion * 100 + minorVersion >= 200) {
+                        m_openGlesAvailable = true;
+                    }
+                } else {
+                    if (majorVersion * 100 + minorVersion >= 201) {
+                        m_openGlAvailable = true;
+                    }
                }


Could we merge this condition to help readability?

Suggested change

if (pContext->isOpenGLES()) {

if (majorVersion * 100 + minorVersion >= 200) {

m_openGlesAvailable = true;

}

} else {

if (majorVersion * 100 + minorVersion >= 201) {

m_openGlAvailable = true;

}

}

if (pContext->isOpenGLES() && majorVersion * 100 + minorVersion >= 200) {

m_openGlesAvailable = true;

} else if (majorVersion * 100 + minorVersion >= 201) {

m_openGlAvailable = true;

}

Also wondering if we need to set m_openGlesAvailable and m_openGlAvailable, such as

Suggested change

if (pContext->isOpenGLES()) {

if (majorVersion * 100 + minorVersion >= 200) {

m_openGlesAvailable = true;

}

} else {

if (majorVersion * 100 + minorVersion >= 201) {

m_openGlAvailable = true;

}

}

m_openGlesAvailable = pContext->isOpenGLES() && majorVersion * 100 + minorVersion >= 200;

m_openGlAvailable = !pContext->isOpenGLES() && majorVersion * 100 + minorVersion >= 201;

If not, could you please put a comment to highlight that these are expected to be set to false otherwise during another step?

acolombier · 2026-03-29T00:22:14Z

+    // With EGLFS there is always exactly one native window and one EGL window surface
+    // OpenGL windows cannot be embedded into our QWidgets main window we already have.
+    // https://doc.qt.io/qt-6/embedded-linux.html
+    bool isEglfs = QGuiApplication::platformName() == "eglfs";


Do we know if this string is normalised? Wondering if somebody passes QT_QPA_PLATFORM=EGLFS, whether the platform name would remain lower cased or ignored, of if we should normalise the output. I am under the impression it should be the doc isn't completely clear about this.

https://doc.qt.io/qt-6/qguiapplication.html#platformName-prop only one spelling is mentioned. I think we can trust it.
I will add a comment.

Co-authored-by: Antoine Colombier <7086688+acolombier@users.noreply.github.com>

daschuer · 2026-03-30T10:20:46Z

Done.

acolombier

LGTM!

github-actions Bot added the waveform label Jan 18, 2026

daschuer changed the base branch from main to 2.6 February 6, 2026 15:47

JoergAtGithub requested a review from Copilot February 16, 2026 08:08

Copilot started reviewing on behalf of JoergAtGithub February 16, 2026 08:08 View session

Copilot AI reviewed Feb 16, 2026

View reviewed changes

Comment thread src/mixxxmainwindow.cpp

Comment thread src/mixxxmainwindow.cpp

daschuer force-pushed the eglfs26 branch from 39ff498 to bfcc166 Compare February 17, 2026 22:46

daschuer added this to the 2.6.0 milestone Feb 18, 2026

ywwg added the blocker label Feb 24, 2026

acolombier self-requested a review February 24, 2026 20:55

daschuer added 3 commits February 28, 2026 23:18

Fix possible null pointer dereference

e54e208

Don't allow GL widgets when we alreay have normal widgets under EGLFS.

1b1aa92

Add missing include

c85da97

daschuer force-pushed the eglfs26 branch from bfcc166 to c85da97 Compare February 28, 2026 22:18

ry-gatoni mentioned this pull request Mar 25, 2026

Require compatibility profile context when not forcing OpenGL ES #16214

Closed

acolombier reviewed Mar 29, 2026

View reviewed changes

daschuer and others added 3 commits March 30, 2026 12:18

make pWidget a partented_ptr

4d756a6

Co-authored-by: Antoine Colombier <7086688+acolombier@users.noreply.github.com>

Improve construction of m_openGLVersion

bcce816

Improve readability in version check.

a0e3606

Add a comment about "eglfs" spelling

4f82810

ywwg requested a review from acolombier April 2, 2026 13:28

acolombier approved these changes Apr 3, 2026

View reviewed changes

acolombier merged commit 1f3cd96 into mixxxdj:2.6 Apr 3, 2026
16 checks passed

daschuer mentioned this pull request Aug 3, 2026

QML: Mixxx freezes runing directly on DRM/KMS (QT_QPA_PLATFORM=eglfs) #15863

Open

Uh oh!

Uh oh!

Conversation

daschuer commented Jan 18, 2026

Uh oh!

JoergAtGithub commented Jan 18, 2026

Uh oh!

daschuer commented Jan 18, 2026

Uh oh!

mirko commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoergAtGithub commented Jan 18, 2026

Uh oh!

mirko commented Jan 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

daschuer commented Feb 16, 2026

Uh oh!

ronso0 commented Feb 16, 2026

Uh oh!

daschuer commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daschuer commented Feb 18, 2026

Uh oh!

ywwg commented Feb 24, 2026

Uh oh!

daschuer commented Feb 28, 2026

Uh oh!

ywwg commented Mar 1, 2026

Uh oh!

acolombier Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

daschuer Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

acolombier Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

acolombier Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

daschuer Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

daschuer commented Mar 30, 2026

Uh oh!

acolombier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mirko commented Jan 18, 2026 •

edited

Loading

mirko commented Jan 18, 2026 •

edited

Loading

daschuer commented Feb 17, 2026 •

edited

Loading