-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add support for 3D procedural textures #15114
Add support for 3D procedural textures #15114
Conversation
packages/dev/core/src/Materials/Textures/Procedurals/proceduralTexture.ts
Show resolved
Hide resolved
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/15114/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/15114/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/15114/merge#BCU1XR#0 |
WebGL2 visualization test reporter: |
Visualization tests for WebGPU (Experimental) |
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.
LGTM
@RaananW do you why the new test is failing? |
Regarding WebGL - I am checking right now. Regarding WebGPU, this is the (shortened) log:
|
Playwright has an allergy to underscores in filenames. I'll see what can be done about it, but until then - not underscores :-) Or let it generate the file by itself, and then the file will be generated with the filename it is looking for. Oh, and I pushed a fix (renaming the file) |
WebGL2 tests pass, however, WebGPU fails (you can see in the link provided in the comments). I'll let you decide if we should merge or investigate. |
cc @Popov72 as well regarding the WebGPU error in case he can spot smthg ? |
Regarding WebGPU, we have to allow unsafe apis to be able to render to 3D texture. Try to start chrome with the flag: If it does not work, try also with |
@RaananW looks like webgpu is ok now, is that expected ? |
It is not: It does render correctly, but the generated image doesn't match |
Oh right was spoiled by the green tick :-) |
@MiiBond it looks like you might need to regenerate the test picture and it should be all good |
I'm currently looking at this still. I'm having an issue with this in my IBL shadows branch. The other two samplers are fine. |
Make sure you added |
And be careful it is a minor case "a" for axis but If you have the broken code somewhere I can try having a look |
Another issue is that when I run |
cc @RaananW for the tests, for spector I am more suspicious of the Spector code :-) It looks like the layers are correct |
As for @MiiBond, I tested the visualization test locally and it does work as expected (I also have 4 other tests that fail, but the 3D procedural texture test is ok). So, maybe there's a bug with the framework we are using to run the tests when rendering to a 3D texture? Note: just for testing purpose, I moved the test to the top of the list and re-run the process. |
In this case should we disable it from auto run ? cc @RaananW as it would be great to get this merged in ? |
i'll ru the tests locally and check what happened there. sorry for the late reply! |
There seems to be a discrepancy between playwright and puppeteer which I will have to investigate. We use playwright for the Ci testing. I get consistently the exact same result as the CI, on a mac (using chromium).
If you want to update the snapshot delete the image and run the test once, playwright will generate the image for you. Might be a mac thing as well TBH - I had that already with other tests (especially with webgpu). what OS are you testing on? Anyhow - I will check what the difference is between the two test coordinators (puppeteer and playwright). There shouldn't be any. |
Running this in the browser - https://playground.babylonjs.com/?snapshot=refs/pull/15114/merge#AYZY4Q#7, changing the canvas to 600x400, i get what playwright is generating, and not what puppeteer renders: Checked under firefox as well: Investigation continues.I would recommend updating the image if we want it merged, while I test what went wrong with puppeteer. Edit - might be an OS/GPU thing? This is the result on my device (mac, M2) when running using puppeteer: I can update the image on my end if we want it to pass the tests, but we can discuss this whenever you have the time |
That's weird, on my local computer (Windows 11), I get the same picture you get on your Mac: It's what I get both with playwright locally and when browsing https://playground.babylonjs.com/?snapshot=refs/pull/15114/merge#AYZY4Q#7 Note that I get the same pictures in WebGPU. Note that puppeeter generates the same picture than playwright. |
I think that if the test can't work both locally and on the CI, we'll have to disable it for all engines? Or do we configure it to run only on the local computer ( |
excludeFromAutomaticTesting: true would be good @MiiBond can you do the change ? |
I would be very interested to know why it generates different results. |
yup, lets check it offline after the merge. |
This adds the ability to have a 3D ProceduralTexture. It renders all layers of the 3D texture, each time setting a uniform called "layer" to a value between 0-1.