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

Allow data to be accessed directly by the GPU #4

Closed
bialpio opened this issue Oct 1, 2020 · 6 comments
Closed

Allow data to be accessed directly by the GPU #4

bialpio opened this issue Oct 1, 2020 · 6 comments

Comments

@bialpio
Copy link
Contributor

bialpio commented Oct 1, 2020

Splitting @cabanier's question into a new issue:

Is there a way to directly push this data to the GPU? Having an intermediate JS buffer will create overhead (and more GC). If the data is going to be used by the GPU, it seems more appropriate to surface it there.

To me, the main question to answer here is: do we want the data to be accessible by the CPU as well? I'd expect so, because if not, we'll lose out on most (all?) use cases for physics.

@cabanier
Copy link
Member

cabanier commented Oct 1, 2020

Have you run any experiments of running physics with the CPU using this feature? If the performance is fine and it allows new experiences, we should have it.

@bialpio
Copy link
Contributor Author

bialpio commented Oct 1, 2020

The prototype I have as of now relies on the roundabout way of exposing the data to the GPU (i.e. manually calling gl.TexImage2D() on the buffer returned from the API) and I was consistently getting 30FPS on the Pixel 3 I'm using for testing (note that we're limiting FPS to 30 in our current ARCore implementation).

Another approach we could take here is to allow some kind of configuration knob for the devs to opt out of CPU access by extending XRSessionInit dictionary. Something like:

navigator.xr.requestSession('immersive-ar', {
  requiredFeatures: ['depth-sensing'],
  depthSensing: { gpuOnly: true }
});

UAs could treat the gpuOnly as a hint that would affect the API behavior - in GPU-only mode, the data and getDepth() could throw an InvalidStateError and the only way to access the depth buffer would be to go through XRWebGLBinding to get the opaque WebGLTexture. I think this would offer the devs flexibility to specify which use case they care about (or rather, that they don't care about CPU access), and we could choose the more efficient code path. As a bonus, adding this capability later would not be a breaking change (although it may mean that early adopters of the API that care only about GPU would use a less-efficient option).

@cabanier
Copy link
Member

cabanier commented Oct 1, 2020

I think you'd have to put the gpuOnly option in a requiredFeatures entry but otherwise this is reasonable.

@bialpio
Copy link
Contributor Author

bialpio commented Oct 1, 2020

Hm, I was going by the precedent set by the DOM Overlay feature, but we can iron out the specifics once I start drafting the specification for the API. I'll keep this issue open for the time being, we can keep the conversation going if there are still some concerns around efficient GPU access to the depth data.

@cabanier
Copy link
Member

cabanier commented Oct 1, 2020

Sorry, I overlooked the '|'. Your proposal is fine :-)

@bialpio
Copy link
Contributor Author

bialpio commented Jan 26, 2021

Closing, should be addressed by the latest explainer (by passing usagePreference at session creation).

@bialpio bialpio closed this as completed Jan 26, 2021
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

2 participants