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

Improve render order / sorting for a-frame scenes with multiple splats #20

Open
kfarr opened this issue Oct 23, 2023 · 13 comments
Open
Assignees
Labels
enhancement New feature or request

Comments

@kfarr
Copy link
Contributor

kfarr commented Oct 23, 2023

As an a-frame developer adding multiple gaussian splats to a scene, I would like splats to naturally occlude each other as though they were placed in real space together.

Issue: By default for transparent objects, A-Frame will use the DOM position to determine render order. This results in scenes where certain camera angles will show an object in front of another even though that object is further away and should be occluded. [picture1]

Workaround: As a scene creator I can carefully arrange entity dom order and entity position / camera position to arrange a scene to look "correct" from specific angles. However this is not a solution for immersive / vr as the user can easily change orientation to see views that do not look correct.

Instead: as a user when I add multiple splats to a scene I should see the rendering of the splats sorted to look natural as though they were part of the same scene

I attempted to solve this problem with using A-Frame sorting for transparent objects. It solves the problem in some contexts but also results in undesired behavior such as entities "popping" in or out of visibility quickly. [video1]

Some other possible solutions:

  • Combine splat data together from all splats in the scene for the purpose of sorting for rendering. A-Frame has the concept of a "system" to link multiple components together. This can be a mechanism to allow fetching the splats together and batching the sorting to happen just 1 time per frame instead of sorting multiple times for separate entities which may also result in performance improvement.
  • ?

picture1:
image

video1:
https://github.com/quadjr/aframe-gaussian-splatting/assets/470477/66474fe6-1911-4012-a9de-f25dc9c00a8b

@quadjr
Copy link
Owner

quadjr commented Oct 26, 2023

Awesome work!
I'll check the concept of "system". I think I can support it on few days.

@quadjr quadjr self-assigned this Oct 26, 2023
@quadjr quadjr added the enhancement New feature or request label Oct 26, 2023
@kfarr
Copy link
Contributor Author

kfarr commented Nov 9, 2023

Related issue: mkkellogg/GaussianSplats3D#37

@kfarr
Copy link
Contributor Author

kfarr commented Nov 10, 2023

