bjornblissing / osgoculusviewer

An OsgViewer with support for the Oculus Rift
Other
106 stars 67 forks source link

OculusDevice improvements #105

Closed rickyviking closed 3 years ago

rickyviking commented 3 years ago
  1. exposed full ovrHmdDescription: this is needed for instance to distinguish which Oculus model is currently in use
  2. added methods to modify near/far planes after creating the Oculus device: this allow an application to modify the value at runtime, depending on the loaded scene. Should be safe as the values are used on a frame basis to compute the projection matrix.
bjornblissing commented 3 years ago
  1. Even though I can see the benefit of this, I do have some (minor) objections to exposing the full ovrHmdDescription struct. My goal is to hide as much of the internal Oculus implementation as possible. As of now the only exposed OculusSDK structs are in the code below. These could probably be wrapped as well in the future. So I would rather have functions to access the relevant fields in the ovrHmdDescription struct and make them public as builtin C++ data types. https://github.com/bjornblissing/osgoculusviewer/blob/4c8a3d92f25e3698bfbcf76c10e3775f875ed321/include/oculusdevice.h#L81-L93

  2. This on the other hand seems sensible.

rickyviking commented 3 years ago

I see. The fact is that also the Type field of the description I'm reading is in turn an enum belonging to the Oculus SDK. Wrapping that enum (which clearly belongs to the SDK) seems a bit of a stretch to me. Let me know what you think, in case I can update the PR with the second point only.

bjornblissing commented 3 years ago

I split the changes into: 1f8ec5ed30dbde9e2aded4ddf6018593dcd67dfb 962690e09f5ea5df61c0ffd310090633d0bf5c44