Skip to content

Fix a few issues around readTexture and iOS camera #1132

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

Merged
merged 5 commits into from
Sep 7, 2022

Conversation

Alex-MSFT
Copy link
Contributor

@Alex-MSFT Alex-MSFT commented Sep 7, 2022

This PR fixes a few issues related to the native camera implementation on iOS and readPixels

  1. The current implementation for readTextureAsync assumes that the frame number will always increment by 1 when DeviceImpl::Frame is called. This would be true if we only ever call bgfx::frame in DeviceImpl::Frame, but we have at least two other call sites for bgfx::frame, so this is not a true assumption. In general this implementation was prone to the queue getting blocked, so I'm making it a bit more lenient. (See Properly handle readTextureAsync cancellation in Native Capture #1131 for a related issue).
  2. NativeCameraImpl was not creating a new ImplData object when calling reset() which meant that the unique pointer was invalid on subesquent NativeCamera open calls, and leading to an AV.
  3. On Apple devices when initializing a video texture in landscape mode on we were never creating the textureRGBA object, because the resolution from the camera already matched the initial camera dimensions, so I am explicitly setting the textureRGBA value to null when calling Open and validating that we have a valid texture at the start of updateCameraTexture.
  4. iOS was using dispatch_sync when creating video textures, this can cause deadlocks on iOS if already running on the main thread or if the main thread is blocked on the current thread. Changed this to dispatch_async instead.

@Alex-MSFT Alex-MSFT merged commit a4fcade into BabylonJS:master Sep 7, 2022
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.

3 participants