-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
NME: Add a rgb output to the ImageProcessing block #13328
Conversation
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://babylonsnapshots.z22.web.core.windows.net/refs/pull/13328/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/13328/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/13328/merge#BCU1XR#0 |
Visualization tests for webgl1 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
Visualization tests for webgl2 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with this area that well, so this is just a code-review :-)
I will let someone else merge that.
NME: Add a rgb output to the ImageProcessing block Former-commit-id: c91258f2be027a1f4233a803d6d182d8822a0f38
The PR also changes the
color
input type to AutoDetect to make it clear that it accepts multiple types.While I was at it, I also updated some inputs in some blocks to AutoDetect for the same reason as above.
Closes BabylonJS/ThePirateCove#296
Note:
The test fix is due to the fact that this test is quite old and the node material generated code used the old way of connecting blocks together: we didn't use the name of the source output or the name of the input to be connected, the system selected them automagically. But this could fail in a number of ways, and we switched to a more robust system where we explicitely name the output of the source block and the input of the connecting block. The changes took place in October 2019.