From f1d2d380fa7e73024f12d48f2ac4e5d8323018db Mon Sep 17 00:00:00 2001 From: Yuriy Toporovskyy Date: Mon, 21 Jun 2021 18:00:38 -0400 Subject: [PATCH 1/2] Bug fix for inability to change actor transform properties. Actor motion extraction applies at all times, sending transform change events to the Qt widget and overwriting any editing the user is doing. If the user is in the middle of editing, do not overwrite the current spinbox value. --- .../Components/Widgets/VectorInput.cpp | 37 ++++++++++++++++++- .../Components/Widgets/VectorInput.h | 19 ++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.cpp b/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.cpp index 7552fb0b50..d7e6cb85cd 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.cpp +++ b/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.cpp @@ -43,7 +43,7 @@ VectorElement::VectorElement(QWidget* parent) VectorElement::layout(this, m_spinBox, m_label, false); connect(m_spinBox, QOverload::of(&AzQtComponents::DoubleSpinBox::valueChanged), this, &VectorElement::onValueChanged); - connect(m_spinBox, &AzQtComponents::DoubleSpinBox::editingFinished, this, &VectorElement::editingFinished); + connect(m_spinBox, &AzQtComponents::DoubleSpinBox::editingFinished, this, &VectorElement::onSpinBoxEditingFinished); } void VectorElement::SetLabel(const char* label) @@ -65,6 +65,22 @@ const QString& VectorElement::label() const void VectorElement::setValue(double newValue) { + // Nothing to do if the value is not actually changed + if (AZ::IsClose(m_value, newValue, std::numeric_limits::epsilon())) + { + return; + } + + // If the spin box currently has focus, the user is editing it, so we should not + // change the value from non-user input while they're in the middle of editing + if (m_spinBox->hasFocus()) + { + auto& deferredValue = m_deferredExternalValue.emplace(); + deferredValue.value = newValue; + deferredValue.prevValue = m_value; + return; + } + m_value = newValue; const QSignalBlocker blocker(m_spinBox); m_spinBox->setValue(newValue); @@ -72,6 +88,23 @@ void VectorElement::setValue(double newValue) emit valueChanged(newValue); } +void VectorElement::onSpinBoxEditingFinished() +{ + if (m_deferredExternalValue) + { + DeferredSetValue deferredValue = *m_deferredExternalValue; + m_deferredExternalValue.reset(); + + if (m_value == deferredValue.prevValue) + { + AZ_Warning("VectorElement", !m_spinBox->hasFocus(), "Editing finished but the spinbox still has focus"); + setValue(deferredValue.value); + } + } + + emit editingFinished(); +} + void VectorElement::setCoordinate(VectorElement::Coordinate coordinate) { setProperty(g_CoordinatePropertyName, QVariant::fromValue(coordinate)); @@ -254,7 +287,7 @@ VectorInput::VectorInput(QWidget* parent, int elementCount, int elementsPerRow, { OnValueChangedInElement(value, elementIndex); }); - connect(m_elements[elementIndex]->GetSpinBox(), &AzQtComponents::DoubleSpinBox::editingFinished, this, &VectorInput::editingFinished); + connect(m_elements[elementIndex], &VectorElement::editingFinished, this, &VectorInput::editingFinished); numberOfElementsRemaining--; } diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.h b/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.h index 8fb906a5cf..a9cd743830 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.h +++ b/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.h @@ -14,6 +14,7 @@ #if !defined(Q_MOC_RUN) #include #include +#include #endif class QLabel; @@ -23,6 +24,12 @@ namespace AzQtComponents class Style; + #pragma warning(push) + + // 'AzQtComponents::VectorElement::m_deferredExternalValue': class 'AZStd::optional' needs to + // have dll-interface to be used by clients of class 'AzQtComponents::VectorElement' + #pragma warning(disable:4251) + /*! * \class VectorElement * \brief All flexible vector GUI's are constructed using a number vector elements. Each Vector @@ -103,7 +110,14 @@ namespace AzQtComponents void resizeLabel(); + void onSpinBoxEditingFinished(); + private: + struct DeferredSetValue + { + double prevValue, value; + }; + // m_labelText must be initialised before m_spinBox. It is used by editFieldRect, which gets // called by the spin box constructor. QString m_labelText = {}; @@ -113,8 +127,13 @@ namespace AzQtComponents double m_value = 0.0; //! Indicates whether the value in the spin box has been edited by the user or not bool m_wasValueEditedByUser = false; + //! If a value is editing, but not by the user, and the user is currently editing the value, + //! avoid overwriting their work, until they finish editing + AZStd::optional m_deferredExternalValue; }; + #pragma warning(pop) + ////////////////////////////////////////////////////////////////////////// /*! From c27b41776136b7d0ddd225041d3c49cfaf03bff7 Mon Sep 17 00:00:00 2001 From: Yuriy Toporovskyy Date: Thu, 24 Jun 2021 10:15:06 -0400 Subject: [PATCH 2/2] Address pr feedback - Use AZ warning macros instead of #pragma warning - std::numeric_limits->AZStd::numeric_limits - IsClose->IsCloseMag - AZ_Warning->AZ_Assert --- .../AzQtComponents/Components/Widgets/VectorInput.cpp | 5 +++-- .../AzQtComponents/Components/Widgets/VectorInput.h | 9 ++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.cpp b/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.cpp index d7e6cb85cd..84ec0a2d33 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.cpp +++ b/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.cpp @@ -15,6 +15,7 @@ #include #include +#include #include #include @@ -66,7 +67,7 @@ const QString& VectorElement::label() const void VectorElement::setValue(double newValue) { // Nothing to do if the value is not actually changed - if (AZ::IsClose(m_value, newValue, std::numeric_limits::epsilon())) + if (AZ::IsCloseMag(m_value, newValue, AZStd::numeric_limits::epsilon())) { return; } @@ -97,7 +98,7 @@ void VectorElement::onSpinBoxEditingFinished() if (m_value == deferredValue.prevValue) { - AZ_Warning("VectorElement", !m_spinBox->hasFocus(), "Editing finished but the spinbox still has focus"); + AZ_Assert(!m_spinBox->hasFocus(), "Editing finished but the spinbox still has focus"); setValue(deferredValue.value); } } diff --git a/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.h b/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.h index a9cd743830..1c015eb584 100644 --- a/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.h +++ b/Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/VectorInput.h @@ -24,11 +24,9 @@ namespace AzQtComponents class Style; - #pragma warning(push) - // 'AzQtComponents::VectorElement::m_deferredExternalValue': class 'AZStd::optional' needs to // have dll-interface to be used by clients of class 'AzQtComponents::VectorElement' - #pragma warning(disable:4251) + AZ_PUSH_DISABLE_DLL_EXPORT_MEMBER_WARNING /*! * \class VectorElement @@ -115,7 +113,8 @@ namespace AzQtComponents private: struct DeferredSetValue { - double prevValue, value; + double prevValue; + double value; }; // m_labelText must be initialised before m_spinBox. It is used by editFieldRect, which gets @@ -132,7 +131,7 @@ namespace AzQtComponents AZStd::optional m_deferredExternalValue; }; - #pragma warning(pop) + AZ_POP_DISABLE_DLL_EXPORT_MEMBER_WARNING //////////////////////////////////////////////////////////////////////////