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.

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

MAYA-106075 - USD selection highlighting doesn't take Color Settings into consideration #2033

Merged
merged 5 commits into from
Jan 31, 2022
Merged

MAYA-106075 - USD selection highlighting doesn't take Color Settings into consideration #2033

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16f6157
Make sure to get up-to-date selection highlighting colors from Maya f…
vlasovi Jan 26, 2022
08a95c8
- Fixing the build for Maya versions below 2023
vlasovi Jan 26, 2022
33baf46
Fixing broken autotests: making sure the old, hardcoded selection hig…
vlasovi Jan 27, 2022
dede536
Using MAYA_API_VERSION instead of MAYA_APP_VERSION to fix the build o…
vlasovi Jan 27, 2022
8469047
- USD mesh selection color now matches Maya mesh polygon color.
vlasovi Jan 27, 2022
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
10 changes: 5 additions & 5 deletions lib/mayaUsd/render/vp2RenderDelegate/basisCurves.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,8 +1207,8 @@ void HdVP2BasisCurves::_UpdateDrawItem(
// with one color vertex buffer.
if (drawItem->ContainsUsage(HdVP2DrawItem::kSelectionHighlight)) {
const MColor colors[] = { drawScene.GetWireframeColor(),
drawScene.GetSelectionHighlightColor(false),
drawScene.GetSelectionHighlightColor(true) };
drawScene.GetSelectionHighlightColor("curve"),
drawScene.GetSelectionHighlightColor() };

// Store the indices to colors.
std::vector<unsigned char> colorIndices;
Expand Down Expand Up @@ -1264,9 +1264,9 @@ void HdVP2BasisCurves::_UpdateDrawItem(
int primitiveStride = 0;

const MColor& color
= (_selectionStatus != kUnselected
? drawScene.GetSelectionHighlightColor(_selectionStatus == kFullyLead)
: drawScene.GetWireframeColor());
= (_selectionStatus != kUnselected ? drawScene.GetSelectionHighlightColor(
_selectionStatus == kFullyLead ? nullptr : "curve")
: drawScene.GetWireframeColor());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this little bit of logic deciding which color to use based on the selection status move into a common location? I'm not sure if it makes sense on drawScene, maybe if we do a shared parentClass for rprims this logic could live there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think I will address that as part of my future work on points.


if (desc.geomStyle == HdBasisCurvesGeomStylePatch) {
if (_selectionStatus != kUnselected) {
Expand Down
10 changes: 5 additions & 5 deletions lib/mayaUsd/render/vp2RenderDelegate/mesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2106,8 +2106,8 @@ void HdVP2Mesh::_UpdateDrawItem(

// Set up the source color buffers.
const MColor wireframeColors[] = { drawScene.GetWireframeColor(),
drawScene.GetSelectionHighlightColor(false),
drawScene.GetSelectionHighlightColor(true) };
drawScene.GetSelectionHighlightColor("surface"),
drawScene.GetSelectionHighlightColor() };
bool useWireframeColors = stateToCommit._instanceColorParam == kSolidColorStr;

MFloatArray* shadedColors = nullptr;
Expand Down Expand Up @@ -2208,9 +2208,9 @@ void HdVP2Mesh::_UpdateDrawItem(
if ((itemDirtyBits & DirtySelectionHighlight)
&& drawItem->ContainsUsage(HdVP2DrawItem::kSelectionHighlight)) {
const MColor& color
= (_selectionStatus != kUnselected
? drawScene.GetSelectionHighlightColor(_selectionStatus == kFullyLead)
: drawScene.GetWireframeColor());
= (_selectionStatus != kUnselected ? drawScene.GetSelectionHighlightColor(
_selectionStatus == kFullyLead ? nullptr : "surface")
: drawScene.GetWireframeColor());

MHWRender::MShaderInstance* shader = _delegate->Get3dSolidShader(color);
if (shader != nullptr && shader != drawItemData._shader) {
Expand Down
34 changes: 31 additions & 3 deletions lib/mayaUsd/render/vp2RenderDelegate/proxyRenderDelegate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include <pxr/usd/usd/prim.h>
#include <pxr/usdImaging/usdImaging/delegate.h>

#include <maya/MColorPickerUtilities.h>
#include <maya/MEventMessage.h>
#include <maya/MFileIO.h>
#include <maya/MFnPluginData.h>
Expand Down Expand Up @@ -1500,12 +1501,39 @@ GfVec3f ProxyRenderDelegate::GetCurveDefaultColor()
}

//! \brief
const MColor& ProxyRenderDelegate::GetSelectionHighlightColor(bool lead) const
MColor ProxyRenderDelegate::GetSelectionHighlightColor(const char* className)
{
static const MColor kLeadColor(0.056f, 1.0f, 0.366f, 1.0f);
// Construct the query command string.
MString queryCommand;
if (className) {
queryCommand = "int $index = `displayColor -q -active \"";
queryCommand += className;
queryCommand += "\"`; colorIndex -q $index;";
} else {
queryCommand = "displayRGBColor -q \"lead\"";
}

// Query and return the selection color.
{
MDoubleArray colorResult;
std::lock_guard<std::mutex> _(_mayaCommandEngineMutex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename the std::lock_guard local variable to something meaningful? I can guess at your thinking that nobody should be doing anything to it after it is created because of the RAII nature of the object so the name doesn't matter, but I don't like the style of naming it underscore. It just looks strange to me and then I spent some time thinking about it.

MGlobal::executeCommand(queryCommand, colorResult);
Copy link
Contributor

Choose a reason for hiding this comment

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

If a caller passes in a className which isn't valid do we get some kind of error we can warn about?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

executeCommand runs with displayEnabled = false, so I guess it will not write any error info into log. I think it would be good to set displayEnabled to true here


if (colorResult.length() == 3) {
// Since the "lead" color is returned in displace space,
// we need to convert it to rendering space.
MColor color(colorResult[0], colorResult[1], colorResult[2]);
return className
? color
: MColorPickerUtilities::applyViewTransform(color, MColorPickerUtilities::kInverse);
}
}

// In case of an error, return the default color
static const MColor kLeadColor(0.351f, 5.519f, 0.408f, 1.0f);
static const MColor kActiveColor(1.0f, 1.0f, 1.0f, 1.0f);

return lead ? kLeadColor : kActiveColor;
return className ? kActiveColor : kLeadColor;
}

bool ProxyRenderDelegate::DrawRenderTag(const TfToken& renderTag) const
Expand Down
4 changes: 3 additions & 1 deletion lib/mayaUsd/render/vp2RenderDelegate/proxyRenderDelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,10 @@ class ProxyRenderDelegate : public MHWRender::MPxSubSceneOverride
MAYAUSD_CORE_PUBLIC
GfVec3f GetCurveDefaultColor();

// Returns the selection highlight color for a given class.
// If className is null, returns the lead highlight color.
MAYAUSD_CORE_PUBLIC
const MColor& GetSelectionHighlightColor(bool lead) const;
MColor GetSelectionHighlightColor(const char* className = nullptr);

MAYAUSD_CORE_PUBLIC
const HdSelection::PrimSelectionState* GetLeadSelectionState(const SdfPath& path) const;
Expand Down