Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

inherit unversioned QOpenGLFunctions instead of QOpenGLFunctions_2_1 #12803

Closed
wants to merge 1 commit into from
Closed

inherit unversioned QOpenGLFunctions instead of QOpenGLFunctions_2_1 #12803

wants to merge 1 commit into from

Conversation

Be-ing
Copy link
Contributor

@Be-ing Be-ing commented Feb 12, 2024

Currently the aarch64 KDE Flatpak runtime is built with -opengl es passed to Qt's configure script. It's questionable whether that's necessary, but regardless, Mixxx can support Qt built with that option.
https://invent.kde.org/packaging/flatpak-kde-runtime/-/issues/19

Fixes #12802

@Be-ing Be-ing requested a review from m0dB February 12, 2024 20:04
@Be-ing Be-ing added this to the 2.4.0 milestone Feb 12, 2024
@Be-ing Be-ing added the blocker label Feb 12, 2024
@Be-ing Be-ing force-pushed the opengles branch 2 times, most recently from 84eb38e to e51ea2d Compare February 12, 2024 21:13
@daschuer
Copy link
Member

We already have a release candidate and this is not a regression (except reverting the build error). So lets postpone this to 2.4.1 after some testing.

@daschuer daschuer removed the blocker label Feb 12, 2024
@daschuer daschuer modified the milestones: 2.4.0, 2.4.1 Feb 12, 2024
@daschuer
Copy link
Member

Did you consider to use QOpenGLFunctions_2_1 or QOpenGLFunctions_E2 conditionally depending on the QT_OPENGL_ES_2 flag? This way the change does not affect the other builds.

@Be-ing Be-ing mentioned this pull request Feb 13, 2024
Closed
@Be-ing
inherit from QOpenGLFunctions instead of QOpenGLFunctions_2_1
4cae9d7 
The generic QOpenGLFunctions class supports OpenGL ES 2.0, the
minimum supported by Qt 5. Mixxx builds and runs fine with this;
desktop OpenGL 2.1 is only required for the legacy OpenGL
waveform renderers.
@Be-ing Be-ing changed the title support building with Qt built for OpenGL ES inherit unversioned QOpenGLFunctions instead of QOpenGLFunctions_2_1 Feb 13, 2024
@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 13, 2024

Did you consider to use QOpenGLFunctions_2_1 or QOpenGLFunctions_E2 conditionally depending on the QT_OPENGL_ES_2 flag? This way the change does not affect the other builds.

I think that would be overcomplicated. This simpler code works.

@daschuer
Copy link
Member

The difference is that this has an impact on all builds, which needs a good amount of test. The conditional option will only affect GLES only builds and will fail at compile time, not at runtime. I consider such a solution as a reasonable Quick 2.4.1 fix.

By the way, this enables the LEGACY GL waveforms, that are scheduled for removal anyway. So we may also consider to close this without a merge.

@Be-ing
Copy link
Contributor Author

Be-ing commented Feb 14, 2024

Now that the urgent build failure has been fixed with #12804, it is not urgent to merge this to the 2.4 branch, and I agree this would need thorough testing.

By the way, this enables the LEGACY GL waveforms, that are scheduled for removal anyway. So we may also consider to close this without a merge.

That makes sense.

@Be-ing Be-ing closed this Feb 14, 2024
@daschuer daschuer removed this from the 2.4.1 milestone Feb 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m0dB m0dB Awaiting requested review from m0dB

Assignees
No one assigned
Labels
waveform
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Be-ing @daschuer