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

Changed ControlObjectThreadMain instances to ControlObjectThread where no direct connection is used. #43

Merged
merged 7 commits into from Oct 11, 2013

Conversation

daschuer
Copy link
Member

According to our previous discussion ControlObjectThreadMain ensures that the valueChanged signals are propagated by the Main Thread. This makes a different, when Qt::DirectConnection is used in the using code.

I have checked all occurrences of ControlObjectThreadMain and changed them to ControlObjectThread if no direct connection is used. Most of them does not use any connection. So we save a unnecessary round trip thought the Qt event queue.

If my other refactoring branch #38 is accepted, we can change most (or all) of them to ControlObjectSlave. This will save some direct connected function calls ass well because it is optimized for no connection.

ControlObjectThread if no direct connection is used.
ControlObjectThread where no direct connection is used.
pHeadphones->slotSet(defaultHeadphones);
delete pHeadphones;
ControlObject::set(ConfigKey(getGroup(), "master"), (double)defaultMaster);
ControlObject::set(ConfigKey(getGroup(), "pfl"), (double)defaultHeadphones);
Copy link
Member Author

Choose a reason for hiding this comment

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

saves memory allocation.

@daschuer
Copy link
Member Author

daschuer commented Oct 1, 2013

This patch is starting to rod. I would like to have a second pair of eyes to confirm my changes.
If there are non available I will commit it without a review.
Since this patch touches 54 files, which merges should be done before?

@@ -30,16 +23,16 @@ bool WaveformMarkRange::active() {

bool WaveformMarkRange::enabled() {
// Default to enabled if there is no enabled control.
return m_markEnabledControl == NULL ||
Copy link
Member

Choose a reason for hiding this comment

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

If setup() is not called yet, this will segfault. Shouldn't the NULL check stay (for enabled(), start() and end()) as a safety?

@rryan
Copy link
Member

rryan commented Oct 11, 2013

LGTM other than my comment about WaveformMarkRange. Thank you!

Conflicts:
	src/dlgprefcontrols.cpp
	src/dlgprefrecord.cpp
	src/dlgprefrecord.h
	src/widget/wnumberpos.h
@daschuer
Copy link
Member Author

Thank you for review!

daschuer added a commit that referenced this pull request Oct 11, 2013
Changed ControlObjectThreadMain instances to ControlObjectThread where no direct connection is used.
@daschuer daschuer merged commit 955e5a4 into mixxxdj:master Oct 11, 2013
@daschuer daschuer deleted the atomic-co3 branch December 6, 2014 23:57
@daschuer daschuer mentioned this pull request Dec 28, 2014
@daschuer daschuer mentioned this pull request Jun 11, 2018
11 tasks
ronso0 referenced this pull request in ronso0/mixxx Nov 6, 2019
implement Aux,master in skins, hide unconfigured Mic units
ronso0 referenced this pull request in ronso0/mixxx May 14, 2020
put Cue Shift buttons below CurPos button
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.

None yet

2 participants