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

DropInViewer seems to render but is invisible in three.js scene #357

Open
hybridherbst opened this issue Oct 30, 2024 · 17 comments
Open

DropInViewer seems to render but is invisible in three.js scene #357

hybridherbst opened this issue Oct 30, 2024 · 17 comments

Comments

@hybridherbst
Copy link

This project uses Needle Engine (a component system for three.js).
I attempted to add the gaussian splat viewer, however, something seems to be missing - the splats are rendered (I can see the corresponding draw call, but no visuals, in spector.js) but nothing shows up.

Reproduction: https://stackblitz.com/edit/three-dev-n6xvbd?file=main2.js

  1. Load the page
  2. Wait for installation
  3. On the right side, a 3D scene appears
  4. This repo's built-in progress bar correctly shows up
  5. The draw call seems to be sent, but nothing is rendered
@seppestaes
Copy link

I'm not familiar with the product, from what i see in your example the grid and axisHelper are Needle's? Is there a way to access/pass Needle's camera, renderer to the viewer?

@hybridherbst
Copy link
Author

Yes, but since Needle is using three.js under the hood camera and renderer are already accessible to GaussianSplats3D in the onBeforeRender callback that DropInViewer uses. There should be no difference to three.js in this regard.

If need be they can be accessed with this.context.mainCamera and this.context.renderer but I don't believe that should be necessary since they're already available in the callback.

@seppestaes
Copy link

Can you pass them to the DropInViewer?
const viewer = new DropInViewer({ selfDrivenMode: false, threeScene: scene, renderer: gl, camera: camera, ...

@hybridherbst
Copy link
Author

hybridherbst commented Nov 4, 2024

I tried that and it does not change the behaviour.
DropInViewer ignores those parameters since they are passed into onBeforeRender:

options.selfDrivenMode = false;
options.useBuiltInControls = false;
options.rootElement = null;
options.ignoreDevicePixelRatio = false;
options.dropInMode = true;
options.camera = undefined;
options.renderer = undefined;

static onBeforeRender(viewer, renderer, threeScene, camera) {
viewer.update(renderer, camera);
}

@seppestaes
Copy link

As far as i understand: if you use the DropInViewer, the Viewer won't create new ones (camera, renderer) and they need to be passed in the options.

https://github.com/mkkellogg/GaussianSplats3D/blob/f6f4adea2e0a5745f942767214c7d3b335a10751/src/Viewer.js#L1607C5-L1608C73

Can you check the update in Viewer?

@hybridherbst
Copy link
Author

As mentioned, they do get passed in by three.js in the onBeforeRender callback:

static onBeforeRender(viewer, renderer, threeScene, camera) {
viewer.update(renderer, camera);
}

I verified that the correct camera and renderer are passed into the update() call through the onBeforeRender callback. That's how DropInViewer works. I believe the issue here must be something else.

Can you check the update in Viewer?

As expected, passing in those options does not change the behaviour. I updated the StackBlitz repro:
https://stackblitz.com/edit/three-dev-n6xvbd?file=main2.js%3AL21

@seppestaes
Copy link

Screenshot 2024-11-05 at 16 57 31

You're right, they are present. Camera settings? Frustum culling?

@hybridherbst
Copy link
Author

If I knew I probably wouldn't have needed to open an issue :)

@mkkellogg wanted to take a look directly:

@mkkellogg
Copy link
Owner

Sorry for the lack of movement on this one, I've been terribly busy with other things. I hope I can devote some time to it soon.

@mkkellogg
Copy link
Owner

After looking at your sample for a while, there's no obvious reason why it should not be working. There must be something happening under the hood with your engine that is affecting the rendering of the splats, and there's no way for me to debug that.

@hybridherbst
Copy link
Author

hybridherbst commented Nov 11, 2024

Thanks for investigating. Needle is not doing anything special to the three.js renderer – it just adds objects to the scene graph. I'll see if I can find anything else.


EDIT: I found the issue! 🎉 Carefully comparing the uniforms passed to the drawcall, this zero was suspicious:

image

Turns out that Viewer/DropInViewer make a few assumptions that are not necessarily true:

  1. the rootElement can never be passed into DropInViewer, it's overwritten
  2. the rootElement is always the direct parent of the renderer.domElement
  3. the rootElement has the same size (!) as the renderer.domElement 🐛

I believe the latter is what breaks rendering here. The rootElement might have a different size, e.g. height = 0, and then no splats are visible because while the renderer has a specific size, the size of the parent element is used.

If I replace this line with this.renderer.domElement then rendering works correctly:

this.rootElement = this.renderer.domElement.parentElement || document.body;

Couple questions @mkkellogg:

  1. What's the reason why here the parentElement is used instead of the renderer.domElement?
  2. What would be a good fix? For example, if DropInViewer didn't overwrite rootElement, I could pass the renderer.domElement as rootElement to fix the issue.

I updated the StackBlitz page with vanilla three.js and DropInViewer and the broken case –
https://stackblitz.com/edit/three-dev-n6xvbd?file=main.js
This is enough to break rendering:

  // This breaks it, because then the assumptions made by Viewer about the rendered size are wrong
  const div = document.createElement('div');
  div.style.height = 1000000; // stretchy lines
  div.style.height = 1; // giant vertical lines, very slow
  div.style.height = 0; // nothing renders at all (as described in this issue)

  document.body.appendChild(div);
  div.appendChild(renderer.domElement);

@mkkellogg
Copy link
Owner

To be completely honest, I think I somewhat arbitrarily added the .parentElement at some point during the early days of development, and then never thought about it again 😄 (It possibly was for testing purposes, but I can't remember what I would have been testing) I actually have no specific reason for attaching the render to the DOM in that manner, so I think it could probably just go away?

I'll remove it and do some quick testing. Thank you for digging into this!

@mkkellogg
Copy link
Owner

I have a branch with what I think should fix the issue (along with some other updates) here: https://github.com/mkkellogg/GaussianSplats3D/compare/minor-updates

Want to try it out to see if it works for you?

@hybridherbst
Copy link
Author

Tested & looking good! Thank you!
Will you make a new (minor?) release with these changes?

@mkkellogg
Copy link
Owner

Yeah that's the plan!

@hybridherbst
Copy link
Author

Any news on that release so the issue can be closed? :)

@mkkellogg
Copy link
Owner

Well I was waiting on @jesse-small to verify that the changes in https://github.com/mkkellogg/GaussianSplats3D/compare/minor-updates matched what he had proposed in #354

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

No branches or pull requests

3 participants