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

Song editor: can't set or see loop points #4199

Closed
tukkek opened this Issue Feb 28, 2018 · 13 comments

Comments

Projects
None yet
5 participants
@tukkek

tukkek commented Feb 28, 2018

I'm trying out the latest beta from your download page (AppImage) and I can't see or set loops on the Song Editor. Frankly, I can't see how to write a song without looping so this is pretty much a deal breaker for me unless I find a work-around.

I will add a screenshot next showing that a lot of what used to be UI is now just black. Not sure if that's a feature or a bug for 1.2 or how I feel about it if it is, but that's not a problem - while looping is a must-have.

If I enable loops, even though it doesn't show me the marker, it will still loop the first 16 beats (4 bars?), which I assume is just the default starting value for the marker. Trying to change the marker's position, even though it's invisible, like I would on the latest stable release from Debian, does nothing to change this behavior either (ie. doesn't "move" the invisible marker).

@tukkek

This comment has been minimized.

tukkek commented Feb 28, 2018

screenshot_20180228_142527

@tukkek

This comment has been minimized.

tukkek commented Feb 28, 2018

So I restarted the program and it's now drawing everything properly (no more black areas) and the loop feature is working as intended (looks very nice on 1.2 too!). Apparently this is a issue that only arises the first time you open the AppImage?

I'll leave this open for the devs to determine if it's worth looking into. I would recommend so because the first try is all that a user may need to think this is a buggy package and give up on LMMS.

@Anonymouqs Anonymouqs added the bug label Feb 28, 2018

@tresf tresf removed the bug label Feb 28, 2018

@tresf

This comment has been minimized.

Member

tresf commented Feb 28, 2018

Duplicate of #738 #1011 #572 #497, #1109, #3565, #1187, #1412. This was supposed to be fixed in #1442, I'm not sure why that fix isn't executing.

@tresf

This comment has been minimized.

Member

tresf commented Feb 28, 2018

@tukkek please do the following:

  • If you have an old version of LMMS installed, please load it up and remove the "theme" setting in the preferences and relaunch again to get it to reset to default.
  • Try the AppImage again

If you can reliably reproduce the glitch, send us the .lmmsrc.xml file and we'll see why the theme logic is breaking. (or alternately give us steps to reproduce)

If you cannot reliably reproduce the glitch, we'll have to assume that something non-standard happened and we'll mark as duplicates of the rest.

This may be an order of operations bug as well where the theme folder isn't updated until after the UI starts loading. It may be a regression.

@Umcaruje

This comment has been minimized.

Member

Umcaruje commented Mar 1, 2018

This may be an order of operations bug as well where the theme folder isn't updated until after the UI starts loading.

I think this is probably the problem, because next time you open up LMMS the theme folder is properly set.

You can reproduce this by installing LMMS from a repository, and then running the AppImage. First time around the theme is broken, when you close and open the appimage again, the theme is properly set.

@tresf

This comment has been minimized.

Member

tresf commented Mar 1, 2018

@Umcaruje thanks for confirmation. I need help here. @lukas-w is this just a matter of calling ConfigManager::inst()->upgrade() before we use it? If that's the case, shouldn't we upgrade in the constructor? There's some order of operations issue happening and it will affect all settings upgrades until it's corrected, I'm just not sure where to start.

@tresf

This comment has been minimized.

Member

tresf commented Mar 1, 2018

Ok, found it... src/core/ConfigManager.cpp#L59, we're initializing m_version and it's never making it inside any of the upgrade() steps. I'll force it to something bogus on init to see how well it works. Nope, false alarm. Still looking.

@tresf

This comment has been minimized.

Member

tresf commented Mar 1, 2018

Nope, that was wrong... I think I found it...

	QStringList searchPaths;
	if(! qgetenv("LMMS_THEME_PATH").isNull())
		searchPaths << qgetenv("LMMS_THEME_PATH");
	searchPaths << artworkDir() << defaultArtworkDir();
	QDir::setSearchPaths( "resources", searchPaths);

	// Create any missing subdirectories in the working dir, but only if the working dir exists
	if( hasWorkingDir() )
	{
		createWorkingDir();
	}

	upgrade();

We're calling and thus populating our searchPaths before running upgrade. It should be the other way around. I threw in a Q_ASSERT and now I clearly see that artworkDir() gets called BEFORE upgrade() and I can't see it getting called again afterward.

I'm not entirely sure how moving upgrade() above that will impact the software though. I'll make an attempt, but @lukas-w I'd appreciate a second set of eyes as I believe you rewrote this stuff.

@Anonymouqs

This comment has been minimized.

Contributor

Anonymouqs commented Mar 1, 2018

This glitch also happens in Mac. I was running Stable-1.2 RC5 today and the exact same thing happened when I ran it for the first time.

@tresf

This comment has been minimized.

Member

tresf commented Mar 1, 2018

@Anonymouqs do you run multiple simultaneous versions on Mac?

tresf added a commit to tresf/lmms that referenced this issue Mar 1, 2018

tresf added a commit to tresf/lmms that referenced this issue Mar 1, 2018

@tresf

This comment has been minimized.

Member

tresf commented Mar 1, 2018

PR available at #4206. Testing appreciated.

@zonkmachine

This comment has been minimized.

Member

zonkmachine commented Mar 1, 2018

Tested. That made the trick!

LinuxMint 17.3, Switching between 1.1.3 and 1.2.0 with no gui glitches. Pretty!

@tresf

This comment has been minimized.

Member

tresf commented Mar 2, 2018

Great, thanks for testing. I've taken a second look an the logic which now comes after the upgrade(); call doesn't appear like it will negatively affect any other part of the loading process, so I'll merge this. This fill will be available in 1.2.0-RC6 and higher.

@tresf tresf closed this in #4206 Mar 2, 2018

tresf added a commit that referenced this issue Mar 2, 2018

gi0e5b06 added a commit to gi0e5b06/lmms that referenced this issue Mar 23, 2018

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