Skip to content

Commit e6fe438

Browse files
alexmontAmazonalexmontAmazon
authored
Speed up DPE's color picker handling by 2-3x and disallow redundant updates (o3de#17493)
* queue refresh in singleshot to keep it from happening more than once a loop Signed-off-by: Alex Montgomery <[email protected]> * doubled the color picker performance Signed-off-by: Alex Montgomery <[email protected]> --------- Signed-off-by: Alex Montgomery <[email protected]>
1 parent 9cb554b commit e6fe438

File tree

8 files changed

+64
-63
lines changed

8 files changed

+64
-63
lines changed

‎Code/Framework/AzFramework/AzFramework/DocumentPropertyEditor/ReflectionAdapter.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1295,7 +1295,12 @@ namespace AZ::DocumentPropertyEditor
12951295

12961296
void ReflectionAdapter::UpdateDomContents(const PropertyChangeInfo& propertyChangeInfo)
12971297
{
1298-
NotifyContentsChanged({ Dom::PatchOperation::ReplaceOperation(propertyChangeInfo.path / "Value", propertyChangeInfo.newValue) });
1298+
auto valuePath = propertyChangeInfo.path / "Value";
1299+
auto currValue = GetContents()[valuePath];
1300+
if (currValue != propertyChangeInfo.newValue)
1301+
{
1302+
NotifyContentsChanged({ Dom::PatchOperation::ReplaceOperation(valuePath, propertyChangeInfo.newValue) });
1303+
}
12991304
}
13001305

