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

LibOVRIntegration #3

Merged
merged 14 commits into from Jun 21, 2015

Conversation

Projects
2 participants
@Squareys
Contributor

Squareys commented Jun 7, 2015

Hi @mosra, Hi everybody,

Since this definitely will be a thing now, let's move discussion of LibOVRIntegration to here. Previous discussion can be found in mosra/magnum-examples#10.

If you are okay with it, only the most important compositing layer (ovrLayerEyeFov) will be wrapped for now and I will create an issue which lists the missing wrappers and implement them when needed.

Current TODOs:

  • Finish wrapping compositor.
  • Cleanup code.
    • Cleanup code formatting.
    • Change // comments to /* comments */.
    • Change MagnumLibOvrIntegration_EXPORTS to be all uppercase.
    • Remove #endif description comments.
    • Add copyrights properly.
    • Fix posef test.
    • Use #include<memory>.
    • Add enumset operators.
    • Fix ovrSwapTextureSet* getOvrSwapTextureSet() being a raw pointer.
    • Change case of MAGNUM_LibOvrIntegration_EXPORT.
    • Refactor context.
    • Use absolute includes.
  • Rebase to cleanup commit history.
    • Squash fixups.
  • Finish doc++.
  • Finish FindOVR.cmake.
@mosra

This comment has been minimized.

Owner

mosra commented Jun 7, 2015

Thanks a lot for this effort. Great stuff, as I said already before.

Just a few things before you proceed further so you don't have to fix too much stuff later:

  • To match naming policy of the rest of the project, I'd like to have LibOvrIntegration instead of LibOVRIntegration (you already did use Hmd instead of HMD which is great). But keep the FindOVR.cmake as is so it matches name of the DLL. Changing filename and directory case is rather tricky with Git on Windows, I'm able to help with that if you need.
  • There's no need to have void in parameter-less function signatures (like Layer::layerType(void)). IIRC, this is some leftover from C, where the distinction actually had some meaning (but it doesn't in C++ and shorter is better).
  • The inline keyword is implicit and thus redundant for functions defined inside class body -- little known feature and I don't blame you for not knowing it (but again, shorter is better).
  • Please use either (const) references or std::unique_ptr instead of raw pointers and manual deletion, particularly when returning heap-allocated instances from functions to avoid accidental memory/resource leaks. The array of Texture2D pointers in SwapTextureSet is a tricky one, I have something in mind, but that needs to have some library support from my side.

That's for now, hopefully I didn't overwhelm you with the requests (sorry) :)

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 7, 2015

To match naming policy [...]

I agree, I will just do a soft reset instead of a rebase then an recommit everything with the renamed directories, shouldn't be a problem.

The inline keyword is implicit and thus redundant for functions defined inside class body

Good to know, thank you.

Please use either (const) references or std::unique_ptr instead of raw pointers

In the cases where order of deletion matters, is it "clean" to use unique_ptr.reset() to delete?

That's for now, hopefully I didn't overwhelm you with the requests (sorry) :)

No problem, I learn alot from this kind of feedback :)

One thing regarding includes: I stated having problems with QTCreator showing me installed includes of LibOvrIntegration instead of the local ones so I am using relative imports now. Any idea what I could do about this?

@mosra

This comment has been minimized.

Owner

mosra commented Jun 7, 2015

I agree, I will just do a soft reset instead of a rebase then an recommit everything with the renamed directories, shouldn't be a problem.

The workaround that did the job for me was to rename in two steps (two commits that would be squashed later), first renaming the files/directories to completely different names and then back with proper case. Manual recommiting is not needed.

In the cases where order of deletion matters, is it "clean" to use unique_ptr.reset() to delete?

Can you point me to the code where this is the case? Could reordering the members help? (they are destructed in reverse order of declaration, last declared one is destructed first)

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 7, 2015

Can you point me to the code where this is the case?

https://github.com/Squareys/magnum-examples/blob/ovr-example/src/ovr/OVRExample.cpp#L190-L193 Was the code snippet I had in mind, can be solved through reordering, though, as you pointed out.

@mosra

This comment has been minimized.

Owner

mosra commented Jun 7, 2015

Added the infrastructure for converting quaternions (mosra/magnum@94c4225), dual quaternions (mosra/magnum@52434fb) and ranges (mosra/magnum@562d71d) so you can finish the conversion utilities. The usage should be similar to what you have already done (see the test cases for hints). Funnily enough the new code revealed a GCC bug, so it took me much longer than I would like.

If I understand correctly, the ovrPosef class is just an orientation + position, so using dual quaternion instead of a matrix might be better idea in terms of memory efficiency and performance (and also way simpler conversion).

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 7, 2015

If I understand correctly, the ovrPosef class is just an orientation + position, so using dual quaternion instead of a matrix might be better idea [...]

Yes, that's right. I never used dual quaternions before, but I'll figure it out.

Added the infrastructure for [...]

Thanks! That'll make things alot cleaner.

I need to concentrate on my studies for a bit, so sorry in advance that there will be a little pause in progress, I hope to get back to this next weekend, though, maybe I'll even find some time in between aswell.

@mosra

This comment has been minimized.

Owner

mosra commented Jun 7, 2015

Sure, you did amazing amount of work already :)

You don't need to understand the internal structure of dual quaternions to use them, they can be multiplied just like matrices and created from quaternion/translation pair like this:

Quaternion rotation;
Vector3 translation
DualQuaternion a = DualQuaternion::translation(translation)*DualQuaternion{rotation};

Conversion to matrices is then done via the obvious toMatrix(). Also, for the example, scene graph can (but doesn't need to be) be templated on dual quaternion type instead of matrices, just use SceneGraph::DualQuaternionTransformation instead of SceneGraph::MatrixTransformation3D.

@Squareys Squareys force-pushed the Squareys:libovr branch from bf2437f to af21bcc Jun 8, 2015

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 8, 2015

Alright, I think I the code should now pretty much respect your requests. The test for converting the DualQuaternions still fails, though. (The Texture2D** _textures; is still there, also.)

What is your opinion on the includes? I had had problems with QtCreator opening the installed headers instead of the local ones, so includes of LibOvrIntegration headers are currently all relative.

Also, I should probably add some more tests for the other code aswell. I saw there is a AbstractGLTester or similiar in Magnum. That might come in handy.

else()
unset(MAGNUM_${_COMPONENT}INTEGRATION_LIBRARY)
endif()
endif()

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

Looks like you mixed up indentation with tabs and spaces by accident.

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

Indeed, code formatting has not been done yet, though.

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(OVR OVR_LIBRARY OVR_LIBRARIES OVR_INCLUDE_DIR)
message(STATUS "OVR_FOUND = " ${OVR_FOUND})

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

These two status messages shouldn't be needed (find_package_handle_standard_args() prints a message on success/failure anyway).

}} /* namespace Magnum::LibOvrIntegration */
#endif /* Magnum_LibOvrIntegration_Conversion_h */

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

Yes, I know that QtCreator adds these comments at namespace ending braces and #endif automatically, but I'd prefer to not have them, as it adds another burden when doing refactoring/file renaming later. Similarly elsewhere.

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

Actually, I added those :D And, yes, I also had to change it a few times already. In that case I will remove them again.

}
protected:
ovrLayer_Union _layer;

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

(This is also rather QtCreator's fault -- I was unable to persuade it to do otherwise myself.)

I'm indenting the public:/private: access specifiers with 4 spaces and the contents inside with another 4 spaces. It might be considered wasteful, but I already did that for whole project, so just for consistency. Similarly elsewhere.

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

Indeed, Qt did that, but again, I did not do code formatting yet. (See Todos above)

* @author Jonathan Hale (Squareys)
*/
#include <bits/unique_ptr.h>

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

#include <memory> ;)

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

I hoped to get rid of some big includes that way. I guess that is not recommended?

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

Yeah, I know what you mean, but sadly this won't work in libc++ (OSX) and with MSVC (both have unique_ptr defined right in <memory> header).

}
/** @brief The underlying ovrSwapTextureSet. */
ovrSwapTextureSet* getOvrSwapTextureSet() const {

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

I would return reference here (assumption is that the member is never nullptr anyway).

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

I think there was a reason why I left this one, but I'll check again.

This comment has been minimized.

@Squareys

Squareys Jun 10, 2015

Contributor

There wasn't, I just need to keep the _swapTextureSet as a pointer, because ovrHmd_CreateSwapTextureSetGL() expects ovrSwapTextureSet**.

@brief Hmd tracking capabilities flags
*/
typedef Containers::EnumSet<HmdStatusFlag> HmdStatusFlags;

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

There should be also the following lines:

CORRADE_ENUMSET_OPERATORS(HmdCapabilities)
CORRADE_ENUMSET_OPERATORS(HmdTrackingCapabilities)
CORRADE_ENUMSET_OPERATORS(HmdStatusFlags)

Otherwise code like HmdCapability::LowPersistence|HmdCapability::NoVSync wouldn't compile.

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

Ah, good to know, I wondered why that didn't work already.

*/
#include <Magnum/Magnum.h>
#include "Conversion.h"

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

The purpose of forward declaration headers is to be as small as possible to have no negative impact on compilation times (just forward declarations and nothing else), so I'd remove the #include "Conversion.h" line.

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

Makes sense, will do.

#include <Corrade/Utility/VisibilityMacros.h>
#ifdef MagnumLibOvrIntegration_EXPORTS
#define MAGNUM_LibOvrIntegration_EXPORT CORRADE_VISIBILITY_EXPORT

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

Sorry, I'm now really nitpicking, but rather MAGNUM_LIBOVRINTEGRATION_EXPORT here (macros (except header guards) are always uppercase to be distinguishable from other stuff).

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

Alright, I request change of https://github.com/mosra/magnum-integration/blob/master/src/Magnum/BulletIntegration/visibility.h for consistency then ;)

I would have chosen it to be in all uppercase anyway, i just though, if it's that way in BulletIntegration, it would probably have a reason.

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

Um... (yeah, sorry, it's complicated), the MagnumLibOvrIntegration_EXPORTS/MagnumBulletIntegration_EXPORTS macro is provided by CMake, its name is hardcoded to <library-filename>_EXPORTS and I can't do much about that.

I meant the MAGNUM_LibOvrIntegration_EXPORT one, which is then used in Compositor.h etc.

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

Ah, I see, sorry. What's with include guards? They should match case of folder an filenames, right?

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

Yup :)

namespace Magnum { namespace Math { namespace Implementation {
// ovrSizei

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

Please use /* .. */ comments rather than // ... (except in Doxygen blocks, because these comments cannot be nested).

(Explanation: I have such policy that I'm using // ... exclusively for quick-and-dirty coding (commenting-out some part of code, short-lived todo/fixme comments etc.) so when I see it in a commit, it subconsciously triggers a warning that there is something that shouldn't be here.)

This comment has been minimized.

@Squareys

Squareys Jun 9, 2015

Contributor

Alright, I think there are alot more of these, I will go through all the code again anyway, so changing the comments is not that much more work anyway.

if (a.y != b.y) return false;
if (a.z != b.z) return false;
if (a.w != b.w) return false;
return true;

This comment has been minimized.

@mosra

mosra Jun 9, 2015

Owner

Well, thinking about these, I think it might be better to drop the operators and do the comparisons directly in the test cases, as it would do proper fuzzy comparison for floats, will verbosely print what's wrong when the test fails and won't break if next release of Oculus SDK adds the comparison operators itself. So in the test case below:

ovrQuatf c{a};
CORRADE_COMPARE(c.x, b.x);
CORRADE_COMPARE(c.y, b.y);
CORRADE_COMPARE(c.z, b.z);
CORRADE_COMPARE(c.w, b.w);

You can call the CORRADE_COMPARE macro in a loop, so that would work fine also in the matrix case.

@mosra

This comment has been minimized.

Owner

mosra commented Jun 9, 2015

Okay, sorry again about annoying coding style comments :)

I'll look at the includes, it works for me without a problem in other projects, so maybe there's something wrong just in this repo. How's the test failing? (Maybe I was wrong about how the dual quaternion should be initialized.)

The AbstractOpenGLTester currently doesn't work on Windows (winapi requires me to pass some data through the constructor and I need to work around that somehow), will try to fix that soon.

I have one last note about the LibOvrContext class. It apparently needs to be some sort of a singleton, but I think it could be done without the initialize()/finalize() methods. Looking at the example, it's now like this:

``` cpp`
// LibOvrContext is created on heap statically before entering main()
{
LibOvrContext::get()->initialize();

// code that accesses the context through LibOvrContext::get() ... 

LibOvrContext::get()->finalize();

}


I propose changing that to the following:

``` cpp
{
    LibOvrContext _context; // as a private member of the application class

    // (unchanged) code that accesses the context through LibOvrContext::get() ... 

    // _context gets automatically destroyed when application is destructed
}

The static _instance variable will be initialized to nullptr, the constructor then will point it to itself (and assert that there was no other instance before), destructor sets it back to nullptr (and assert that it was equal to this) and get() method would return a reference instead of pointer (and assert that static instance is not nullptr). Should be a bit easier for the end user.

@mosra

This comment has been minimized.

Owner

mosra commented Jun 9, 2015

Just checked the absolute includes and they appear to work in QtCreator on Windows (e.g. ctrl-clicking on #include "Magnum/LibOvrIntegration/Conversion.h" inside Conversion.cpp file was working), if you install them to the same location as the rest of Magnum then they should work also when installed.

Huh... it looks like the first two commits still have the LibOVRIntegration directory name, case insensitive filesystems are painful :)

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 9, 2015

Okay, sorry again about annoying coding style comments :)

I think I got used to it already ;)

How's the test failing?

.translation() on the instance does not give me the values I set in the first place with DualQuaternion::translation.

I have one last note about the LibOvrContext class. [...]

I had remembered that ovr_Initialize() needed to be called before any OpenGL call. That was removed in the current sdk version, though. And in this case your suggestion is alot cleaner, of course.

In your suggestion though, even accessing _context directly from Application would work, right?

Huh... it looks like the first two commits still have the LibOVRIntegration directory name, case insensitive filesystems are painful :)

Oh, well. I am on Linux currently, I might just cleanup the directories here later then.

Just checked the absolute includes [...]

Ctrl-clicking did work, yes, but it kept sending me to the installed files, causing a bit of confusion when I edited them and the local files did not change. I will try changing back the includes to be absolute and see if that problem persists.

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 9, 2015

Folder renaming complete, I guess the squashing should happen under linux aswell.

@mosra

This comment has been minimized.

Owner

mosra commented Jun 9, 2015

.translation() on the instance does not give me the values I set in the first place with DualQuaternion::translation.

I think this is related to the fact that the rotation quaternion you are using in the test case is not normalized. Try this instead:

DualQuaternion{Quaternion{{0.1, 0.2, 0.3}, 1.0}.normalized()}

I need to look into the math again, apparently I completely forgot everything about dual quaternions :) I should also put some assertion somewhere to catch this case.

I had remembered that ovr_Initialize() needed to be called before any OpenGL call.

Even before creating GL context? Huh... Also, doesn't the SDK mess somehow with GL state so it needs to be accounted for on our side using Context::resetState()? Or are they reverting back all the state that they touch (slow...)?

In your suggestion though, even accessing _context directly from Application would work, right?

Yup. Best case of course would be if there is no need for the global instance.

it kept sending me to the installed files

Well, yeah :/ I have this exact problem with KDevelop, somehow system-wide includes are more important than the local ones. I'll try to investigate, maybe it is somehow related to order of include directories in CMake.

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 9, 2015

I think this is related to the fact that the rotation quaternion you are using in the test case is not normalized.

Yeah, pretty sure that's it. As far as I know, non-normalized quaternions cannot be used for rotations.

Even before creating GL context?

You might have missed the next sentence in which I explained that this not the case in the current SDK version anymore and won't be so in the future.

Yup. Best case of course would be if there is no need for the global instance.

Well, it doesn't really make sense to have more than one context, also that would probably mess up the ovr_Initialize() and ovr_Shutdown() calls. So, I would keep it as a singleton.

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 10, 2015

Some comments got lost, since they where attatched to diffs:

@mosra wrote:

Please use /* .. */ comments rather than // ... (except in Doxygen blocks, because these comments cannot be nested).

(Explanation: I have such policy that I'm using // ... exclusively for quick-and-dirty coding (commenting-out some part of code, short-lived todo/fixme comments etc.) so when I see it in a commit, it subconsciously triggers a warning that there is something that shouldn't be here.)

I think, this probably needs an update: http://mosra.cz/blog/corrade-doc/corrade-coding-style.html#corrade-coding-style-documentation

@mosra wrote:

Well, thinking about these, [the ovr structure comparison operators] I think it might be better to drop the operators and do the comparisons directly in the test cases, as it would do proper fuzzy comparison for floats, will verbosely print what's wrong when the test fails and won't break if next release of Oculus SDK adds the comparison operators itself. So in the test case below:

ovrQuatf c{a};
CORRADE_COMPARE(c.x, b.x);
CORRADE_COMPARE(c.y, b.y);
CORRADE_COMPARE(c.z, b.z);
CORRADE_COMPARE(c.w, b.w);

You can call the CORRADE_COMPARE macro in a loop, so that would work fine also in the matrix case.

Alright.

@mosra

This comment has been minimized.

Owner

mosra commented Jun 10, 2015

I think, this probably needs an update: http://mosra.cz/blog/corrade-doc/corrade-coding-style.html#corrade-coding-style-documentation

I know, yeah, it's confusing a lot, really, and it's bugging me all the time, but I was unable to persuade Doxygen to accept anything else -- writing /** /* */ */ completely breaks syntax highlighting in any editor (even though Doxygen somehow accepts that). That would be somehow bearable, though (abandon all editor sanity and autocompletion benefits), but writing /** */ inside Doxygen @code block is even more impossible because Doxygen then tries to parse it as an actual documentation block and that part then magically gets lost from the docs completely :D

I'll try to do something about it, though.

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 10, 2015

I'll try to do something about it, though.

You could use /// /* */ as an exception for documenting documentation.

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 10, 2015

So, should
int s; //< short line.
be
int s; /*< short line. */
?

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 19, 2015

@mosra Alright, will do.

I'm currently wrapping another compositor layer for debugging (LayerDirect), and it turns out, that all of the methods needed are already implemented in LayerEyeFov.
I could let LayerEyeFov inherit from LayerDirect, but then the method chaining would get messed up (calls to setViewport() for example would return a reference to LayerDirect instead of LayerEyeFov when called from an instance of the later.)
What would you recommend?

  • Is it okay to have a LayerEyeFov method return a reference to LayerDirect "for method chaining", or
  • is that so bad that you would recommend code duplication (or using macros to define those methods 2-5 times in different subclasses of Layer)
  • Or templates? Could it work defining the six method templates and then creating the required instances of those templates in the Layer subclasses?
  • Or maybe just remove method chaining for Layer?

The inheritance doesn't really serve any more purpose than not having to rewrite the methods.

@Squareys Squareys force-pushed the Squareys:libovr branch from c756fff to 55d4521 Jun 19, 2015

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 19, 2015

While I was at it, I implemented all the layer types after all. I went with the code duplication solution, since inheritance is not actually desirable with the layers and there weren't that many shared methods overall.

And back to the example.

@Squareys Squareys force-pushed the Squareys:libovr branch from 55d4521 to b35a226 Jun 19, 2015

@Squareys Squareys changed the title from [WIP] LibOVRIntegration to LibOVRIntegration Jun 19, 2015

@Squareys Squareys force-pushed the Squareys:libovr branch from a01b7b3 to b80d234 Jun 19, 2015

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 19, 2015

@mosra I declare this ready for review aswell :)

You may want to cherry-pick the bullet doc fix (I could also create a new pull request for that, but it seemed a little overkill for one line change), to keep things clean. I can then rebase this onto master make it automatically mergable again.

@mosra

This comment has been minimized.

Owner

mosra commented Jun 20, 2015

No need to be so strict about pull requests, I'll merge (or fast-forward) everything in one go.

@Squareys Squareys force-pushed the Squareys:libovr branch 2 times, most recently from 70bdfc7 to b605009 Jun 20, 2015

Squareys added some commits Jun 8, 2015

LibOvrIntegration: Add conversions.
A function to wrap ovrTexture as Texture2D, aswell as onverters and their
corresponding tests for:
 - ovrSizei,
 - ovrVector2i,
 - ovrVector2f,
 - ovrVector3f,
 - ovrMatrix4f,
 - ovrQuatf,
 - ovrPosef,
 - ovrRecti

Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Add visibility definitions.
Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Add enums.
Add `HmdType`, `HmdCapability`, `HmdTrackingCapability` and
`HmdStatusFlag`.

Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Add Hmd and SwapTextureSet.
Hmd wraps `ovrHmd_ConfigureTracking()`, `ovrHmd_CreateMirrorTextureGL()`,
`ovrHmd_DestroyMirrorTexture()`, `ovrHmd_SetEnabledCaps()`

SwapTextureSet wraps `ovrHmd_CreateSwapTextureSetGL()`,
`ovrHmd_DestroySwapTextureSet()`

Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Add Compositor, Layer, LayerEyeFov and LayerType.
Compositor wraps `ovrHmd_SubmitFrame()` and creates layers for the libOVR
compositor.

Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Add LibOvrContext.
LibOVRContext ist responsible for creating `Hmd` and owns the only instance
of `Compositor`. It wraps `ovrHmd_Detect()`, `ovrHmd_CreateDebug()`,
`ovrHmd_Create()`, `ovr_Initialize()`, `ovr_Shutdown()`.

Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Add forward declaration header.
Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Add to CMake project files.
Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Implement remaining layer types.
Add LayerDirect, LayerFovDepth, LayerQuad.

Signed-off-by: Squareys <Squareys@googlemail.com>
BulletIntegration: Fix Doc++
Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Use frameIndex and 'new' way to get hmd pose
Use `ovr_CalcEyePoses()` instead of `ovrHmd_GetEyePoses()` as
suggested by [Migrating from SDK 0.5 to SDK 0.6](https://developer.oculus.com/documentation/pcsdk/latest/concepts/release-migration/).

Signed-off-by: Squareys <Squareys@googlemail.com>
LibOvrIntegration: Wrap ovrMatrix4f_*Projection().
Wrap `ovrMatrix4f_Projection()` and `ovrMatrix4f_OrthoSubProjection()`
as methods of `Hmd`.

Signed-off-by: Squareys <Squareys@googlemail.com>

@Squareys Squareys force-pushed the Squareys:libovr branch from b605009 to 7cc9c77 Jun 21, 2015

@Squareys

This comment has been minimized.

Contributor

Squareys commented Jun 21, 2015

@mosra Okay, ready for review/merge once again.

LibOvrIntegration: Cleanup const in method parameters.
Omit the `const` in method declarations for doxgen output, and use
pass-by-reference where plausible.

Signed-off-by: Squareys <Squareys@googlemail.com>

@mosra mosra merged commit 94bc2a7 into mosra:master Jun 21, 2015

@Squareys Squareys deleted the Squareys:libovr branch Jul 28, 2015

@mosra mosra added this to the 2018.02 milestone Feb 15, 2018

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