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

Some fixes/enhancements for #3786 #3991

Merged
merged 8 commits into from
Nov 24, 2017
Merged

Conversation

PhysSong
Copy link
Member

@PhysSong PhysSong commented Nov 20, 2017

TODO list:

  • Make it build with Qt4(it doesn't mean this is fully compatible with Qt4!)
  • Restrict "qt" embedding to Qt >= 5.1(QWidget::CreateWindowContainer was introduced in Qt 5.1)
  • Add Win32 API embedding
  • Move SC_CLOSE handling code into messageWndProc(as suggested by @DomClark)
  • Add a fallback logic for unavailable VST embedding methods
  • Provide a better way for determining available embedding methods, especially on Qt4

There are still some incomplete logics and weird style of code, and it might be buggy for some system configurations. Any reviews and suggestions are appreciated. 😄
You may checkout the test version here: https://github.com/PhysSong/lmms/releases/tag/v1.2.0-rc49

@zonkmachine
Copy link
Member

Some issues that aren't present with the latest AppImage from @tresf

lmms-1.2.0-rc4.241-linux-x86_64.AppImage
Version 1.2.0-rc4.96 (Linux/x86_64, Qt 5.2.1, GCC 4.8.4)

lmms-1.2.0-rc4.72-linux-x86_64.AppImage
Version 1.2.0-rc4.72 (Linux/x86_64, Qt 5.5.1, GCC 7.2.0)
wine-1.6.2

Vestige, GigPlayer and ZynAddSubFX are missing. I don't know if any of them are supposed to be disabled for AppImage.

I get a crash when opening settings.

ALSA lib pcm_dsnoop.c:618:(snd_pcm_dsnoop_open) unable to open slave
ALSA lib pcm_dmix.c:1022:(snd_pcm_dmix_open) unable to open slave
ALSA lib pcm.c:2239:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.rear
ALSA lib pcm.c:2239:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.center_lfe
ALSA lib pcm.c:2239:(snd_pcm_open_noupdate) Unknown PCM cards.pcm.side
bt_audio_service_open: connect() failed: Connection refused (111)
bt_audio_service_open: connect() failed: Connection refused (111)
bt_audio_service_open: connect() failed: Connection refused (111)
bt_audio_service_open: connect() failed: Connection refused (111)
ALSA lib pcm_dmix.c:1022:(snd_pcm_dmix_open) unable to open slave
Cannot connect to server socket err = No such file or directory
Cannot connect to server request channel
jack server is not running or cannot be started
libgcc_s.so.1 must be installed for pthread_cancel to work
/tmp/.mount_lmms-1JxxZw2/AppRun: line 9: 19617 Aborted (core dumped) 
   QT_X11_NO_NATIVE_MENUBAR=1 QT_AUTO_SCREEN_SCALE_FACTOR=1 $DIR/usr/bin/lmms.real "$@"

@tresf
Copy link
Member

tresf commented Nov 20, 2017

Off-topic... but just an FYI to people reading this thread... Qt4 AppImages won't ever be supported by our codebase because linuxdeployqt doesn't have support for them, so it's not going to ever hit our stable-1.2 branch, the final branch which we plan to keep Qt4 working on.

Vestige, GigPlayer and ZynAddSubFX are missing. I don't know if any of them are supposed to be disabled for AppImage.

All three are supported. At a glance, I'm not sure why they'd be different. I build from Ubuntu 14.04 to simulate Travis-CI but my latest AppImage may have been built in a rush without calling qt58-env.sh which would explain the difference in Qt versions.

@PhysSong
Copy link
Member Author

PhysSong commented Nov 20, 2017

libgcc_s.so.1 must be installed for pthread_cancel to work
@zonkmachine I guess there are some problems with my AppImage. My build environment wasn Could you test it by compilng from source?

@zonkmachine
Copy link
Member

zonkmachine commented Nov 21, 2017

OK. Building this one: https://github.com/PhysSong/lmms/commits/v1.2.0-rc49

Got an ouchie!

/usr/bin/ld: cannot find -lqx11embedcontainer
collect2: error: ld returned 1 exit status

Edit: I'm missing some dependency here.

@PhysSong
Copy link
Member Author

Vestige, GigPlayer and ZynAddSubFX are missing.

They works on my side. I guess some shared libraries are missing(something related to pthread) in my AppImage.

@PhysSong
Copy link
Member Author

  • Provide a better way for determining available embedding methods, especially on Qt4

Qt4 doesn't provide a exact way to determine if an application is running with xcb platform or not, while Qt5 does. It is needed because Wine uses X11 and LMMS uses X11 window ID of VST windows(more precisely, windows created by RemoteVstPlugin bridge process).
If LMMS is running with wayland Qt platform abstraction plugin, no VST embedding methods are available(QWidget::createWindowContainer is also unusable in that situation). There are several other plugins(ex. linuxfb), but they are used very rarely.

@lukas-w
Copy link
Member

lukas-w commented Nov 24, 2017

@PhysSong I pushed some Qt4 compile fixes to your branch, which fixes @zonkmachine's link error. I noticed Qt4 Linux builds are disabled on Travis (and I think it's my fault: bd4a93c), I'll test re-enabling them on a separate branch.

@PhysSong
Copy link
Member Author

@lukas-w Thanks! Qt4 build passed.

@lukas-w
Copy link
Member

lukas-w commented Nov 24, 2017

@PhysSong Great, pushed it to this branch.

@lukas-w
Copy link
Member

lukas-w commented Nov 24, 2017

@PhysSong

Qt4 doesn't provide a exact way to determine if an application is running with xcb platform or not

I don't think we need that because Qt4 only supports X11 on Linux. So if we're running Qt4 on Linux, we know it's using X11. Whether that's in an X11 session or through xwayland doesn't matter.

Copy link
Member

@lukas-w lukas-w left a comment

Choose a reason for hiding this comment

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

Tested and all options work fine on every platform. Tested Windows/Qt5, Linux/Qt5 with both X11 and Wayland (Weston). It shows the correct available embedding options in all situations I tested, so I think the platform logic is fine. Didn't test Windows/Qt4, but I don't see a reason why it shouldn't work, and we're distributing Qt5 builds for Windows anyway.

Thanks for the great work @PhysSong!

@lukas-w lukas-w merged commit e95e379 into LMMS:fix/qt5-vst Nov 24, 2017
@PhysSong PhysSong deleted the fix/qt5-vst branch November 24, 2017 13:41
@PhysSong
Copy link
Member Author

From Qt 4.8 documentation:

Note: QPA is considered an experimental feature in Qt 4.8.

In principle, Qt 4.8 can run with non-xcb platform abstraction plug-in. I think Handling is still better than not handling.

Qt4 doesn't provide a exact way to determine if an application is running with xcb platform or not

The current way works if Qt is configured to use xcb by default, which should be true in most use-cases.

This PR was merged while I was writing this comment. If something goes wrong, we may fix that on the original PR #3786.

@lukas-w
Copy link
Member

lukas-w commented Nov 25, 2017

Note: QPA is considered an experimental feature in Qt 4.8.

The word "experimental" would be enough reason for me not to support it. I know of no QPA plug-in that makes Qt4 run on Wayland natively. I really don't think it's worth investing any time in. Qt4 has been superseded by Qt5 five years ago, and I guess it'll be at least another five years until Wayland has replaced X in major Linux distributions. I see no need to marry the two. Those who want to run LMMS on Wayland and insist on not using xwayland will have to use Qt5.

If something goes wrong, we may fix that on the original PR #3786.

👍

sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
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.

4 participants