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.

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

Update dependencies #239

Merged
merged 6 commits into from
Feb 20, 2024
Merged

Update dependencies #239

merged 6 commits into from
Feb 20, 2024

Conversation

RytoEX
Copy link
Member

@RytoEX RytoEX commented Feb 17, 2024

Description

Update dependencies that have patch updates available. We are not updating minor/major versions at this time.

  • deps.ffmpeg: Update zlib to 1.3.1
  • deps.ffmpeg: Update aom to 3.8.1
  • deps.ffmpeg: Update mbedTLS to 3.4.1
  • deps.qt: Update Qt6 to 6.6.2 for Windows
  • deps.qt: Update Qt6 to 6.6.2 for macOS

The update from libpng 1.6.40 to 1.6.42 has been left out because it would require adjustments to our patch. I would rather us spend time and effort getting that upstreamed.

Motivation and Context

Patch updates should be safe for us to adopt.

How Has This Been Tested?

I haven't tested these beyond checking that the deps build on my fork. I haven't tested building/running OBS with these yet. Testing appreciated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Tweak (non-breaking change to improve existing functionality)

Checklist:

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

@RytoEX RytoEX self-assigned this Feb 17, 2024
@gxalpha
Copy link
Member

gxalpha commented Feb 17, 2024

What are your thoughts on reverting https://codereview.qt-project.org/c/qt/qtbase/+/525424 for macOS (see https://bugreports.qt.io/browse/QTBUG-121351)?

@RytoEX
Copy link
Member Author

RytoEX commented Feb 17, 2024

What are your thoughts on reverting https://codereview.qt-project.org/c/qt/qtbase/+/525424 for macOS (see https://bugreports.qt.io/browse/QTBUG-121351)?

I would want a bit more context.

@gxalpha
Copy link
Member

gxalpha commented Feb 18, 2024

In Qt 6.5, Qt added the native dialogs for QMessageBox, which made use of the native OS API's to display the message box. Imo, this looks far superior to the non-native version.
Then, at QTBUG-120054 someone complained that stylesheets no longer work (which makes sense, it's a native OS dialog after all). Unfortunately, that report made Qt change the behavior between 6.6.1 and 6.6.2 (and future versions) so that when a stylesheet is in use, the dialog will show the non-native version (this is the Qt PR I linked above). Unfortunately, it's now no longer possible at all to get a native dialog if a stylesheet is in use.
I filed QTBUG-121351 a while back to complain about this, but unfortunately that does not yet have any replies by Qt on how this could be handled in the future, if at all.

In short, this patch update will make us lose native dialogs unless the commit is reverted (qt/qtbase@1b71e2d in dev or qt/qtbase@04dda6c in 6.6).

@RytoEX
Copy link
Member Author

RytoEX commented Feb 18, 2024

And we have confirmed that this actually happens with Qt 6.6.2 and OBS? Do we have a simple reproducer for the QTBUG that isn't OBS itself?

@gxalpha
Copy link
Member

gxalpha commented Feb 18, 2024

And we have confirmed that this actually happens with Qt 6.6.2 and OBS?

I have confirmed that this actually happens with OBS, yes. I also now reconfirmed with the artefact of this PR.

Do we have a simple reproducer for the QTBUG that isn't OBS itself?

Seeing that I did not attach it to the QTBUG I don't think it exists, but it should be trivial to create one if required.

It did just cross my mind that in theory, we could

widget->setAttribute(Qt::WA_StyleSheet, false);

on a QMessageBox on the OBS side before displaying it. This would be quite the hack and have to be done everywhere QMessageBox is used, but should be possible if you would prefer it to a Qt patch/revert.

@RytoEX
Copy link
Member Author

RytoEX commented Feb 18, 2024

And we have confirmed that this actually happens with Qt 6.6.2 and OBS?

I have confirmed that this actually happens with OBS, yes. I also now reconfirmed with the artefact of this PR.

Do we have a simple reproducer for the QTBUG that isn't OBS itself?

Seeing that I did not attach it to the QTBUG I don't think it exists, but it should be trivial to create one if required.

It did just cross my mind that in theory, we could

widget->setAttribute(Qt::WA_StyleSheet, false);

on a QMessageBox on the OBS side before displaying it. This would be quite the hack and have to be done everywhere QMessageBox is used, but should be possible if you would prefer it to a Qt patch/revert.

I'll have to mull over which I prefer.

On the QTBUG side, I find they're more apt to address bugs for which there is a trivial reproducer attached so they can quickly observe the issue.

Revert qtbase 1b71e2d894c2be7052518cdcb96020c9950e2dc7:
QMessageBox: don't use a native dialog if a style sheet is active

The commit in question causes us to lose native dialogs on macOS. We can
restore them by reverting the above commit. This is hopefully a
temporary measure until either this is addressed in Qt itself or we get
guidance on the correct way to deal with this.
@RytoEX RytoEX merged commit e81a04e into obsproject:master Feb 20, 2024
21 checks passed
@RytoEX RytoEX deleted the update-deps branch February 20, 2024 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants