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.

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

proposal: selected geometry is highlighted with xray outline. #11417

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented Jan 20, 2021

Purpose

This PR uses a standard helix post effect shader to display hidden geometry (occluded) with an xray outline post effect.
Note that in newer versions of helix the color is more saturated - in this pr I've scaled down our selection color by .5f because the shader multiplies the color and makes it quite bright. We'll have to evaluate the color changes when moving to new helix versions.

The highlight works for points, meshes and lines.

Screen Shot 2021-01-20 at 5 08 30 PM
Screen Shot 2021-01-20 at 5 08 35 PM
Screen Shot 2021-01-20 at 5 09 01 PM

PR also removes some dead references to dlls we no longer require.

TODO -

  • new tests
  • update image comparison tests that will now fail.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

remove passes we dont need
setup post effect string once
remove effects references we dont need
@@ -169,10 +169,6 @@
<ItemGroup>
<None Include="packages.config" />
</ItemGroup>
<ItemGroup>
<Content Include="sharpdx_direct3d11_effects_x64.dll" />
Copy link
Member Author

Choose a reason for hiding this comment

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

sadly, I may need to revert this for patch compatibility.... @QilongTang what do you think?

@mjkkirschner mjkkirschner changed the title proposal selected geometry is highlighted with xray outline. proposal: selected geometry is highlighted with xray outline. Jan 21, 2021
@aparajit-pratap
Copy link
Contributor

@mjkkirschner what was it you were saying about the x-ray outline not appearing on a white background preview using the current helix version?

@mjkkirschner
Copy link
Member Author

The way the color is blended with a white background has been changed in later versions of helix so that the blended color is less bright. Today it becomes white and is not very visible on a bright background.

@@ -105,6 +105,26 @@ protected void AddDynamoTechniques()
BlendStateDescription = DefaultBlendStateDescriptions.BSAlphaBlend,
DepthStencilStateDescription = DefaultDepthStencilDescriptions.DSSDepthLess
},
new ShaderPassDescription(DefaultPassNames.MeshOutline)
Copy link
Contributor

Choose a reason for hiding this comment

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

extra indentation?

Copy link
Contributor

Choose a reason for hiding this comment

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

I notice this is an extra shader pass description for mesh over that of points and lines in addition to the DefaultPassNames.EffectOutlineP1. Could you explain what's the difference between MeshOutline and EffectOutlineP1?

},
BlendStateDescription = DefaultBlendStateDescriptions.BSOverlayBlending,
DepthStencilStateDescription = DefaultDepthStencilDescriptions.DSSLessEqualNoWrite
}, new ShaderPassDescription(DefaultPassNames.EffectOutlineP1)
Copy link
Contributor

Choose a reason for hiding this comment

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

how about starting the new on a new line?

foreach (var element in items)
{
AttachedProperties.SetShowSelected(element, isSelected);
(element as GeometryModel3D).PostEffects = postEffect;
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this relate to the ShaderPassDescription for DefaultPassNames.EffectOutlineP1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants