mosra / magnum-integration

Integration libraries for the Magnum C++11 graphics engine
https://magnum.graphics/
Other
99 stars 44 forks source link

OvrIntegration: Adapt to API changes in Oculus SDK 1.17.0 #26

Closed Squareys closed 6 years ago

Squareys commented 7 years ago

Hi @mosra !

Just came across this, this is not urgent, merge whenever you find the time.

Cheers, Jonathan

mosra commented 7 years ago

Thank you!

One problem I see immediately (maybe you remember): there was no easy way to get the latest SDK from Oculus website, so the CI is stuck on something very outdated. Any idea how to solve this (besides hosting it privately)?

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 50.0% when pulling 10d684bb8fd79bc972296d47a9abe9d31afdd17b on Squareys:ovr-sdk-1.17.0 into 3e4b285ef2ccf50c1e6a7fc834cc30e2cda9928e on mosra:master.

Squareys commented 7 years ago

@mosra Yup, see last commit. I wonder if the access token expires sometime... otherwise we can refresh it and hope for the cache staying valid? :P

Squareys commented 7 years ago

Well, alright, it seems to download something else it seems...

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 50.0% when pulling 6f9419b341cbbdd06bbe52d1adde13c28f61ed16 on Squareys:ovr-sdk-1.17.0 into 3e4b285ef2ccf50c1e6a7fc834cc30e2cda9928e on mosra:master.

mosra commented 6 years ago

Huh, this is still a thing? :D

Squareys commented 6 years ago

@mosra Yes, we didn't merge it, because we didn't manage to get the ovr sdk downloaded by the CI.

Squareys commented 6 years ago

Actually, now that I think of it, we wanted to try whether the download key expires somehow. Also, it looks like I need to add some lines to make it build the Debug libraries of the SDK.

mosra commented 6 years ago

Thanks. So, do I understand correctly that without these patches the integration doesn't work with latest SDK?

Since I have the magnum.graphics cloud thingy now, I could maybe host the SDK there and circumvent this problem, but I don't know how many license agreements that would break.

Squareys commented 6 years ago

Summarizing offline discussions:

Solution to Oculus SDK not being automatically downloadable anymore is to host some minimal archive required for the build and use that. To generate it, I added a python script with 869ca71. This is useful anyway, since LibOVR links static CRT by default and therefore is not compatible with all other magnum libraries (which link dynamic CRT by default) and we therefore need to patch the VS projects.

This will be merged once mosra finds the time after some important bigger changes in the main repository.

mosra commented 6 years ago

Random idea since I don't have time to set up ci.magnum.graphics yet (and won't have for the next x weeks): does the SDK provide some version define that could be used to make it temporarily work both with the currently still downloadable SDK and the latest one? Yes, it's ugly and all, but better than have this totally non-compilable with latest SDK -- I may have a need for a working version soon.

codecov-io commented 6 years ago

Codecov Report

Merging #26 into master will decrease coverage by <.01%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #26      +/-   ##
=========================================
- Coverage   49.41%   49.4%   -0.01%     
=========================================
  Files          12      12              
  Lines         425     423       -2     
=========================================
- Hits          210     209       -1     
+ Misses        215     214       -1
Impacted Files Coverage Δ
src/Magnum/BulletIntegration/MotionState.cpp 100% <0%> (ø) :arrow_up:
src/Magnum/BulletIntegration/DebugDraw.cpp 10.25% <0%> (+0.25%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b2a285f...d77584d. Read the comment docs.

mosra commented 6 years ago

this is not urgent, merge whenever you find the time.

You probably shouldn't have said that, because it took me almost 12 months to merge :D

Merged in https://github.com/mosra/magnum-integration/compare/4e45cd18624cf348437920999077f66e73dc9248...11b5740bced8bc319e0659dd10b8e43ddf8cf082, thanks a lot for continuing with the maintenance here. And for the very helpful Python script.

Squareys commented 6 years ago

it took me almost 12 months to merge

Better late than never 😆 Since nobody complained yet... :D

Thank you for merging! 🎉