Skip to content

Revert "Enable Qt message loop on Linux"#472

Merged
RytoEX merged 1 commit into
obsproject:masterfrom
tytan652:revert_enable_qt_loop_on_linux
Feb 28, 2025
Merged

Revert "Enable Qt message loop on Linux"#472
RytoEX merged 1 commit into
obsproject:masterfrom
tytan652:revert_enable_qt_loop_on_linux

Conversation

@tytan652

@tytan652 tytan652 commented Jan 31, 2025

Copy link
Copy Markdown
Contributor

Description

Closes obsproject/obs-studio#11485

This reverts commit 98d94a4.

Motivation and Context

Performance issues were met by multiple users.

So back to use_gtk=false builds.

How Has This Been Tested?

Used use_gtk=false for testing (even tested with #453) everything seem to work properly.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

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 requested a review from WizardCM January 31, 2025 18:51
@RytoEX

RytoEX commented Jan 31, 2025

Copy link
Copy Markdown
Member

cc @kkartaltepe @GeorgesStavracas

@kkartaltepe

Copy link
Copy Markdown
Contributor

Everything looks good with a new build of cef and this applied.

@WizardCM WizardCM added the kind/bug Categorizes issue or PR as related to a bug. label Feb 2, 2025
@RytoEX RytoEX self-assigned this Feb 12, 2025

@RytoEX RytoEX left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine.

Note: this must be paired with new CEF builds that use use_gtk=false.

@RytoEX RytoEX merged commit 8223215 into obsproject:master Feb 28, 2025
@tytan652 tytan652 deleted the revert_enable_qt_loop_on_linux branch March 8, 2025 10:47
@hoshinolina

Copy link
Copy Markdown
Contributor

I'm curious, what's the issue when the CEF build has use_gtk=true with this revert/change?

@tytan652

tytan652 commented Jul 2, 2025

Copy link
Copy Markdown
Contributor Author

Qt by default relies on Glib for its event loop, which completely goes wrong with the GTK inside CEF (e.g. obsproject/obs-studio#7146).

We also normally should not need use_gtk or use_qt being enabled in our usecase, since both can lead to side effects.

@hoshinolina

Copy link
Copy Markdown
Contributor

@tytan652 Thanks! I hadn't noticed that since browser panels don't work on Wayland anyway. Is the GN define list used for OBS CEF builds viewable somewhere?

@tytan652

tytan652 commented Jul 4, 2025

Copy link
Copy Markdown
Contributor Author

I can give it to you, note that we enable H264 decoding in ours.

For the latest CEF 6533, I used GN_DEFINES="is_official_build=true use_sysroot=true use_partition_alloc_as_malloc=false symbol_level=1 is_cfi=false use_thin_lto=false proprietary_codecs=true ffmpeg_branding=Chrome chrome_pgo_phase=0 use_gtk=false" and note that we use the sysroot.

@hoshinolina

Copy link
Copy Markdown
Contributor

Thanks! Looks like the only relevant override there is use_gtk=false, everything else is already handled similarly at some other level in CEF/Fedora packaging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Abnormally high CPU usage for OBS 31.0.0-betaX versions caused by obs-browser

5 participants