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

Don't immediately jump when loopOut is pressed in quantized mode #2260

Closed
wants to merge 1 commit into from
Closed

Don't immediately jump when loopOut is pressed in quantized mode #2260

wants to merge 1 commit into from

Conversation

goddisignz
Copy link
Contributor

@goddisignz goddisignz commented Sep 2, 2019

Don't immediately jump to loop start when loopOut is pressed in quantized mode.
Same holds true for loopIn in reverse mode

Fixes https://bugs.launchpad.net/bugs/1837077

@daschuer
Copy link
Member

daschuer commented Sep 3, 2019

Thank you for taking care. I think this fix is also relevant for 2.2.
Does this patch work in the 2.2 branch in the same way?

@daschuer
Copy link
Member

daschuer commented Sep 3, 2019

Your comments can be improved by removing the c++ explaining parts and add some more background info. I will give an example inline.

@@ -354,19 +354,29 @@ double LoopingControl::nextTrigger(bool reverse,

LoopSamples loopSamples = m_loopSamples.getValue();

// adjust loop in was toggled (slotLoopIn or slotLoopOut)
Copy link
Member

Choose a reason for hiding this comment

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

//m_bAdjustingLoopIn is true while the LoopIn button is pressed, (slotLoopIn)


// adjust loop in was true and is now false and is only relevant in reverse mode
// in non-quantized mode we jump immediately to the end of the loop while in quantized mode we wait until the loop start is reached
if (reverse && !m_bAdjustingLoopIn && !m_pQuantizeEnabled->toBool()) {
Copy link
Member

Choose a reason for hiding this comment

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

// If the loop in button was released in reverse mode, we need to jump to loop end to not fall out of the just set loop. This must not happen in qantized mode. Here we need to jump to a qantized position, which is handeled in ....

@goddisignz
Copy link
Contributor Author

goddisignz commented Sep 5, 2019

I think this fix should work on 2.2 as well (commit can be cherry-picked). But I can not build 2.2 currently.
After fixing a tab/space indentation in the SConscript I get the following error:

scons: *** [lin64_build/controllers/dlgprefcontrollers.o] Error 1
In file included from src/controllers/controller.h:15,
                 from src/controllers/midi/midicontroller.h:16,
                 from src/controllers/midi/midioutputhandler.cpp:12:
src/controllers/controllerengine.h: In member function ‘ScriptConnection& ScriptConnection::operator=(const ScriptConnection&)’:
src/controllers/controllerengine.h:31:7: warning: implicitly-declared ‘ConfigKey& ConfigKey::operator=(const ConfigKey&)’ is deprecated [-Wdeprecated-copy]
   31 | class ScriptConnection {
      |       ^~~~~~~~~~~~~~~~
In file included from src/preferences/usersettings.h:7,
                 from src/controllers/midi/midimessage.h:8,
                 from src/control/controlbehavior.h:7,
                 from src/control/control.h:9,
                 from src/control/controlproxy.h:8,
                 from src/controllers/midi/midioutputhandler.h:14,
                 from src/controllers/midi/midioutputhandler.cpp:11:
src/preferences/configobject.h:19:5: note: because ‘ConfigKey’ has user-provided ‘ConfigKey::ConfigKey(const ConfigKey&)’
   19 |     ConfigKey(const ConfigKey& key);
      |     ^~~~~~~~~
In file included from src/controllers/controller.h:15,
                 from src/controllers/midi/midicontroller.h:16,
                 from src/controllers/midi/midioutputhandler.cpp:12:
src/controllers/controllerengine.h: In constructor ‘ScriptConnectionInvokableWrapper::ScriptConnectionInvokableWrapper(ScriptConnection)’:
src/controllers/controllerengine.h:62:30: note: synthesized method ‘ScriptConnection& ScriptConnection::operator=(const ScriptConnection&)’ first required here
   62 |         m_scriptConnection = conn;
      |                              ^~~~
scons: building terminated because of errors.

Seems like a missing include directory because the files exist under lin64_build/src but I have no knowledge of scons to fix this.
Any advice?

@daschuer
Copy link
Member

daschuer commented Sep 5, 2019

Sometimes scons tricks itself when switching branches. You can try to call scons --clean or just delete the lin64_build folder an the .sconsign.dblite or a fresh checkout.

In this case I think you there is no nee to bother with the scons issues.
Just cherry pick this commit into the 2.2 branch and create a new PR.
We can rely on Appveyor and Jenkins to see if it is builds and I can finally do a manual tests here.

@goddisignz
Copy link
Contributor Author

OK, cleaning didn't help. Did not try a new checkout. I will create a new PR soon.

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

Successfully merging this pull request may close these issues.

2 participants