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

[Stride] Changes the pin types of ColorMap, ColorIn (and others) to GPU<RGBA> #644

Merged
merged 10 commits into from Dec 18, 2023

Conversation

azeno
Copy link
Member

@azeno azeno commented Jul 4, 2023

PR Details

Description

This PR contains breaking changes as it changes the type on well knows nodes like ColorMap and ColorIn as well as internal sink nodes like ComposeDrawShader.

There are probably even more places where the type should be changed to GPU<RGBA>. We should further ensure that conversion nodes from RGBA and Color4 are present. Note that those nodes will be a no-op on GPU side.

Related Issue

#643

Motivation and Context

Once we start add/change shaders to make use of GPU<RGBA> pins it's only logical to be able to connect those pins to the obvious candidates such as ColorMap or ColorIn. Also sink nodes composing a pixel shader should express their intent by using this new pin type.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation

@azeno azeno added VL.Stride Wrapper for 3d game engine Stride help wanted Extra attention is needed labels Jul 4, 2023
@joreg
Copy link
Member

joreg commented Jul 13, 2023

  • check all current nodes with ComputeFloat4 inputs, like: TextureFX, ShaderFX, Colors on material nodes

@joreg joreg added this to the 5.3 milestone Jul 13, 2023
@azeno
Copy link
Member Author

azeno commented Sep 21, 2023

#652 should be merged first before continuing work on this one.

…rawShader` to `GPU<RGBA>`

This commit is a breaking change. It probably requires more thought on how it affects other nodes and we need to ensure that conversion nodes from/to Vector4 & RGBA are present.

Consider this commit WIP.
@joreg
Copy link
Member

joreg commented Dec 4, 2023

  • helppatches of texturefx nodes are not found: eg. press F1 on the Metallica node
  • many shader related enums are still in main Stride pack under "Advanced" aspect
  • some "Experimental" Filters are still in main Stride pack
  • shall we only allow ComputeColor instead of also the [Color] attribute for ComputeFloat4?
  • shall we also add a "VectorMap" node that returns GPU(Vector4)? and then SwizzleMap...
  • SetAlpha has Mask GPU(Vector4) input, see keying help

@azeno azeno changed the base branch from dev/compute-color-support to main December 4, 2023 13:05
@azeno
Copy link
Member Author

azeno commented Dec 4, 2023

helppatches of texturefx nodes are not found: eg. press F1 on the Metallica node

fixed

many shader related enums are still in main Stride pack under "Advanced" aspect

done

some "Experimental" Filters are still in main Stride pack

moved

shall we only allow ComputeColor instead of also the [Color] attribute for ComputeFloat4?

done (bdc6913)

shall we also add a "VectorMap" node that returns GPU(Vector4)? and then SwizzleMap...

no, use new Convert (RGBAToVector4) for that

SetAlpha has Mask GPU(Vector4) input, see keying help

use new Convert (RGBAToVector4)

@joreg
Copy link
Member

joreg commented Dec 11, 2023

  • SetAlpha node does not show up as "Also found in HowTo Key a Texture" help

@azeno azeno marked this pull request as ready for review December 18, 2023 17:20
@azeno azeno merged commit 80f00de into main Dec 18, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed VL.Stride Wrapper for 3d game engine Stride
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants