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

[BETA] ESM Grid Renderer Script #7079

Merged
merged 40 commits into from
Dec 17, 2024
Merged

[BETA] ESM Grid Renderer Script #7079

merged 40 commits into from
Dec 17, 2024

Conversation

kpal81xd
Copy link
Contributor

@kpal81xd kpal81xd commented Oct 30, 2024

Ports grid into a script from supersplat editor with changes:

  • Resolution resolution: GridRenderer.RESOLUTION_HIGH | GridRenderer.RESOLUTION_MEDIUM | GridRenderer.RESOLUTION_LOW
  • Half extents halfExtents: Vec2
  • Grid can be recolored colorX: Color, colorZ: Color

Preview

image

@kpal81xd kpal81xd changed the title ESM Grid ESM Infinite Grid Oct 30, 2024
@kpal81xd kpal81xd self-assigned this Oct 30, 2024
@kpal81xd kpal81xd requested a review from willeastcott October 30, 2024 18:38
@kpal81xd kpal81xd changed the title ESM Infinite Grid [BETA] ESM Infinite Grid Oct 30, 2024
@kpal81xd kpal81xd changed the title [BETA] ESM Infinite Grid [BETA] ESM Grid Oct 31, 2024
@marklundin
Copy link
Member

Just FYI, the grid breaksup over these objects

image

@kpal81xd
Copy link
Contributor Author

kpal81xd commented Nov 1, 2024

Just FYI, the grid breaksup over these objects

image

Yea so thats the dithering alpha @slimbuck we should prob add a flag for this what do you think?

@slimbuck
Copy link
Member

slimbuck commented Nov 1, 2024

Make sure the grid is the last opaque object rendered. Then only semitrans objects will expose the dithering.

@marklundin
Copy link
Member

The grid doesn't scale correctly when the screen aspect ratio changes

Screen.Recording.2024-11-01.at.15.11.30.mov

@marklundin
Copy link
Member

The grid doesn't scale correctly when the screen aspect ratio changes

Screen.Recording.2024-11-01.at.15.11.30.mov

This seems to be related to getFrustumCorners! And I've just seen the corresponding fix https://github.com/playcanvas/engine/pull/7077/files

@mvaligursky
Copy link
Contributor

@kpal81xd - keen to get this ready for the release?

@kpal81xd
Copy link
Contributor Author

kpal81xd commented Dec 2, 2024

@kpal81xd - keen to get this ready for the release?

Id say no since I have another PR for a grid that uses a mesh instead of being just a renderer WIP so I want to evaluate the use case of both

@kpal81xd kpal81xd changed the title [BETA] ESM Grid Renderer [BETA] ESM Grid Renderer Script Dec 9, 2024
}
this.app.scene.on('prerender:layer', (camera, layer, transparent) => {
if (!camera.layers.includes(targetLayer.id)) {
camera.layers = camera.layers.concat(targetLayer.id);
Copy link
Contributor

Choose a reason for hiding this comment

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

I really don't like changing camera.layers from inside the rendering callback.
That means at least one frame will be rendered incorrectly, and could even cause some crashes, as for example meshes on that layer were not culled for the camera, but in the middle of the frame, camera new needs to render them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh i see what would you suggest I change it to instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So ive currently changed it to store the cameras in the prerender but update the layers in an update loop. Would that solve the issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

It would not, it's still a mid frame rendering callback.
Things like that need to be set up before the rendering, and not during.
So doing things like that in the scripts initialise / update is the correct way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess theres no other way then and the grid has to be attached to the camera

Copy link
Contributor

Choose a reason for hiding this comment

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

Or the grid could be assigned a layer, that's what it really needs right? So that the mesh instance is added to that layer.
Inside the preRender callback, a uniform with the camera properties can be changed on the material, I believe there was this need as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the grid does have a layer set so I could add a hook on there but does that pass the camera as parameter?

Not sure what you mean about the material tho

Copy link
Contributor

Choose a reason for hiding this comment

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

this one does:

engine/src/scene/scene.js

Lines 109 to 120 in c26ecf6

* Fired when the camera renders a layer. The handler is passed the {@link CameraComponent},
* the {@link Layer} that will be rendered, and a boolean parameter set to true if the layer is
* transparent. This is called during rendering to a render target or a default framebuffer, and
* additional rendering can be performed here, for example using {@link QuadRender#render}.
*
* @event
* @example
* app.scene.on('postrender:layer', (camera, layer, transparent) => {
* console.log(`Camera ${camera.entity.name} rendered the layer ${layer.name} (transparent: ${transparent})`);
* });
*/
static EVENT_POSTRENDER_LAYER = 'postrender:layer';

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought you needed to set some camera view matrix or similar uniform, not entirely sure. Ignore if not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not set I just need to get the projection and the frustum corners

@kpal81xd kpal81xd merged commit c2d9e35 into main Dec 17, 2024
8 checks passed
@kpal81xd kpal81xd deleted the grid branch December 17, 2024 14:11
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.

6 participants