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

Incomplete support for VRExternal (Firefox Reality). #32

Merged
merged 1 commit into from Jan 21, 2019

Conversation

Projects
None yet
2 participants
@paulrouget
Copy link
Contributor

paulrouget commented Jan 3, 2019

This adds support for Firefox Reality. It's incomplete because:

  1. the rendering part is not supported (submit frame doesn't work yet)
  2. it's missing the servo glue (WIP here: servo/servo@master...paulrouget:immersive)

I'd like to land that for 2 reasons:

  1. get early feedback
  2. being able to start landing stuff in servo, Firefox Reality, and Mozilla Central. rust-webvr is the first piece.
@paulrouget

This comment has been minimized.

Copy link
Contributor Author

paulrouget commented Jan 3, 2019

@paulrouget paulrouget force-pushed the paulrouget:vrexternal branch 3 times, most recently from ee26b57 to 9dd8b7f Jan 7, 2019

Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
@paulrouget

This comment has been minimized.

Copy link
Contributor Author

paulrouget commented Jan 11, 2019

In sync_poses, I block, but I do not copy the state. That means when synced_frame_data and inmediate_frame_data are called, I use the system mutex before reading the shared memory.

Is that fine, or should I copy the state in sync_poses and use that copy in synced_frame_data and inmediate_frame_data?

@MortimerGoro

This comment has been minimized.

Copy link
Collaborator

MortimerGoro commented Jan 11, 2019

@paulrouget It's better to copy the state in sync_poses and use that state to generate synced_frame_data and inmediate_frame_data results.

@paulrouget

This comment has been minimized.

Copy link
Contributor Author

paulrouget commented Jan 14, 2019

To be clear:

  • in sync_poses, I block and copy the system state
  • in synced_frame_data and inmediate_frame_data I reuse the copied state

Questions:

  • in synced frame data, I just call inmediate_frame_data. Is that ok? If so, why is there 2 different functions for the same thing?
  • in sync_poses, I need to block with cond variable. The condition for waiting according to Gecko's code is: 1) mLastSubmittedFrameId has been incremented or 2) mSuppressFrames is true or 3) mIsConnected is false. Then just copy and unblock. And if mPresentingGeneration, I unblock and exit. Is that right? Do I need to do anything special if 2) or 3)?
  • And what about sensorState.inputFrameId? Does it need to be checked in sync_poses or in synced_frame_data?
@MortimerGoro

This comment has been minimized.

Copy link
Collaborator

MortimerGoro commented Jan 14, 2019

in synced frame data, I just call inmediate_frame_data. Is that ok? If so, why is there 2 different functions for the same thing?

inmediate_frame_data() was created for the "magic window mode" in WebVR 1.1 API, retrieving the current sensor data without having to enter WebVR session and submit frames. I think magic window has changed in WebXR API so maybe we can revisit this API and simplify/unify the methods if it makes sense cc @Manishearth.

Related, we could also unify sync_poses() and synced_frame_data() if you find them confusing or for WebXR API. The only reason to split them was to send the near_z/far_z as fast as possible without delays (they z values could change while sync_poses is blocked because on Servo I dispatched JS RAF and sync_poses in parallel), but it shouldn't be a big deal because they are normally set once.

in sync_poses, I need to block with cond variable. The condition for waiting according to Gecko's code is: 1) mLastSubmittedFrameId has been incremented or 2) mSuppressFrames is true or 3) mIsConnected is false. Then just copy and unblock. And if mPresentingGeneration, I unblock and exit. Is that right? Do I need to do anything special if 2) or 3)?

That's correct. For 2 and 3 you just need to skip the block. Nothing else should be required

And what about sensorState.inputFrameId? Does it need to be checked in sync_poses or in synced_frame_data?

You can just check mLastSubmittedFrameId as Gecko does. inputFrameId is going to be changed at the same time so both checks are going to be equivalent right now. They are splitted because Gecko supports some API to queue frames but we are not using it in Firefox Reality.

@paulrouget paulrouget force-pushed the paulrouget:vrexternal branch 2 times, most recently from 1905d1c to e2bb210 Jan 16, 2019

@paulrouget

This comment has been minimized.

Copy link
Contributor Author

paulrouget commented Jan 16, 2019

@MortimerGoro I addressed your comments, added some more logic, and updated moz_external_vr.h. r?

Show resolved Hide resolved rust-webvr/src/api/vrexternal/display.rs Outdated
data.left_projection_matrix = proj(left_fov);
data.right_projection_matrix = proj(right_fov);

data.timestamp = utils::timestamp();

This comment has been minimized.

@MortimerGoro

MortimerGoro Jan 16, 2019

Collaborator

You can use timestamp from VRHMDSensorState::timestamp in the external memory struct

Show resolved Hide resolved rust-webvr/src/api/vrexternal/mod.rs Outdated
Show resolved Hide resolved rust-webvr/src/api/vrexternal/service.rs Outdated
@paulrouget

This comment has been minimized.

Copy link
Contributor Author

paulrouget commented Jan 18, 2019

@MortimerGoro I must be missing something, enumerationCompleted is always false here.

@paulrouget

This comment has been minimized.

Copy link
Contributor Author

paulrouget commented Jan 21, 2019

@MortimerGoro I must be missing something, enumerationCompleted is always false here.

My fault. I was resetting the system state on engine switch.

@paulrouget paulrouget force-pushed the paulrouget:vrexternal branch from e2bb210 to 26ef88d Jan 21, 2019

@paulrouget paulrouget force-pushed the paulrouget:vrexternal branch from 26ef88d to 87fe8cd Jan 21, 2019

@paulrouget

This comment has been minimized.

Copy link
Contributor Author

paulrouget commented Jan 21, 2019

@MortimerGoro I addressed your comments. It appears to work ok here (as in - a threejs webvr follows head motions).

@MortimerGoro MortimerGoro merged commit 4e30999 into servo:master Jan 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment