Skip to content

Commit 667d309

Browse files
authored
Fix overwriting of alpha values in Viewer colorpicker (#2404)
Currently, if an user uses the MaterialXView colorpicker for a `Color4` field, the alpha component is automatically overwritten and transformed to a value of 1, regardless of original value. Although it is shown unmodified in the UI, if written out to file, the alpha component is always "1". e.g. ``` <input name="bias" type="color4" value="0.0908109, 0.0908109, 0.0908109, 1" /> ``` This is because the Nanogui `ColorWheel` class does not handle color values with 4 components, yet the Nanogui `ColorPicker` class refers to its color wheel member to set the new picked color ([code ref](https://github.com/mitsuba-renderer/nanogui/blob/6452dd6944d2ba5c0c9bc0042a1894f703ce1ace/src/colorpicker.cpp#L58-L65)). To prevent this from happening, I modified the current `class EditorColorPicker : public ng::ColorPicker`, and created custom callbacks for `m_color_wheel` and `m_pick_button`. The logic essentially is -- since we have additional color widget members in our color picker, we instead refer to their values when setting the color value. Then, the alpha component can only be changed via the color widgets, so interacting with the color wheel does not change / overwrite the original alpha component.
1 parent 656a7f6 commit 667d309

1 file changed

Lines changed: 28 additions & 3 deletions

File tree

source/MaterialXView/Editor.cpp

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,38 @@ class EditorColorPicker : public ng::ColorPicker
4848
});
4949
}
5050

51-
// The color wheel does not handle alpha properly, so only
52-
// overwrite RGB in the callback.
51+
// Overwrite default callback of color wheel.
52+
m_color_wheel->set_callback([this](const ng::Color& c)
53+
{
54+
ng::Color cAlpha = ng::Color(c[0], c[1], c[2], _colorWidgets[3]->value());
55+
m_color_wheel->set_color(cAlpha);
56+
57+
// Account for Alpha value when setting m_pick_button properties.
58+
m_pick_button->set_background_color(cAlpha);
59+
m_pick_button->set_text_color(cAlpha.contrasting_color());
60+
61+
m_callback(cAlpha);
62+
});
63+
64+
// Overwrite default callback of m_pick_button.
65+
m_pick_button->set_callback([&]()
66+
{
67+
if (m_pushed)
68+
{
69+
// Use _colorWidgets to construct new color value, which ensures that Alpha component is correctly written.
70+
ng::Color value(_colorWidgets[0]->value(), _colorWidgets[1]->value(), _colorWidgets[2]->value(), _colorWidgets[3]->value());
71+
set_pushed(false);
72+
set_color(value);
73+
m_final_callback(value);
74+
}
75+
});
76+
5377
m_callback = [this](const ng::Color& value)
5478
{
5579
_colorWidgets[0]->set_value(value[0]);
5680
_colorWidgets[1]->set_value(value[1]);
5781
_colorWidgets[2]->set_value(value[2]);
82+
_colorWidgets[3]->set_value(value[3]);
5883
};
5984
}
6085

@@ -374,7 +399,7 @@ void PropertyEditor::addItemToForm(const mx::UIPropertyItem& item, const std::st
374399
{
375400
// Transform sRGBA color picker value to linear space for writing to material
376401
mx::Color3 linearCol = mx::Color3(c.r(), c.g(), c.b()).srgbToLinear();
377-
mx::Vector3 v(linearCol[0], linearCol[1], linearCol[2]);
402+
mx::Vector4 v(linearCol[0], linearCol[1], linearCol[2], c.a());
378403

379404
material->modifyUniform(path, mx::Value::createValue(v));
380405
}

0 commit comments

Comments
 (0)