13011306
ExpanderSettings* ReflectionAdapter::CreateExpanderSettings(

‎Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/ColorPicker.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -419,9 +419,8 @@ ColorPicker::ColorPicker(ColorPicker::Configuration configuration, const QString
419419
connect(m_currentColorController, &Internal::ColorController::hsvHueChanged, m_colorGrid, &ColorGrid::setHue);
420420
connect(m_currentColorController, &Internal::ColorController::hsvSaturationChanged, m_colorGrid, &ColorGrid::setSaturation);
421421
connect(m_currentColorController, &Internal::ColorController::valueChanged, m_colorGrid, &ColorGrid::setValue);
422-
connect(m_colorGrid, &ColorGrid::hueChanged, m_currentColorController, &Internal::ColorController::setHsvHue);
423-
connect(m_colorGrid, &ColorGrid::saturationChanged, m_currentColorController, &Internal::ColorController::setHsvSaturation);
424-
connect(m_colorGrid, &ColorGrid::valueChanged, m_currentColorController, &Internal::ColorController::setValue);
422+
connect(m_colorGrid, &ColorGrid::hsvChanged, m_currentColorController, &Internal::ColorController::setHSV);
423+
425424
connect(m_colorGrid, &ColorGrid::gridPressed, this, &ColorPicker::beginDynamicColorChange);
426425
connect(m_colorGrid, &ColorGrid::gridReleased, this, &ColorPicker::endDynamicColorChange);
427426

‎Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/ColorPicker/ColorController.cpp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,36 @@ namespace AzQtComponents
645645
emit colorChanged(m_state->rgb());
646646
}
647647

648+
void ColorController::setHSV(float hue, float saturation, float value)
649+
{
650+
const bool hChanged = !qFuzzyCompare(hue, hsvHue());
651+
const bool sChanged = !qFuzzyCompare(saturation, hsvSaturation());
652+
const bool vChanged = !qFuzzyCompare(value, this->value());
653+
if (hChanged || sChanged || vChanged)
654+
{
655+
const ColorState previousColor = *m_state;
656+
m_state->setHsv(hue, saturation, value);
657+
validate();
658+
659+
if (hChanged)
660+
{
661+
emit hsvHueChanged(hsvHue());
662+
}
663+
if (sChanged)
664+
{
665+
emit hsvSaturationChanged(hsvSaturation());
666+
}
667+
if (vChanged)
668+
{
669+
emit valueChanged(this->value());
670+
}
671+
672+
emitRgbaChangedSignals(previousColor);
673+
emitHslChangedSignals(previousColor);
674+
emit colorChanged(m_state->rgb());
675+
}
676+
}
677+
648678
void ColorController::setAlpha(float alpha)
649679
{
650680
if (qFuzzyCompare(alpha, this->alpha()))

‎Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/ColorPicker/ColorController.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ namespace AzQtComponents
8989
void setHsvHue(float hue);
9090
void setHsvSaturation(float saturation);
9191
void setValue(float value);
92+
void setHSV(float hue, float saturation, float value);
9293

9394
void setAlpha(float alpha);
9495

‎Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/ColorPicker/ColorGrid.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,25 +230,23 @@ void ColorGrid::handleLeftButtonEvent(const QPoint& eventPos)
230230
{
231231
m_hue = hsv.hue;
232232
updated = true;
233-
emit hueChanged(m_hue);
234233
}
235234

236235
if (!qFuzzyCompare(hsv.saturation, m_saturation))
237236
{
238237
m_saturation = hsv.saturation;
239238
updated = true;
240-
emit saturationChanged(m_saturation);
241239
}
242240

243241
if (!qFuzzyCompare(hsv.value, m_value))
244242
{
245243
m_value = hsv.value;
246244
updated = true;
247-
emit valueChanged(m_value);
248245
}
249246

250247
if (updated)
251248
{
249+
emit hsvChanged(m_hue, m_saturation, m_value);
252250
update();
253251
}
254252
}

‎Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/ColorPicker/ColorGrid.h‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ namespace AzQtComponents
1818
: public QFrame
1919
{
2020
Q_OBJECT
21-
Q_PROPERTY(qreal hue READ hue WRITE setHue NOTIFY hueChanged)
22-
Q_PROPERTY(qreal saturation READ saturation WRITE setSaturation NOTIFY saturationChanged)
23-
Q_PROPERTY(qreal value READ value WRITE setValue NOTIFY valueChanged)
21+
Q_PROPERTY(qreal hue READ hue WRITE setHue)
22+
Q_PROPERTY(qreal saturation READ saturation WRITE setSaturation)
23+
Q_PROPERTY(qreal value READ value WRITE setValue)
2424

2525
public:
2626
explicit ColorGrid(QWidget* parent = nullptr);
@@ -50,9 +50,7 @@ namespace AzQtComponents
5050

5151
Q_SIGNALS:
5252
void gridPressed();
53-
void hueChanged(qreal hue);
54-
void saturationChanged(qreal saturation);
55-
void valueChanged(qreal value);
53+
void hsvChanged(qreal hue, qreal saturation, qreal value);
5654
void gridReleased();
5755

5856
protected:

‎Code/Framework/AzToolsFramework/AzToolsFramework/UI/DocumentPropertyEditor/DPEComponentAdapter.cpp‎

Lines changed: 19 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -38,56 +38,34 @@ namespace AZ::DocumentPropertyEditor
3838
{
3939
if (m_componentId == componentId)
4040
{
41-
m_queuedRefreshLevel = AzToolsFramework::PropertyModificationRefreshLevel::Refresh_Values;
42-
QPointer<QObject> stillAlive(&m_stillAlive);
43-
QTimer::singleShot(
44-
0,
45-
[this, stillAlive]()
46-
{
47-
// make sure the component adapter still exists by the time this refresh resolves
48-
if (stillAlive)
49-
{
50-
DoRefresh();
51-
}
52-
});
41+
RequestRefresh(AzToolsFramework::PropertyModificationRefreshLevel::Refresh_Values);
5342
}
5443
}
5544

5645
void ComponentAdapter::InvalidatePropertyDisplay(AzToolsFramework::PropertyModificationRefreshLevel level)
5746
{
58-
if (level > m_queuedRefreshLevel)
59-
{
60-
m_queuedRefreshLevel = level;
61-
QPointer<QObject> stillAlive(&m_stillAlive);
62-
QTimer::singleShot(
63-
0,
64-
[this, stillAlive]()
65-
{
66-
// make sure the component adapter still exists by the time this refresh resolves
67-
if (stillAlive)
68-
{
69-
DoRefresh();
70-
}
71-
});
72-
}
47+
RequestRefresh(level);
7348
}
7449

7550
void ComponentAdapter::RequestRefresh(AzToolsFramework::PropertyModificationRefreshLevel level)
7651
{
7752
if (level > m_queuedRefreshLevel)
7853
{
79-
m_queuedRefreshLevel = level;
80-
QPointer<QObject> stillAlive(&m_stillAlive);
81-
QTimer::singleShot(
82-
0,
83-
[this, stillAlive]()
84-
{
85-
// make sure the component adapter still exists by the time this refresh resolves
86-
if (stillAlive)
54+
if (m_queuedRefreshLevel == AzToolsFramework::PropertyModificationRefreshLevel::Refresh_None)
55+
{
56+
QPointer<QObject> stillAlive(&m_stillAlive);
57+
QTimer::singleShot(
58+
0,
59+
[this, stillAlive]()
8760
{
88-
DoRefresh();
89-
}
90-
});
61+
// make sure the component adapter still exists by the time this refresh resolves
62+
if (stillAlive)
63+
{
64+
DoRefresh();
65+
}
66+
});
67+
}
68+
m_queuedRefreshLevel = level;
9169
}
9270
}
9371

@@ -126,17 +104,11 @@ namespace AZ::DocumentPropertyEditor
126104

127105
void ComponentAdapter::DoRefresh()
128106
{
129-
if (!IsComponentValid())
130-
{
131-
return;
132-
}
133-
134-
if (m_queuedRefreshLevel == AzToolsFramework::PropertyModificationRefreshLevel::Refresh_None)
107+
if (IsComponentValid())
135108
{
136-
return;
109+
m_queuedRefreshLevel = AzToolsFramework::PropertyModificationRefreshLevel::Refresh_None;
110+
NotifyResetDocument();
137111
}
138-
m_queuedRefreshLevel = AzToolsFramework::PropertyModificationRefreshLevel::Refresh_None;
139-
NotifyResetDocument();
140112
}
141113

142114
Dom::Value ComponentAdapter::HandleMessage(const AdapterMessage& message)

‎Code/Framework/AzToolsFramework/AzToolsFramework/UI/DocumentPropertyEditor/DPEComponentAdapter.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@ namespace AZ::DocumentPropertyEditor
4646
//! Sets the component, connects the appropriate Bus Handlers and sets the reflect data for this instance
4747
virtual void SetComponent(AZ::Component* componentInstance);
4848

49-
//! Trigger a refresh based on messages from the listeners
50-
void DoRefresh();
51-
5249
Dom::Value HandleMessage(const AdapterMessage& message) override;
5350

5451
//! Creates a node for displaying label information.
@@ -64,6 +61,7 @@ namespace AZ::DocumentPropertyEditor
6461
private:
6562
//! Checks if the component is still valid in the entity.
6663
bool IsComponentValid() const;
64+
void DoRefresh();
6765

6866
protected:
6967
AZ::EntityId m_entityId;

0 commit comments

Comments
 (0)