Skip to content

Conversation

fran6co
Copy link
Contributor

@fran6co fran6co commented Aug 13, 2015

Rebased and squashed #261

fixed compilation; fixed segfaults in CpuDepthPacketProcessor; disabled timing
@floe
Copy link
Contributor

floe commented Aug 14, 2015

@larshg OK if I merge this one instead of your original PR? Your contributions are still part of the commit log, of course.

@fran6co fran6co force-pushed the glviewer branch 6 times, most recently from 41fcadc to 45e524e Compare August 14, 2015 13:51
@fran6co
Copy link
Contributor Author

fran6co commented Aug 14, 2015

I squashed some changes into larshg's commit for standardizing the timer and the inclusion of the opencv header.

@fran6co
Copy link
Contributor Author

fran6co commented Aug 14, 2015

Wouldn't it be better to remove the opencv dependency for good?

@fran6co fran6co force-pushed the glviewer branch 2 times, most recently from 07cc598 to 33b4dfe Compare August 14, 2015 15:13
@larshg
Copy link
Contributor

larshg commented Aug 14, 2015

@floe sure, as I don't have much time right now it's nice someone continues the work :-)

@fran6co fran6co changed the title A opengl viewer to use instead of opencv. Removes opencv dependency Aug 14, 2015
@fran6co
Copy link
Contributor Author

fran6co commented Aug 14, 2015

Removed opencv dependency, the only problem would be test_opengl_depth_packet_processor.cpp. But I think it needs it's own cmake rules

@floe
Copy link
Contributor

floe commented Aug 15, 2015

Could you add division by 4500.0f in the grayfragment shader, then I think we're good to go.

larshg and others added 2 commits August 15, 2015 09:58
Added define for opencv to be able to use either opencv or opengl.

Removed dublicate of flextGL .c/.h
@fran6co
Copy link
Contributor Author

fran6co commented Aug 15, 2015

I don't have the kinect at hand at the moment to test it out. Could you check if it's okay? https://github.com/OpenKinect/libfreenect2/pull/361/files#diff-8ca25d621f26a3215be9537b77208b46R59

@xlz
Copy link
Member

xlz commented Aug 16, 2015

The last commit introduces hard dependency on C++11. As discussed before in #261, we want to avoid this.

Can you add a wrapper like this?

double getTime()
{
#if __cplusplus >= 201103L
  //not tested:
  static auto start = std::chrono::high_resolution_clock::now();
  auto end = std::chrono::high_resolution_clock::now();
  auto duration = end - start;
  return std::chrono::duration_cast<std::chrono::duration<double>>(duration).count();
#elif LIBFREENECT2_WITH_OPENGL_SUPPORT
  return glfwGetTime();
#else
  return 0;
#endif
}

I guess you need to find a place for this function. Ultimately it should be in logging.h logging.cpp because logging is the only user of this function. But right now the logging PR #309 is unfinished, to be delayed after this PR. To avoid conflict, I think it's ok to temporarily put static double getTime() and the whole definition in libfreenect2/packet_processor.h. This creates a bit duplicate code, but the logging PR will promptly move it back into logging.cpp.

@fran6co fran6co force-pushed the glviewer branch 2 times, most recently from aa71965 to 271f9c9 Compare August 16, 2015 15:35
@fran6co
Copy link
Contributor Author

fran6co commented Aug 16, 2015

Made a new Timer class, when #309 is merged it can be moved to the logging source files.

@xlz
Copy link
Member

xlz commented Aug 16, 2015

The rest looks fine to me.

@fran6co
Copy link
Contributor Author

fran6co commented Aug 16, 2015

Now it's using the full resolution of chrono.

@xlz
Copy link
Member

xlz commented Aug 17, 2015

Tests OK on Intel/Linux/OpenCL.

@floe
Copy link
Contributor

floe commented Aug 18, 2015

Alright, last big one. I'm pretty certain it will still break in some contexts, but if we don't merge it now, this release will never happen :-)

floe added a commit that referenced this pull request Aug 18, 2015
Removes opencv dependency, add OpenGL viewer & own timer class
@floe floe merged commit f62332f into OpenKinect:master Aug 18, 2015
@floe floe mentioned this pull request Aug 18, 2015
@fran6co fran6co deleted the glviewer branch August 18, 2015 13:30
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.

5 participants