obs-deps icon indicating copy to clipboard operation
obs-deps copied to clipboard

deps.qt: Add qtcharts

Open pkviet opened this issue 1 year ago • 12 comments

Description

This enables building of qtcharts (as well as qtopengl, qtopenglwidgets) on which it depends.

Motivation and Context

This is a companion to other PRs in obs-studio enabling stats charts for SRT [1] & maybe regular stats.

[1] https://github.com/obsproject/obs-studio/pull/9027

How Has This Been Tested?

Builds locally on windows and passes CI on my fork.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • [x] My code has been run through clang-format.
  • [x] I have read the contributing document.
  • [x] My code is not on the master branch.
  • [x] The code has been tested.
  • [x] All commit messages are properly formatted and commits squashed where appropriate.
  • [x] I have included updates to all appropriate documentation.

pkviet avatar Jun 05 '23 18:06 pkviet

As far as I can tell, qtopengl and qtopenglwidgets are optional dependencies of qtcharts, not required dependencies. Could you verify?

correct: https://github.com/qt/qtcharts/blob/dev/src/charts/CMakeLists.txt#L123-L128

pkviet avatar Jul 07 '23 19:07 pkviet

also: https://github.com/qt/qtcharts/blob/dev/CMakeLists.txt#L19-L20

pkviet avatar Jul 07 '23 19:07 pkviet

@RytoEX mmh even though qt6OpenGL is not mandatory it seems it builds with Qt6OpenGL and Qt6OpenGLWidgets on default even if the latter are not specified. See the screenshot: DependenciesGui_2023-07-08_06-49-18 I'll investigate if I do need OpenGL/OpenGLWidgets or not in my current code and how to force building without these deps.

pkviet avatar Jul 08 '23 04:07 pkviet

It seems Qt6OpenGL and Qt6OpenGlWidgets are already shipping with the deps. This is a screenshot from the last obs-deps release (06-22): explorer_2023-07-08_07-01-35

I checked the various qt dlls in the bin folder but none had QtOpenGL as a dependency ...

pkviet avatar Jul 08 '23 05:07 pkviet

I do see that. We don't currently ship them with OBS Studio itself because they aren't needed. Qt's default configuration is "dynamic OpenGL". The old Qt 5.15 build instructions explain the difference in configuration options. Maybe even though they are considered optional dependencies of Qt Charts, it tries to opportunistically load them?

RytoEX avatar Jul 08 '23 14:07 RytoEX

Seems to explicitly require -DQT_FEATURE_opengl:BOOL=NO to disable it opportunistically adding it.

The way the logic works is: "If OpenGL support is not disabled, use it". Thus it becomes a hard requirement of QtCharts. We haven't explicitly disabled it so far, but might be required now.

PatTheMav avatar Jul 16 '23 12:07 PatTheMav

Are the OpenGL DLLs copied in the obs-studio Qt Charts PR in CMake 3? Does Qt Charts fail to work if the OpenGL DLLs are not in the obs-studio directory?

RytoEX avatar Jul 16 '23 15:07 RytoEX

Are the OpenGL DLLs copied in the obs-studio Qt Charts PR in CMake 3? Does Qt Charts fail to work if the OpenGL DLLs are not in the obs-studio directory?

I think they might be - we resolve which DLLs to copy by recursively resolving the dependency tree, and I can imagine the following happening:

  • obs-studio depends on Qt6::Charts
  • Qt6::Charts depends on Qt6::OpenGL (because the CMake files are generated by Qt at build time)
  • obs-studio has Charts and OpenGL as direct dependencies

I would expect the only way to remove that is build Charts explicitly without it, so the dependency doesn't end up in Charts' CMake package file.

PatTheMav avatar Jul 16 '23 15:07 PatTheMav

Are the OpenGL DLLs copied in the obs-studio Qt Charts PR in CMake 3? Does Qt Charts fail to work if the OpenGL DLLs are not in the obs-studio directory?

Yes currently i'm copying the qt opengl DLLs in the two stats PRs in obs-studio. (I plan to overhaul them pending mockups from George and ui/ux discussions.) Qt Charts fails to load without them. I'm trying to do without the openGL deps following Pat's pointers. I thus passed -DFEATURE_opengl:BOOL=OFF But QtGui fails to compile on windows with errors in qwindowswindow.cpp (line 2310):

use of undefined type QOpenGLContext

And another similar error ...

pkviet avatar Jul 16 '23 15:07 pkviet

Update With Pat's help I managed to compile without any openGL dependence.

pkviet avatar Jul 19 '23 23:07 pkviet

Update With Pat's help I managed to compile without any openGL dependence.

Update With the update from 6.4.3 to 6.5.2, QtMultimedia fails to build when one disables opengl. So we're back to square one. Not sure why we would need QtMultimedia btw.

pkviet avatar Sep 01 '23 10:09 pkviet

Update With Pat's help I managed to compile without any openGL dependence.

Update With the update from 6.4.3 to 6.5.2, QtMultimedia fails to build when one disables opengl. So we're back to square one. Not sure why we would need QtMultimedia btw.

QtMultimedia was to be used for accessibility features to play sound effects on certain actions. It wasn't implemented initially because it required Windows Media features that were not installed by default on Windows N systems, and earlier versions of Qt and FFmpeg would crash if they tried to load the necessary libraries. We pushed for changes in those implementations to safely attempt to load the libraries and just never circled back around to implementing the feature on our end.

RytoEX avatar Sep 01 '23 14:09 RytoEX