So here is some psuedo code of how this might work:

  • at the moment each splat sends its points to the worker to be sorted during the component tick lifecycle method: https://github.com/quadjr/aframe-gaussian-splatting/blob/main/index.js#L594
  • if there are multiple entities each with separate gaussian_splatting components, they will each be doing their own sorting without considering the points of each other
  • Instead, there could be a system defined such that the single tick of the system runs a sorting algorithm once, instead of each component sorting separately.
  • However it should do the single sorting only after combining the splats together especially including applying translations that may have been applied from the a-frame entity / child relationship (three.js object3d inheritance) to place all together in the proper world position
  • therefore upon each system tick you have to know the camera position and the world position and rotation of each splat in the scene (this relates to this comment in the system documentation https://aframe.io/docs/1.4.0/core/systems.html#gathering-all-components-of-a-system)
  • I am probably capable of doing the boilerplate work to convert a-frame component to system, or at least to get some progress there, however then I could use help to understand how to combine splats together for the purposes of sorting.

@arthurmougin
Copy link

arthurmougin commented Dec 7, 2023

Working out the system concept, I get to this understanding :

  • Centralizing the sorting is not better if the rendering is done at the component level. Reason is that we move from a multithreaded pattern (one component one worker) to a single threaded one, and, at best, the sorted data a component will receive from the system will be identical to what it's own worker could have provided

  • Centralizing the sorting AND the rendering could solve the transparency issue, as we use just one shader. Though as we get one shader, vertex count, texture and buffer size could become be a bottleneck

  • Other option would be to work on the shader to better handle depth buffer and transparency. The reason for this one is seeing how just switching depthWrite to true improve non recursive occlusion

what do you all think about it ?

@kfarr
Copy link
Contributor Author

kfarr commented Dec 7, 2023

Thanks @arthurmougin

Other option would be to work on the shader to better handle depth buffer and transparency. The reason for this one is seeing how just switching depthWrite to true improve non recursive occlusion

I am intrigued by this, there may be some very "low hanging fruit" to modify transparency shader since depth buffer appears to be respected now but in a binary fashion (on / off) instead of gradient

@arthurmougin
Copy link

depth buffer appears to be respected now but in a binary fashion (on / off) instead of gradient

Looking at online discussions, depth testing is literally binary information, telling a shader to render, or not, a certain pixel.

Other online discussions suggest Order Independent Transparency as a solution (not natively supported by threejs) and donmccurdy give an alternative through AlphaHash with significant visual artifacts, though.

image
(Exaggerated effect for example, you just need to move the camera in the example link to get that effect)

Another approach, in which the example looks very similar to our depthWrite:true scenario, would be to discard based on a threshold.

My current guess is that discarding could help improve the result obtained in this pr.

@arthurmougin
Copy link

Wow, I got closer than I thought with just changing the scene tree.
I previously had the seal and the train splat as both root of the scene, and no luck having one or the other not to mask the other.

By just putting the seal as the child of the train, and setting depthWrite:true on at least the seal, I get a very interesting composition that works on every angle.
image

Now the discarding of the black blobs seems to be the only thing missing !

@arthurmougin
Copy link

image

Discarding in the fragment shader seems to have improved drastically the artifacts, but it also impacts the details.

void main () {
	float A = -dot(vPosition, vPosition);
	if (A < -4.0) discard;
	float B = exp(A) * vColor.a;
	if(B < 0.2) discard;  // new discard recently added
	gl_FragColor = vec4(vColor.rgb, B);
}

I think that, just as depthWrite, it should be set on a per-splatting basis.

Higher numbers mean less obvious borders, but fewer splats overall, leading to reduced quality and longer time to populate the splat to a decent level at load time.

B < 0.5
image

One thing is now clear to me, we must differentiate visual sorting artifacts and cloudy (not treated) splattings. And raising the discard filter will only hide ghost blobs for so long before hiding valuable onces. So it's, as always, a trade-off balance and author must choose.

@kfarr
Copy link
Contributor Author

kfarr commented Dec 11, 2023

@arthurmougin I did some testing and wanted to share my results

First here is a comparison of using the version in this PR (left side) with default settings and the current version (right side) with default settings:
image

They are materially different and since it's a big change from the default setting before this PR, I think there should be a way to have this change only be enabled with setting depthWrite: true, or another recommended mechanism. That said, I think this is more than sufficient to meet the need, I just want to make sure there is still an easy option for users to have the existing rendering style.

Here is another example of the rendering tradeoff. The left-side has artifacting on the bottom of the flower box with correct occlusion, whereas the right-side has less artifacting but incorrect occlusion:
occlusion-depthwrite-true-left-false-right

Here is an example of using discardFilter to adjust the size of the artifacts. On the left side is discardFilter: 0.1 and right side discardFilter: 0.3 . On the right side the artifacting on the base of the flower box is less pronounced but in some areas the surface is disintegrating
discardFilter-0point1-left-0point3-right

My next step:

  • Create a scene with cropped splats mixed with polygon ground surfaces

@arthurmougin
Copy link

I agree with your take, discardFilter help cleanup artifacts that become super obvious as soon as the depth buffer is written into, but that has an impact on the clarity of the individual splat.

Diving into the shader and Buffer Geometry reminded me of another Aframe component, that is somewhat underrated imho : aframe-geometry-merger-component

What it does, is put multiple geometries into one, to reduce draw call. But in our case, performance might not be the only impact, and that's what I wanted to test here. This scene represents the seal behind the corner of the train.

Code for left :

      <a-entity gaussian_splatting="src: https://huggingface.co/cakewalk/splat-data/resolve/main/train.splat;depthWrite:true;discardFilter:0.1" rotation="0 0 0" position="0 1.5 -2">
        <a-entity gaussian_splatting="src: https://huggingface.co/quadjr/aframe-gaussian-splatting/resolve/main/luma-seal.splat;depthWrite:true;discardFilter:0.1"  rotation="0 0 0" position="-3.3 -0.33 -1.7"></a-entity>
      </a-entity>

Code for right :

      <a-entity geometry-merger="preserveOriginal: false">
        <a-entity gaussian_splatting="src: https://huggingface.co/cakewalk/splat-data/resolve/main/train.splat;depthWrite:true;discardFilter:0.1" rotation="0 0 0" position="0 1.5 -2"></a-entity>
        <a-entity gaussian_splatting="src: https://huggingface.co/quadjr/aframe-gaussian-splatting/resolve/main/luma-seal.splat;depthWrite:true;discardFilter:0.1"  rotation="0 0 0" position="-3.3 1.17 -3.7"></a-entity>
      </a-entity> 

image

Looking at the result, maybe a global system, not for sorting, but just centralizing the geometry, could help improve on the quality of blending between splats.

In the case of this scene, you can see that both options has its advantages and limitations. Maybe a more tailored merge system would allow us to reach better results ?

@kfarr
Copy link
Contributor Author

kfarr commented Dec 12, 2023

@arthurmougin here are some more things to test and suggestion for next steps:

I created a test repo with cropped and cleaned up splats mixed with low poly mesh. Here is a page to test:
https://3dstreet.github.io/splat-playground/ (use wasd to move around)
And the repo which uses the latest version of the splatting library with the PR you have added.
https://github.com/3DStreet/splat-playground

It seems to work well for the intended purpose that this ticket was created for! Yay!
image

However, I noticed that the splats do not render in iOS safari anymore. This older example does, so I'm guessing something in the recent changes have affected iOS:
https://kfarr.github.io/aframe-gaussian-splatting/compositing-demo.html

I will try to set up safari remote debugging to see what error is displayed on ios.

I would suggest that getting the existing demo working in ios as well as an option to turn on / off return to original style rendering is most important for now. Then we can leave the door open for future improvements via merged geometry, etc.

@kfarr
Copy link
Contributor Author

kfarr commented Dec 12, 2023

The error that I get on vision pro safari debugging is:
> Unhandled Promise Rejection: TypeError: undefined is not an object (evaluating 'data.body')

Presumably line 248 from index.js
const reader = data.body.getReader();

@kfarr
Copy link
Contributor Author

kfarr commented Dec 20, 2023

The iOS error has been resolved and #25 is ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants