Skip to content

Fix overwriting of alpha values in Viewer colorpicker #2404

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.

Sign up for GitHub

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

Merged
jstone-lucasfilm merged 6 commits into from
May 23, 2025
Merged

Fix overwriting of alpha values in Viewer colorpicker #2404

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f3eca85
MaterialXView: Fix incorrectly written Color4 field
mialana May 21, 2025
9788b79
MaterialXView: Fix alpha value being overwritten by colorpicker's col…
mialana May 21, 2025
44494cf
Merge branch 'main' into fix-viewer-colorpicker-alpha
jstone-lucasfilm May 22, 2025
7961db2
Merge branch 'main' into fix-viewer-colorpicker-alpha
jstone-lucasfilm May 22, 2025
c3c8303
Minor coding style updates
jstone-lucasfilm May 23, 2025
afa020b
Minor comment update
jstone-lucasfilm May 23, 2025
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
31 changes: 28 additions & 3 deletions source/MaterialXView/Editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,38 @@ class EditorColorPicker : public ng::ColorPicker
});
}

// The color wheel does not handle alpha properly, so only
// overwrite RGB in the callback.
// Overwrite default callback of color wheel.
m_color_wheel->set_callback([this](const ng::Color& c)
{
ng::Color cAlpha = ng::Color(c[0], c[1], c[2], _colorWidgets[3]->value());
m_color_wheel->set_color(cAlpha);

// Account for Alpha value when setting m_pick_button properties.
m_pick_button->set_background_color(cAlpha);
m_pick_button->set_text_color(cAlpha.contrasting_color());

m_callback(cAlpha);
});

// Overwrite default callback of m_pick_button.
m_pick_button->set_callback([&]()
{
if (m_pushed)
{
// Use _colorWidgets to construct new color value, which ensures that Alpha component is correctly written.
ng::Color value(_colorWidgets[0]->value(), _colorWidgets[1]->value(), _colorWidgets[2]->value(), _colorWidgets[3]->value());
set_pushed(false);
set_color(value);
m_final_callback(value);
}
});

m_callback = [this](const ng::Color& value)
{
_colorWidgets[0]->set_value(value[0]);
_colorWidgets[1]->set_value(value[1]);
_colorWidgets[2]->set_value(value[2]);
_colorWidgets[3]->set_value(value[3]);
};
}

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

@mialana mialana May 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Additionally, I realized that in my previous PR to handle color space transformations, I erroneously concatenated all 4D color vectors to 3D when writing to material. This is my mistake, and is fixed in this PR.


material->modifyUniform(path, mx::Value::createValue(v));
}
Expand Down