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

Add support for 3D procedural textures #15114

Merged
merged 9 commits into from
May 29, 2024

Conversation

MiiBond
Copy link
Contributor

@MiiBond MiiBond commented May 17, 2024

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.

@bjsplat
Copy link
Collaborator

bjsplat commented May 17, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented May 17, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented May 17, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented May 17, 2024

Visualization tests for WebGPU (Experimental)
Important - these might fail sporadically. This is an optional test.

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15114/merge/testResults/webgpuplaywright/index.html

@sebavan sebavan requested a review from Popov72 May 17, 2024 19:46
Copy link
Member

@sebavan sebavan left a comment

Choose a reason for hiding this comment

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

LGTM

@deltakosh
Copy link
Contributor

@RaananW do you why the new test is failing?

@RaananW
Copy link
Member

RaananW commented May 21, 2024

Regarding WebGL - I am checking right now.

Regarding WebGPU, this is the (shortened) log:

 undefined
Running test: 3D Procedural Textures . Meta:  #AYZY4Q#6
[3D Procedural Textures] The texture dimension must not be TextureDimension::e3D for a render attachment if allow_unsafe_apis is not enabled. See crbug.com/dawn/1020.
 - While validating [TextureDescriptor "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"].
 - While calling [Device "BabylonWebGPUDevice197"].CreateTexture([TextureDescriptor "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"]).

[3D Procedural Textures] BJS - [16:40:36]: WebGPU uncaptured error (1): [object GPUValidationError] - The texture dimension must not be TextureDimension::e3D for a render attachment if allow_unsafe_apis is not enabled. See crbug.com/dawn/1020.
 - While validating [TextureDescriptor "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"].
 - While calling [Device "BabylonWebGPUDevice197"].CreateTexture([TextureDescriptor "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"]).

[3D Procedural Textures] [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"] is invalid.
 - While calling [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"].CreateView([TextureViewDescriptor "BabylonWebGPUDevice197_TextureView3D_256x256x256_womips_rgba8unorm_3d_all_noname"]).

[3D Procedural Textures] BJS - [16:40:36]: WebGPU uncaptured error (2): [object GPUValidationError] - [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"] is invalid.
 - While calling [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"].CreateView([TextureViewDescriptor "BabylonWebGPUDevice197_TextureView3D_256x256x256_womips_rgba8unorm_3d_all_noname"]).

[3D Procedural Textures] [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"] is invalid.
 - While calling [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"].CreateView([TextureViewDescriptor]).

[3D Procedural Textures] BJS - [16:40:36]: WebGPU uncaptured error (3): [object GPUValidationError] - [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"] is invalid.
 - While calling [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"].CreateView([TextureViewDescriptor]).

[3D Procedural Textures] [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"] is invalid.
 - While calling [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"].CreateView([TextureViewDescriptor]).

[3D Procedural Textures] BJS - [16:40:36]: WebGPU uncaptured error (4): [object GPUValidationError] - [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"] is invalid.
 - While calling [Invalid Texture "BabylonWebGPUDevice197_Texture3D_256x256x256_womips_rgba8unorm_samples1"].CreateView([TextureViewDescriptor]).

[3D Procedural Textures] WebGPU: too many warnings, no more warnings will be reported to the console for this GPUDevice.

@RaananW
Copy link
Member

RaananW commented May 21, 2024

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)

@RaananW
Copy link
Member

RaananW commented May 21, 2024

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.

@sebavan
Copy link
Member

sebavan commented May 21, 2024

cc @Popov72 as well regarding the WebGPU error in case he can spot smthg ?

@Popov72
Copy link
Contributor

Popov72 commented May 21, 2024

Regarding WebGPU, we have to allow unsafe apis to be able to render to 3D texture. Try to start chrome with the flag: --enable-unsafe-webgpu

If it does not work, try also with --enable-webgpu-developer-features

@sebavan
Copy link
Member

sebavan commented May 21, 2024

@RaananW looks like webgpu is ok now, is that expected ?

@RaananW
Copy link
Member

RaananW commented May 21, 2024

@RaananW looks like webgpu is ok now, is that expected ?

It is not:

https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15114/merge/testResults/webgpuplaywright/index.html#?testId=5e014ad2de5afab284ac-e489ce6871b4ce2a05b2

It does render correctly, but the generated image doesn't match

@sebavan
Copy link
Member

sebavan commented May 21, 2024

Oh right was spoiled by the green tick :-)

@sebavan
Copy link
Member

sebavan commented May 22, 2024

@MiiBond it looks like you might need to regenerate the test picture and it should be all good

@MiiBond
Copy link
Contributor Author

MiiBond commented May 22, 2024

@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.
I have 3 samplers in my procedural shader and the first works only for layer 0. After that, the uniform looks like this:
image

The other two samplers are fine.

@Popov72
Copy link
Contributor

Popov72 commented May 23, 2024

Make sure you added voxelXAxisSampler in the list of sampler names when you create the shader. Also, you should try to debug in WebGPU, as you may get better error messages than in WebGL.

@sebavan
Copy link
Member

sebavan commented May 23, 2024

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

@MiiBond
Copy link
Contributor Author

MiiBond commented May 23, 2024

I haven't figured out what's happening in my branch but I made a simple playground and it seems to be behaving as I expect.
However, Spector.js isn't showing the render of layer 0 properly and that makes me a bit suspicious that there really is something wrong. Every layer has a snapshot displayed except the first one.
image

Playground is #AYZY4Q#11

@MiiBond
Copy link
Contributor Author

MiiBond commented May 23, 2024

Another issue is that when I run npm run test:visualization -- -i "webgl2" -t "3D Procedural Textures" -u to regenerate the image, all the tests run. What's more, 4 tests fail, including lens-flare, and the 3D Procedural Textures test passes.

@sebavan
Copy link
Member

sebavan commented May 23, 2024

cc @RaananW for the tests, for spector I am more suspicious of the Spector code :-) It looks like the layers are correct
image

@Popov72
Copy link
Contributor

Popov72 commented May 27, 2024

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.

@sebavan
Copy link
Member

sebavan commented May 27, 2024

In this case should we disable it from auto run ? cc @RaananW as it would be great to get this merged in ?

@RaananW
Copy link
Member

RaananW commented May 27, 2024

i'll ru the tests locally and check what happened there. sorry for the late reply!

@RaananW
Copy link
Member

RaananW commented May 27, 2024

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 run playwright use the following command:

npm run test:playwright -w @tools/tests -- --project=webgl2 --ui, making sure the local babylon server is running.
If you have a .env file you can test against this PR instead of running the local server by adding

CDN_BASE_URL=https://babylonsnapshots.z22.web.core.windows.net/refs/pull/15114/merge as environment variable

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.

@RaananW
Copy link
Member

RaananW commented May 28, 2024

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:

image

Checked under firefox as well:

image

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:

image

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

@Popov72
Copy link
Contributor

Popov72 commented May 28, 2024

That's weird, on my local computer (Windows 11), I get the same picture you get on your Mac:

image

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.

@Popov72
Copy link
Contributor

Popov72 commented May 28, 2024

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) so that it at least runs somewhere?

@sebavan
Copy link
Member

sebavan commented May 28, 2024

excludeFromAutomaticTesting: true would be good @MiiBond can you do the change ?

@RaananW
Copy link
Member

RaananW commented May 28, 2024

I would be very interested to know why it generates different results.

@sebavan
Copy link
Member

sebavan commented May 29, 2024

yup, lets check it offline after the merge.

@sebavan sebavan merged commit 5c9cdeb into BabylonJS:master May 29, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants