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

add option to disable OpenGL & disable it by default for macOS #3403

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -852,6 +852,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/waveform/widgets/rgbwaveformwidget.cpp
src/waveform/widgets/softwarewaveformwidget.cpp
src/waveform/widgets/waveformwidgetabstract.cpp
src/waveform/widgets/waveformwidgettype.cpp
src/widget/controlwidgetconnection.cpp
src/widget/hexspinbox.cpp
src/widget/paintable.cpp
Expand Down
2 changes: 1 addition & 1 deletion res/skins/Deere (64 Samplers)/skin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<attribute persist="true" config_key="[Skin],show_8_hotcues">1</attribute>
<attribute persist="true" config_key="[Skin],show_intro_outro_cues">1</attribute>
<attribute persist="true" config_key="[Skin],show_coverart">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">0</attribute>
Copy link
Member

Choose a reason for hiding this comment

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

What is the reasoning for this change?
If you think it is an issue to show an empty Spinny in case of no OpenGL, we should always hide it in this case.
But that is only a one time issue for Mac users, while new Windows user will have hard times to discover the spinnies.

Copy link
Contributor Author

@Be-ing Be-ing Dec 5, 2020

Choose a reason for hiding this comment

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

If you think it is an issue to show an empty Spinny in case of no OpenGL, we should always hide it in this case.

I disagree. Then we would need to explain somewhere in the GUI that spinnies will be disabled without OpenGL. Users will be confused if spinnies disappear when updating Mixxx without any explanation and the toggles in the skin settings do not work. With this solution, users installing Mixxx for the first time will probably not notice anything. If they enable the spinnies in the skin settings, they will immediately see why they are not working rather than confusingly having no feedback.

new Windows user will have hard times to discover the spinnies.

I don't mind that.

