search

tpaviot / oce

OpenCASCADE Community Edition (OCE): a community driven fork of the Open CASCADE library.
http://groups.google.com/group/oce-dev
GNU Lesser General Public License v2.1
811 stars 284 forks source link

Pull in Cocoa Code #333

Closed rainman110 closed 11 years ago

rainman110 commented 11 years ago

The folks at occt are working an a native cocoa implementation according to http://git.dev.opencascade.org/gitweb/?p=occt.git;a=commit;h=4fe56619213b38f852e55d0cff034b0ee9a5bfc4 . It would be cool if we'd pull this code and integrate it into oce.

tpaviot commented 11 years ago

I've been studying this patch for one week. I created a branch named tp/OSX-Cocoa including OCCT OSX patch, and modified some cmake files in order to compile the branch. Please @rainman110 test and report issues/feedbacks/patches.

There's something I still can't understand in this patch : although the commit messages tellls "added support for Cocoa OpenGL context", I can't see any #include <OpenGL/gl.h>which is known to be the way to use Cocoa OpenGL rather than X11 OpenGL implementation.

rainman110 commented 11 years ago

@tpaviot Indeed there seems to the correct apple includes, for example in OpenGl/OpenGl_GlCore11.hxx line 46,47 . Probably these includes were done before this patch was released. I will try your branch as soon I am back home this evening ;)

tpaviot commented 11 years ago

I can't find such header include in OpenGl/OpenGl_Core11.hxx. Which file (from which branch) do you refer to ? I actually modified original files with the following code:

#if defined(__APPLE__)
  #include "OpenGL/gl.h"
  #include "OpenGL/glu.h"
#else
  #include "GL/gl.h"
  #include "GL/glu.h"
#endif

but did not commit changes since it does not seem to work (gl.h, glu.h and glext.h are very different from Linux to OSX).

rainman110 commented 11 years ago

I referred to the "official" occt git: http://git.dev.opencascade.org/gitweb/?p=occt.git;a=blob;f=src/OpenGl/OpenGl_GlCore11.hxx;h=9a62a05dfdfb87eb7ec083752b12d10710c72930;hb=6852a4c09d0b4e3047fb9753f9fa73aef7b18105 They seemed to patch this is june or so...

tpaviot commented 11 years ago

ok, thanx, I didn't see that point. Strange that it's not in the same commit.

rainman110 commented 11 years ago

Apparently the header files were not synced, when upgrading to 6.5.4, because in the original source some of the headers are in src, and in oce they are all in inc. Could that explain, why we missed these code changes in oce?

dbarbier commented 11 years ago

No, I check that files are identical before removing them from inc (but of course, errors can occur when merging OCCT changes). These includes had been modified by 87f6ab2.

tpaviot commented 11 years ago

Ok, I got it. OCCT released 6.5.4 including some experimental code related to OSX Cocoa support. Without commit 87f6ab2, oce-0.11 can't compile and work on osx. I didn't check 6.5.4.

tpaviot commented 11 years ago

@rainmain110 With commit f2bb292, tp/OSX-Cocoa should compile. I did not test any Cocoa based gui though.

dbarbier commented 11 years ago

@tpaviot Is it possible to support both Cocoa and X11 implementations? I guess that MACOSX_USE_GLX had been introduced for that purpose, maybe you can replace #if defined(__APPLE__) by #if defined(__APPLE__) && !defined(MACOSX_USE_GLX) in this commit?

tpaviot commented 11 years ago

In the current OCCT git master branch, OpenGl_GlCore11.hxx does not define MACOSX_USE_GLX anymore. But in the meantime, we can leave it as an option, and add a new 'Experimental Cocoa OpenGL support' option(disabled by default) to the cmake builder. Commit amended 2685bf8.

dbarbier commented 11 years ago

Looks right. The #undef caused trouble previously, maybe you will have to protect them with #if !defined(MACOSX_USE_GLX) in order to build with X11.

rainman110 commented 11 years ago

Wow great! It seems that I hit a nerve ;) I will test it asap! I'll write a small qt based program to test, if I can bind the cocoa window into a qt widget.

rainman110 commented 11 years ago

I am wondering, if we'd need that opengl includes in all the cxx files (e.g. OpenGl_FontMgr.cxx), as many of them include OpenGl_Core11.hxx, which is meant to handle that platform specific opengl stuff. Thus we could end up including both OpenGL/gl.h and GL/gl.h accidentally (if we are not careful). The orginal occt code also seems to have these kind of problems.

dbarbier commented 11 years ago

@rainman110 I agree, we should include OpenGl_Core11.hxx instead of GL/gl includes. There is one exception, OpenGl_Context.cxx, which is special (and already handle Mac OS).

tpaviot commented 11 years ago

I had to commit 17b24e1 in order to get TKOpenGl compile on OSX/Cocoa (note that these changes comply with @rainman110 suggestion)

rainman110 commented 11 years ago

I just managed yesterday to compile oce, but couldn't really test it yet. It compiles fine, no problems. However, according to otool, libTKOpenGl.dylib is still linked against the X11 libraries Xmu, Sm, ICE, X11 and Xext. As @dbarbier said, we should either support X11 or Cocoa, not both. That will definitely cause problems. @tpaviot if you want, I could create the switch in cmake for native cocoa support and I would try to remove all X11 references.

rainman110 commented 11 years ago

It WORKS! Attached is the proof... tiglviewer

tpaviot commented 11 years ago

@rainman110 Great news. It would be great that you contribute a small samples program in the /examples directory. libTKOpenGl depends upon TKService, TKKernel, TKV3D and freetype. TKService depends upon Xw and is linked against X11 libs. We should remove the Xw dependency from TKService when using OSX/Cocoa. I ready tried to remove the 'Xw' line from adm/cmake/TKService/CMakeLists.txt but the compilation fails. I think some classes/methods of the Graphic3d package have to be protected.

rainman110 commented 11 years ago

Indeed, the class Graphic3d_GraphicDevice (which I am using) has still some X11 based code in it. That will probably change in future when occt for osx advances. My example program is based on the official qt examples in opencascade. I have currently no knowledge of cocoa/obj-c but I will try, to create a simple cocoa based example of the "bottle tutotial".

tpaviot commented 11 years ago

We need a Cocoa_GraphicDevice class in order to definitely drop X11. Anyway, it's not an emergency, the most important is already there: it's possible to use a Cocoa based GUI manager to handle a Graphic3d_GraphicDevice.

rainman110 commented 11 years ago

Thats true, currently it's more cosmetic! The application itself looks native. By the way, there is a GUI test program ViewerTest_ViewerCommands which is more or less a good example of using the Cocoa port. Are these tests build in oce?

tpaviot commented 11 years ago

Please review commits 1d2110672c3bdc2cda0425c00868172b10825425 and bc611ba36ff486e5f6ca24d53af4d8dfe6f98d5f.

rainman110 commented 11 years ago

looks correct but have not tested it yet...

rainman110 commented 11 years ago

A small update: The folks at OCCT are reworking TKOpenGL. It looks like we will get rid of X11 for good. For example they removed the class Graphic3d_GraphicDevice. Some small API changes seem to be necessary however. http://git.dev.opencascade.org/gitweb/?p=occt.git;a=shortlog;h=refs/heads/CR23712_1

rainman110 commented 11 years ago

Hi @tpaviot . I am just wondering, what your workflow is, to pull in changes from official occt git. I want to incorporate a commit in my fork, that removes all X11 dependencies on OSX . However, this commit consists of many files and i am wondering, how you sync oce with official occt on new releases. Do you have a occt git access or are you just copying their files?

tpaviot commented 11 years ago

@rainman110 I don't have any access to the official OCCT git repository since I did not sign their CLA. Here is how I proceeded to include OSX/Cocoa related patch :

  1. go to the gitweb interface
  2. click the related commit
  3. select/copy all the text
  4. paste to a text editor (Sublime Text here)
  5. modify the patch so that all .hxx header files point to the /inc directory
  6. save as ...
  7. check that the patch can apply (git apply --check the_patch.diff)
  8. modify the patch if needed (if the patch can't apply)
  9. apply the patch
  10. commit/edit the log message to copy/paste the original log

This surely could be automated with bash/curl/grep.

rainman110 commented 11 years ago

Thanks for the infos @tpaviot! Sounds like a lot of manual work with probably lots of conflicts. But I will try it as you described. When you switch to a new occt release (lets say 6.5.5 or 6.6.0), do you do the same or do you copy the complete code and apply oce related patches?

tpaviot commented 11 years ago

@dbarbier takes care of applying oce patches over occt releases. Nobody asked him so far how he proceeds, I have no idea about the amount of work required.

dbarbier commented 11 years ago

@rainman110 I tried to describe in https://github.com/tpaviot/oce/wiki/Git-workflow how I proceed. Please ask on the mailing-list if you need more info.