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

Switching to Qt5 #2611

Closed
16 tasks done
tresf opened this issue Feb 25, 2016 · 57 comments
Closed
16 tasks done

Switching to Qt5 #2611

tresf opened this issue Feb 25, 2016 · 57 comments

Comments

@tresf
Copy link
Member

tresf commented Feb 25, 2016

Many of our users are already compiling against Qt5, but most of them tend to be on Linux. This topic is related to switching all builds to Qt5 permanently and what's needed prior to that switch.

Screenshot, because everyone loves screenshots...
image

@liushuyu
Copy link
Member

liushuyu commented Mar 3, 2016

Windows: Language dropdown is blank (see #1498 (comment))

Not only on Windows, but also on the other platforms namely Linux and Mac can't select languages

@tresf
Copy link
Member Author

tresf commented Mar 3, 2016

In fact, I saw the binary translation files were generated, but not installed to the installation directory. But if you re-run make install, the files will appear in the correct directory. Strange.

@liushuyu thanks. While looking for the language block you commented on, I noticed it was moved in ee3a998. Tagging @lukas-w.

The language logic has moved quite a bit over the last year including moving from CMakeLists.txt to src/CMakeLists.txt although I feel this may have a better home in data/locale/CMakeList.txt.

It's pretty obvious that order of operations has changed after this move, breaking language generation on first run for all platforms.

@lukas-w
Copy link
Member

lukas-w commented Mar 3, 2016

I feel this may have a better home in data/locale/CMakeList.txt.

Absolutely. Moved in 85011cd. That should fix the generation.

@tresf
Copy link
Member Author

tresf commented Mar 3, 2016

Absolutely. Moved in 85011cd. That should fix the generation.

As it does. 👍 Checking it off here as I'll track the Windows/Travis stuff in #2577 since it's not Qt5 related.

@tresf
Copy link
Member Author

tresf commented Mar 3, 2016

Hmmm... I'm having no luck with the splash screen bug... Qt version is 5.5.1_2

[ ] Mac: Spash window appears above SettingsDialog, MessageDialog

This bug doest NOT seem to occur on Windows.

I tried the following to no avail (clicking it won't raise it).

Solution(s) I tried:

Haven't tried... yuck:

No mention of it in known issues:

Test results:
⛔ didn't work... (GuiApplication.cpp)

    // Show splash screen
    QSplashScreen splashScreen( embed::getIconPixmap( "splash" ) );
+   // Prevent splash from covering settings dialog
+   splashScreen.setWindowFlags( splashScreen.windowFlags() | Qt::WindowStaysOnBottomHint );
    splashScreen.show();

⛔ didn't work... (GuiApplication.cpp)

    // Show splash screen
    QSplashScreen splashScreen( embed::getIconPixmap( "splash" ) );
    splashScreen.show();
+   splashScreen.lower();

⛔ didn't work... (SetupDialog.cpp)

    vlayout->addWidget( settings );
    vlayout->addSpacing( 10 );
    vlayout->addWidget( buttons );
    vlayout->addSpacing( 10 );
    vlayout->addStretch();

+   setWindowFlags(windowFlags() | Qt::WindowStaysOnTopHint);

    show();

+   raise();

@softrabbit
Copy link
Member

Windows: Visual artifacts in MDI window decorations

I'm pretty sure I've seen those "ragged" MDI close buttons before, possibly in an LMMS/Qt5 build on Linux. Could they be a part of a style after all? And IMO they look too consistent for artifatcs.

@tresf
Copy link
Member Author

tresf commented Mar 3, 2016

Could ["ragged" MDI close buttons] be a part of a style after all?

Indeed they are -- tested by moving the themes directory. @Umcaruje any interest in tracking this one down?

@Umcaruje
Copy link
Member

Umcaruje commented Mar 3, 2016

@Umcaruje any interest in tracking this one down?

My life is pretty crowded rn, I can take a look, but not sure when I'll have the time for it (I have some other issues I'm looking at).

Isn't #2516 going to fix this though? We are loading custom pixmaps for the buttons there.

@tresf
Copy link
Member Author

tresf commented Mar 3, 2016

Isn't #2516 going to fix this though? We are loading custom pixmaps for the buttons there.

Well, it would certainly be a moot point if #2516 were to be implemented, but I'm not placing any bets on a hypothetical, and this would give us a better understanding of things, anyhow.

@Umcaruje
Copy link
Member

Umcaruje commented Mar 3, 2016

Well, it would certainly be a moot point if #2516 were to be implemented, but I'm not placing any bets on a hypothetical, and this would give us a better understanding of things, anyhow.

What if I started working on #2516 until its mergeable? ;) (though idk how that would be organised)

@tresf
Copy link
Member Author

tresf commented Mar 3, 2016

What if I started working on #2516 until its mergeable? ;) (though idk how that would be organised)

Well, actually, that may be our only option... Here's Windows with the theme disabled... (still happens)

image

I'll take a second look at the code, but this may be a not-our-bug-type scenario after-all...

@tresf
Copy link
Member Author

tresf commented Mar 3, 2016

Adding new item to list...

[ ] Windows: Channel activity indicator on by default

tresf added a commit that referenced this issue Mar 4, 2016
@tresf
Copy link
Member Author

tresf commented Mar 4, 2016

Update:

Closed:

Critical/Open:

  • Channel activity indicator on by default (screenshot in description above)

Cosmetic:

  • White line on edge of all QWidgets (screenshot in description above)
  • MDI window controls are incorrectly colored (screenshot in description above)
  • Drop-down menus are "crammed" (screenshots below)

@Umcaruje can you tell me if this is the intended look of the dropdowns? I ask because they seem a bit crammed -- specifically the lack of padding between the icon and the menu. I'm tagging you per #2567. If they look fine, we'll remove it from the checklist.

Mac Windows
image image

@Umcaruje
Copy link
Member

Umcaruje commented Mar 4, 2016

Well, on linux, I actually have no icons in the dropdown menu, so I don't know how they looked.

I increased the padding accordingly in #2567, but I'm not sure how the icons behave. They should be moved 3 px to the left I guess, though Qt should render them centered, and not aligned to left.

If it can't be fixed in code, we can revert the margins back.

@Umcaruje
Copy link
Member

Umcaruje commented Mar 4, 2016

Fixed it via d098a39

screenshot from 2016-03-04 22 13 58

@mikobuntu
Copy link
Contributor

@Umcaruje Refering to your comment about no drop-down icons, this is a known issue... iirc I was talking to tobydox several years ago about this and if my memory server me right it was an issue with the actual DR.
I wonder if there is a method to override this behaviour? Thanks

@Umcaruje
Copy link
Member

Umcaruje commented Mar 4, 2016

@mikobuntu well, the problem is gone for me when I compiled using qt5, so I guess when we switch to it, it won't matter.

But no, I couldn't fix the issue, it seems to be a qt4 bug.

@Umcaruje Umcaruje added the meta label Mar 4, 2016
@Umcaruje
Copy link
Member

Umcaruje commented Mar 4, 2016

@tresf I rebased #2516 and the White line on edge of all QWidgets is also gonna be superseded by it:
screenshot from 2016-03-04 23 24 22

@tresf
Copy link
Member Author

tresf commented Mar 4, 2016

Fixed [menu icons] it via d098a39

👍

White line on edge of all QWidgets [...] superseded by #2516

👍 👍

Do you guys get the LED indicator bug too on Linux? I don't have a ready Qt5 Linux environment to test on.

@softrabbit
Copy link
Member

Do you guys get the LED indicator bug too on Linux? I don't have a ready Qt5 Linux environment to test on.

Yes, it's present.
LMMS 1.1.90-g2e2abdf
(Linux x86_64, Qt 5.4.2, GCC 5.2.1 20151010)

@tresf
Copy link
Member Author

tresf commented Mar 6, 2016

@softrabbit thanks for confirming.

Is there a chance this is just a bad initiation? Don't know the CSS side of things that well, but seems like this may be intended to be Inactive rather than Active. Pinging @Umcaruje https://github.com/LMMS/lmms/blob/master/src/tracks/InstrumentTrack.cpp#L916:L920

@tresf
Copy link
Member Author

tresf commented Mar 6, 2016

Meh... nevermind that statement... that's more in regards to the QWidget being active or inactive from a user-interaction perspective. I'm pretty sure Active is what we want, so this must be something else.

@tresf
Copy link
Member Author

tresf commented Mar 6, 2016

Ok, on second look, this is most likely due to the way Qt5 is handling the timer...

https://github.com/LMMS/lmms/blob/master/src/gui/widgets/FadeButton.cpp#L86:L100

When the FadeButton is initially created, this statement is most likely coming back as true. m_stateTimer is QTime. According to the Qt5 docs, we should be using m_stateTimer.isNull() as a check before doing a time comparison. Qt4.8 states this function exists there as well, so this shouldn't break backwards compat. 👍

@lukas-w
Copy link
Member

lukas-w commented Mar 6, 2016

we should be using m_stateTimer.isNull() as a check before doing a time comparison

Indeed, that fixes it. Fixed via 8768769.

@liushuyu
Copy link
Member

liushuyu commented May 15, 2016 via email

@tresf
Copy link
Member Author

tresf commented May 15, 2016

Although the checklist would suggest that, I feel we should make our build environment do this by default to consider this task closed.

@Umcaruje
Copy link
Member

I feel we should make our build environment do this by default to consider this task closed.

Hmm, you're right. Forgot about that one.

@Umcaruje Umcaruje reopened this May 15, 2016
@tresf
Copy link
Member Author

tresf commented May 16, 2016

Added new item to the checklist:

Goes without saying but this bug is a deal breaker on Windows if not resolved. It will spike a 100% usage on a CPU core if running Qt5. Discussion specific to the bug should remain in #2767.

Teuthis pushed a commit to Teuthis/lmms that referenced this issue May 18, 2016
@tresf
Copy link
Member Author

tresf commented Jul 6, 2016

Added new items to the checklist:

Yet another deal breaker for Qt5. Help/workarounds, etc are appreciated. 👍

@tresf
Copy link
Member Author

tresf commented Aug 31, 2016

Since Windows VSTs seem to be in usable shape and Mac does not yet support VSTs, I feel we are ready to switch to Qt5 for Mac and Windows for our next release.

I'm marking off the VST bug and adding a new task, "Remove Mac/Windows Qt4 builds from Travis-CI".

@simonvanderveldt
Copy link
Contributor

simonvanderveldt commented Sep 20, 2016

Will building with Qt4 as an option be removed?
(I'm on Linux in case it's relevant)

@tresf
Copy link
Member Author

tresf commented Sep 20, 2016

Will building with Qt4 as an option be removed?

No, not right away as VST GUIs are still horribly broken with Qt5 on Linux. #2855.

We do have plans to shift Windows and Mac builds to Qt5 since they have Qt5 bundled --there is no downside. Note, we may eventually offer Qt5 bundles for Linux as well per #2932, assuming VSTs are sorted.

@simonvanderveldt
Copy link
Contributor

👍

@tresf
Copy link
Member Author

tresf commented Oct 8, 2016

New issue... #2890 Qt5 prevents Mac ⌘+Drag copy as well as Mac ⌘+Drag to Automation editor.

Although Copy/Paste has a right-click alternative, no such shortcut exists for linking to an automation pattern.

This may be unrelated to Qt5 (could be a code change) but my instinct is that it's directly Qt5 related.

Edit: It seems to be related to the XCode SDK used for building. For now, ALT + Option works just fine.

@musikBear
Copy link

That "Drive not ready" thing?
drivenotready
-that appears when lmms is loaded. I suspect that it is an error caused by the absence of a disk in Drive-B, or iow. This popup will occur, if the user do not have a SSD in drive-B, and the compiling is made on a system with a drive-B.
Could that be correct?

@tresf
Copy link
Member Author

tresf commented Dec 4, 2017

Could that be correct?

Nope.

@tresf
Copy link
Member Author

tresf commented Dec 23, 2017

Closed via #4041. The last remaining item was switching Travis to Qt5 and that is completed.

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

No branches or pull requests

10 participants