<attribute persist="true" config_key="[Skin],show_starrating">1</attribute>
<attribute persist="true" config_key="[Deere],show_track_info">1</attribute>
<attribute persist="true" config_key="[Deere],show_bpm_info">1</attribute>
Expand Down
2 changes: 1 addition & 1 deletion res/skins/Deere/skin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<attribute persist="true" config_key="[Skin],show_8_hotcues">1</attribute>
<attribute persist="true" config_key="[Skin],show_intro_outro_cues">1</attribute>
<attribute persist="true" config_key="[Skin],show_coverart">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">0</attribute>
<attribute persist="true" config_key="[Skin],show_starrating">1</attribute>
<attribute persist="true" config_key="[Deere],show_track_info">1</attribute>
<attribute persist="true" config_key="[Deere],show_bpm_info">1</attribute>
Expand Down
2 changes: 1 addition & 1 deletion res/skins/LateNight/skin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<attribute persist="true" config_key="[Skin],show_hotcues">1</attribute>
<attribute persist="true" config_key="[Skin],show_8_hotcues">1</attribute>
<attribute persist="true" config_key="[Skin],show_intro_outro_cues">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">0</attribute>
<attribute persist="true" config_key="[Skin],show_coverart">1</attribute>
<attribute persist="true" config_key="[Skin],show_big_spinny_coverart">0</attribute>
<!-- ToDo: deck-independent vinyl controls? -->
Expand Down
4 changes: 2 additions & 2 deletions res/skins/Shade/skin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@
<attribute config_key="[Master],num_samplers">8</attribute>
<attribute config_key="[Master],num_preview_decks">1</attribute>
<!--Optionally, make elements visible on skin load-->
<attribute persist="true" config_key="[Spinny1],show_spinny">1</attribute>
<attribute persist="true" config_key="[Spinny2],show_spinny">1</attribute>
<attribute persist="true" config_key="[Spinny1],show_spinny">0</attribute>
<attribute persist="true" config_key="[Spinny2],show_spinny">0</attribute>
<attribute persist="true" config_key="[Samplers],show_samplers">0</attribute>
<attribute persist="true" config_key="[Skin],sampler_row_1_expanded">0</attribute>
<attribute persist="true" config_key="[Skin],sampler_row_2_expanded">0</attribute>
Expand Down
2 changes: 1 addition & 1 deletion res/skins/Tango (64 Samplers)/skin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<attribute persist="true" config_key="[Skin],show_intro_outro_cues">1</attribute>
<attribute persist="true" config_key="[Skin],show_8_hotcues">1</attribute>
<attribute persist="true" config_key="[Skin],show_coverart">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">0</attribute>
<attribute persist="true" config_key="[Skin],show_big_spinny_coverart">1</attribute>
<attribute persist="true" config_key="[Tango],vinylControlsDeck1">0</attribute>
<attribute persist="true" config_key="[Tango],vinylControlsDeck2">0</attribute>
Expand Down
2 changes: 1 addition & 1 deletion res/skins/Tango/skin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<attribute persist="true" config_key="[Skin],show_intro_outro_cues">1</attribute>
<attribute persist="true" config_key="[Skin],show_8_hotcues">1</attribute>
<attribute persist="true" config_key="[Skin],show_coverart">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">1</attribute>
<attribute persist="true" config_key="[Skin],show_spinnies">0</attribute>
<attribute persist="true" config_key="[Skin],show_big_spinny_coverart">1</attribute>
<attribute persist="true" config_key="[Tango],vinylControlsDeck1">0</attribute>
<attribute persist="true" config_key="[Tango],vinylControlsDeck2">0</attribute>
Expand Down
7 changes: 5 additions & 2 deletions src/mixxx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1559,8 +1559,11 @@ void MixxxMainWindow::checkDirectRendering() {

UserSettingsPointer pConfig = m_pSettingsManager->settings();

if (!factory->isOpenGlAvailable() && !factory->isOpenGlesAvailable() &&
pConfig->getValueString(ConfigKey("[Direct Rendering]", "Warned")) != QString("yes")) {
bool openGlEnabled = pConfig->getValue(ConfigKey("[Waveform]", "OpenGlEnabled"),
WaveformWidgetFactory::defaultOpenGlEnabled);

if (openGlEnabled && !factory->isOpenGlAvailable() && !factory->isOpenGlesAvailable() &&
pConfig->getValueString(ConfigKey("[Direct Rendering]", "Warned")) != QString("yes")) {
QMessageBox::warning(
0, tr("OpenGL Direct Rendering"),
tr("Direct rendering is not enabled on your machine.<br><br>"
Expand Down
108 changes: 78 additions & 30 deletions src/preferences/dialog/dlgprefwaveform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,18 @@ DlgPrefWaveform::DlgPrefWaveform(QWidget* pParent, MixxxMainWindow* pMixxx,
waveformOverviewComboBox->addItem(tr("HSV")); // "1"
waveformOverviewComboBox->addItem(tr("RGB")); // "2"

// Populate waveform options.
WaveformWidgetFactory* factory = WaveformWidgetFactory::instance();
QVector<WaveformWidgetAbstractHandle> handles = factory->getAvailableTypes();
for (int i = 0; i < handles.size(); ++i) {
waveformTypeComboBox->addItem(handles[i].getDisplayName(),
handles[i].getType());
if (factory->isOpenGlAvailable() || factory->isOpenGlesAvailable()) {
openGlStatusIcon->setText(factory->getOpenGLVersion());
} else {
openGlStatusIcon->setText(tr("OpenGL not available") + ": " + factory->getOpenGLVersion());
}

connect(openGLEnabledCheckBox,
&QCheckBox::stateChanged,
this,
&DlgPrefWaveform::openGlEnabledChanged);

// Populate zoom options.
for (int i = static_cast<int>(WaveformWidgetRenderer::s_waveformMinZoom);
i <= static_cast<int>(WaveformWidgetRenderer::s_waveformMaxZoom);
Expand Down Expand Up @@ -136,19 +140,19 @@ DlgPrefWaveform::~DlgPrefWaveform() {
}

void DlgPrefWaveform::slotUpdate() {
WaveformWidgetFactory* factory = WaveformWidgetFactory::instance();
m_openGlEnabled = m_pConfig->getValue(ConfigKey("[Waveform]", "OpenGlEnabled"),
WaveformWidgetFactory::defaultOpenGlEnabled);
openGLEnabledCheckBox->setChecked(m_openGlEnabled);

if (factory->isOpenGlAvailable() || factory->isOpenGlesAvailable()) {
openGlStatusIcon->setText(factory->getOpenGLVersion());
} else {
openGlStatusIcon->setText(tr("OpenGL not available") + ": " + factory->getOpenGLVersion());
}
populateWaveformTypeCombobox();

WaveformWidgetType::Type currentType = factory->getType();
int currentIndex = waveformTypeComboBox->findData(currentType);
if (currentIndex != -1 && waveformTypeComboBox->currentIndex() != currentIndex) {
waveformTypeComboBox->setCurrentIndex(currentIndex);
}
WaveformWidgetFactory* factory = WaveformWidgetFactory::instance();

auto waveType = static_cast<WaveformWidgetType::Type>(
m_pConfig->getValue(ConfigKey("[Waveform]", "WaveformType"),
static_cast<int>(factory->chooseWidgetType(m_openGlEnabled))));
waveformTypeComboBox->setCurrentIndex(
waveformTypeComboBox->findData(waveType));

frameRateSpinBox->setValue(factory->getFrameRate());
frameRateSlider->setValue(factory->getFrameRate());
Expand Down Expand Up @@ -194,18 +198,25 @@ void DlgPrefWaveform::slotApply() {
waveformSettings.setWaveformCachingEnabled(enableWaveformCaching->isChecked());
waveformSettings.setWaveformGenerationWithAnalysisEnabled(
enableWaveformGenerationWithAnalysis->isChecked());
}

void DlgPrefWaveform::slotResetToDefaults() {
WaveformWidgetFactory* factory = WaveformWidgetFactory::instance();
bool openGlWasEnabled = m_pConfig->getValue(ConfigKey("[Waveform]", "OpenGlEnabled"),
WaveformWidgetFactory::defaultOpenGlEnabled);

// Get the default we ought to use based on whether the user has OpenGL or
// not.
WaveformWidgetType::Type defaultType = factory->autoChooseWidgetType();
int defaultIndex = waveformTypeComboBox->findData(defaultType);
if (defaultIndex != -1 && waveformTypeComboBox->currentIndex() != defaultIndex) {
waveformTypeComboBox->setCurrentIndex(defaultIndex);
if (m_openGlEnabled != openGlWasEnabled) {
m_pConfig->setValue(ConfigKey("[Waveform]", "OpenGlEnabled"), m_openGlEnabled);
#ifdef __APPLE__
QMessageBox::information(this,
tr("Information"),
tr("Mixxx must be restarted to %1 OpenGL.")
.arg(m_openGlEnabled ? tr("enable") : tr("disable")));
#else
m_pMixxx->rebootMixxxView();
#endif
}
}

void DlgPrefWaveform::slotResetToDefaults() {
openGLEnabledCheckBox->setChecked(WaveformWidgetFactory::defaultOpenGlEnabled);

allVisualGain->setValue(1.0);
lowVisualGain->setValue(1.0);
Expand Down Expand Up @@ -239,6 +250,35 @@ void DlgPrefWaveform::slotResetToDefaults() {
playMarkerPositionSlider->setValue(50);
}

void DlgPrefWaveform::openGlEnabledChanged(bool checked) {
m_openGlEnabled = checked;
populateWaveformTypeCombobox();
pickDefaultWaveformType();
}

void DlgPrefWaveform::populateWaveformTypeCombobox() {
waveformTypeComboBox->blockSignals(true);
waveformTypeComboBox->clear();

WaveformWidgetFactory* factory = WaveformWidgetFactory::instance();
QVector<WaveformWidgetAbstractHandle> handles = factory->getAvailableTypes(m_openGlEnabled);
for (int i = 0; i < handles.size(); ++i) {
waveformTypeComboBox->addItem(handles[i].getDisplayName(),
handles[i].getType());
}

waveformTypeComboBox->blockSignals(false);
}

void DlgPrefWaveform::pickDefaultWaveformType() {
WaveformWidgetFactory* factory = WaveformWidgetFactory::instance();
WaveformWidgetType::Type defaultType = factory->chooseWidgetType(m_openGlEnabled);
int defaultIndex = waveformTypeComboBox->findData(defaultType);
if (defaultIndex != -1 && waveformTypeComboBox->currentIndex() != defaultIndex) {
waveformTypeComboBox->setCurrentIndex(defaultIndex);
}
}

void DlgPrefWaveform::notifyRebootNecessary() {
// make the fact that you have to restart mixxx more obvious
QMessageBox::information(
Expand All @@ -253,19 +293,27 @@ void DlgPrefWaveform::slotSetWaveformEndRender(int endTime) {
WaveformWidgetFactory::instance()->setEndOfTrackWarningTime(endTime);
}

void DlgPrefWaveform::slotSetWaveformType(int index) {
// Ignore sets for -1 since this happens when we clear the combobox.
if (index < 0) {
void DlgPrefWaveform::slotSetWaveformType(int comboboxIndex) {
Q_UNUSED(comboboxIndex);

auto waveType = static_cast<WaveformWidgetType::Type>(
waveformTypeComboBox->currentData().toInt());

bool openGlWasEnabled = m_pConfig->getValue(ConfigKey("[Waveform]", "OpenGlEnabled"),
WaveformWidgetFactory::defaultOpenGlEnabled);
if (m_openGlEnabled != openGlWasEnabled) {
m_pConfig->setValue(ConfigKey("[Waveform]", "WaveformType"), static_cast<int>(waveType));
return;
}
WaveformWidgetFactory::instance()->setWidgetTypeFromHandle(index);

WaveformWidgetFactory::instance()->setWidgetType(waveType);
#if QT_VERSION >= QT_VERSION_CHECK(5, 14, 0) && defined __WINDOWS__
// This regenerates the waveforms twice because of a bug found on Windows
// where the first one fails.
// The problem is that the window of the widget thinks that it is not exposed.
// (https://doc.qt.io/qt-5/qwindow.html#exposeEvent )
// TODO: Remove this when it has been fixed upstream.
WaveformWidgetFactory::instance()->setWidgetTypeFromHandle(index, true);
WaveformWidgetFactory::instance()->reloadWaveforms();
#endif
}

Expand Down
7 changes: 6 additions & 1 deletion src/preferences/dialog/dlgprefwaveform.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ class DlgPrefWaveform : public DlgPreferencePage, public Ui::DlgPrefWaveformDlg
void slotSetWaveformEndRender(int endTime);

private slots:
void openGlEnabledChanged(bool checked);
void slotSetFrameRate(int frameRate);
void slotSetWaveformType(int index);
void slotSetWaveformType(int comboboxIndex);
void slotSetWaveformOverviewType(int index);
void slotSetDefaultZoom(int index);
void slotSetZoomSynchronization(bool checked);
Expand All @@ -40,10 +41,14 @@ class DlgPrefWaveform : public DlgPreferencePage, public Ui::DlgPrefWaveformDlg
void slotSetPlayMarkerPosition(int position);

private:
void populateWaveformTypeCombobox();
void pickDefaultWaveformType();
void initWaveformControl();
void calculateCachedWaveformDiskUsage();
void notifyRebootNecessary();

bool m_openGlEnabled;

UserSettingsPointer m_pConfig;
Library* m_pLibrary;
MixxxMainWindow* m_pMixxx;
Expand Down
Loading