Renewed PULL for merging the branches by CKehl · Pull Request #2 · miragetech/osgAndroid

CKehl · 2016-08-31T16:27:46Z

Dear osgAndroid team,

we've been mailing each other around half a year ago. Thing is that back then, I made one, big commit and it's hard to cluster/de-cluster all the changes.

On the commit after that (c178f04) I reset back to your old main branch (last commit I had from y'all) (commit: e49ad0d; Fixed Jordi Torres name in the readme). So, from commit c178f04 onwards I commit each change piece-by-piece so you can merge that calmly and easily get back to me if there are errors in the meanwhile.

Kind Regards,

Christian

- osg Image Reader/Writer - osg Node Reader/Writer - Off-Screen Rendering (using Pixel Buffer Objects) - wrapper for Vec2/Vec3/Vec4 and their Arrays - extended wrapper for the Matrix - basic classes to construct a renderable object Mesh (Geode, Geometry, ...) - wrapped native functions for texture projection and Raycasting intersection

…works for GLES1 and GLES2

modified: jni/JNIosg.cpp expanded wrapped C++ functionality (functions) in the osg Core domain of the following classes: Node - setTexture - setPointSize - setLineWidth Group - addChild - removeChild - getNumberOfChildren - getChild Camera - setClearColorVec - getClearColorVec - getViewMatrix Vec2 (new) - dispose, create - x,y - set, setX, setY - length, length2 - div, dotProduct, scalarProduct, sum, sub - normalize - negation Matrix - inverse, transpose - scale, mult - get, getTranslation, getRotation - [pre/post]MultVec[3/4] Image - getRed/Green/Blue/Alpha - s,t

modified: src/org/openscenegraph/osg/core/Camera.java modified: src/org/openscenegraph/osg/core/Group.java modified: src/org/openscenegraph/osg/core/Image.java modified: src/org/openscenegraph/osg/core/Matrix.java modified: src/org/openscenegraph/osg/core/Node.java expanded Java functionality (functions), which C++ function calls are previously added, in the osg Core domain of the following classes: Node - setTexture - setPointSize - setLineWidth Group - addChild - removeChild - getNumberOfChildren - getChild Camera - setClearColorVec - getClearColorVec - getViewMatrix Vec2 (new) - dispose, create - x,y - set, setX, setY - length, length2 - div, dotProduct, scalarProduct, sum, sub - normalize - negation Matrix - inverse, transpose - scale, mult - get, getTranslation, getRotation - [pre/post]MultVec[3/4] Image - getRed/Green/Blue/Alpha - s,t

new file: src/org/openscenegraph/osg/core/Array.java new file: src/org/openscenegraph/osg/core/DrawArrays.java new file: src/org/openscenegraph/osg/core/DrawElementsUInt.java new file: src/org/openscenegraph/osg/core/Drawable.java new file: src/org/openscenegraph/osg/core/Geode.java new file: src/org/openscenegraph/osg/core/Geometry.java new file: src/org/openscenegraph/osg/core/Object.java new file: src/org/openscenegraph/osg/core/PrimitiveSet.java new file: src/org/openscenegraph/osg/core/PrimitiveSetList.java new file: src/org/openscenegraph/osg/core/Texture.java new file: src/org/openscenegraph/osg/core/Texture2D.java new file: src/org/openscenegraph/osg/core/Vec2.java new file: src/org/openscenegraph/osg/core/Vec2Array.java modified: src/org/openscenegraph/osg/core/Vec3.java new file: src/org/openscenegraph/osg/core/Vec3Array.java modified: src/org/openscenegraph/osg/core/Vec4.java new file: src/org/openscenegraph/osg/core/Vec4Array.java added a large collection of Java classes to interface via Android SDK; because the osg::Object is now wrapped, a minor adaptation had to be done for Vec3 and Vec4 in the equals() method

modified: jni/JNIosgViewer.cpp new file: jni/screenview.cpp new file: jni/screenview.h Added newly wrapped C++ functions to allow for off-screen rendering on Android. The screenview class is adapted from the public OSG example. The viewer, formerly just being JNIviewer, is now separated into Viewer and OffScreenViewer, which both inherit from ViewerBase.

modified: src/org/openscenegraph/osg/viewer/OSGRenderer.java new file: src/org/openscenegraph/osg/viewer/OffScreenViewer.java modified: src/org/openscenegraph/osg/viewer/Viewer.java new file: src/org/openscenegraph/osg/viewer/ViewerBase.java Added newly JNI functions and classes to allow for off-screen rendering on Android. The viewer, formerly just being JNIviewer, is now separated into Viewer and OffScreenViewer, which both inherit from ViewerBase. NOTE: just ran an ndk build - errors came up as the normally distributed OSG version apparently doesn't have a RefVec2 (just the way it provides one for Vec3 and Vec4). I know I added it locally to an OSG build (only thing I changed is defining the RefVec2) and submitted it to osg-submissions mailing list. please check in the osg trunk of 3.4 or later if it's there for you - OR: add it as external definition is the JNI headers here. Don't know what's more handy.

modified: jni/Android.mk modified: jni/JNIUtils.h modified: jni/JNIosg.cpp BUG SOLVED: Found the RefVec2 error - I forgot to add it to the JNIUtils (done now). Please check your osg installation if ReferenceType<osg::Vec2> is defined. Also added the screenview.h/cpp to Android.mk to make the OffScreenViewer working. Removed an absurdly large test function for vertex coloring so to remove the request for an interpolation function (added soon). ndk-build compiles now again without errors.

modified: jni/JNIUtils.h Added Interpolate() function

modified: jni/JNIosgDB.cpp Added support (in the C++ wrappers) for using the plugins automatically. Also provided functions for ReadImage, WriteNode and WriteImage.

modified: src/org/openscenegraph/osg/db/ReadFile.java new file: src/org/openscenegraph/osg/db/WriteFile.java Added Java-callable classes (or functions) to read or write a node or an image.

modified: jni/Android.mk modified: jni/JNIosgDB.cpp BUG SOLVED: changed the log message of osgDB to avoid conflicts occurring from the recent addition. Also reverted the Android.mk OSG_HEAD definition to the <path-to....> one.

new file: LICENSE new file: Readme.md Changes not staged for commit: deleted: .classpath deleted: .project Added LICENSE and Readme file as given in the main trunk, and added my name to the contributors. MERGE COMPLETE

Conflicts: org.openscenegraph.android/jni/Android.mk org.openscenegraph.android/jni/JNIosg.cpp org.openscenegraph.android/jni/JNIosgViewer.cpp All conflicts fixed but you are still merging. Changes to be committed: modified: org.openscenegraph.android/Readme.md modified: org.openscenegraph.android/jni/Android.mk modified: org.openscenegraph.android/jni/Application.mk modified: org.openscenegraph.android/jni/JNIUtils.h modified: org.openscenegraph.android/jni/JNIosg.cpp modified: org.openscenegraph.android/jni/JNIosgUtil.cpp modified: org.openscenegraph.android/jni/JNIosgViewer.cpp modified: org.openscenegraph.android/src/org/openscenegraph/osg/util/GeometryUtils.java modified: org.openscenegraph.android/src/org/openscenegraph/osg/viewer/Viewer.java Changes not staged for commit: deleted: org.openscenegraph.android/.classpath deleted: org.openscenegraph.android/.project Merging old head with reverted new history - hope that works on remote

modified: Readme.md deleted: org.openscenegraph.android/LICENSE deleted: org.openscenegraph.android/Readme.md removed confusion on doubled LICENSE and Readme.md

jtorresfabra · 2016-09-01T07:15:26Z

Hi @CKehl,

Thank you very much for taking the time to redo your PR, we know it has been a hard effort. We will be reviewing it in the next few days.

Cheers!

jtorresfabra · 2016-09-06T13:03:10Z


+### Static preparation
+OSG_HEAD:=<path-to-osg-gles1-sdk> 
+#OSG_HEAD:=/media/christian/DATA/android-osg-sdk/gles1


Remove comments with fixed paths

will be done

jtorresfabra · 2016-09-06T13:07:34Z

@CKehl as a general comment, we were waiting different PR's for different features, not different commits. this way it is easy to use the github interface searching for changes in pull requests. But ok, we will try to review it as is.

jtorresfabra · 2016-09-06T13:13:39Z

 typedef ReferencedType<osg::Vec4> RefVec4;
 typedef ReferencedType<osg::Quat> RefQuat;
+
+inline osg::Vec4 Interpolate(osg::Vec2 _texcoord, osg::Image* _img)


shouldn't it be "const osg::Vec2 &_texcoord"? Also we tend to use underscore for member class variables, not for parameters.

I agree Jordi: for efficiency reasons, it can be "const osg::Vec2 &texcoord" - I just needed this function quickly, so I wasn't that picky with the reference. The underscoring: noted and will be changed.

CKehl · 2016-09-06T13:16:28Z

Op 06-09-16 om 15:07 schreef Jordi Torres:

@CKehl https://github.com/CKehl as a general comment, we were
waiting different PR's for different features, not different commits.
this way it is easy to use the github interface searching for changes
in pull requests. But ok, we will try to review it as is.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFvds2HdtVJ6crTgT6Q7b8SEmf1cHozZks5qnWWWgaJpZM4Jxzpn.

Okay, I didn't know how it works (new to it). It would be nice if you
can try review it with the different commit. Otherwise, I need to create
a new branch from the main, roll back the whole main and redo all the
changes again with separate PRs.

How do you go now about the commenting ? should I just erase the
fixed-path comments in the future ?

jtorresfabra · 2016-09-06T13:19:27Z

Hi @CKehl,

The idea is that you fix yourself your branch, it automatically appears fixed in the PR.

Thanks.

jtorresfabra · 2016-09-06T13:21:44Z

+}
+
+/*
+JNIEXPORT jlong JNICALL Java_org_openscenegraph_osg_core_PrimitiveSet_nativeCreatePrimitiveSet(JNIEnv *, jclass, jint type, jint mode)


Better if we do not leave dead code, if it does not work/serve anymore is better to remove it.

CKehl · 2016-09-06T13:27:49Z

Op 06-09-16 om 15:19 schreef Jordi Torres:

Hi @CKehl https://github.com/CKehl,

The idea is that you fix yourself your branch, it automatically
appears fixed in the PR.

Thanks.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFvds9fbp9RetIMCFaKrMrN-aiy_-q0yks5qnWhggaJpZM4Jxzpn.

Hi,

well, that was not the way I meant it. I guess I understand you
correctly that you want to have a single pull request for each tested
and final version of a feature of my branch. What you now have is one PR
with different commits per feature, tested and working. Sorry for that.

jtorresfabra · 2016-09-06T13:30:12Z

+    getSize(_gc, width, height);
+    if (width!=_width || _height!=height)
+    {
+        std::cout<<"   Window resized "<<width<<", "<<height<<std::endl;


rather to use osg::notify mechanism

okay, will be adapted

jtorresfabra · 2016-09-06T13:37:10Z

Hey @CKehl,

https://help.github.com/articles/about-pull-requests/

So "Once you've created a pull request, you can push commits from your
topic branch to add them to your existing pull request. These commits will
appear in chronological order within your pull request and the changes will
be visible in the "Files changed" tab."

So the only thing you need is to keep adding commits with the fixes to your
PR. It should be enough.

Does it make sense for you?

2016-09-06 15:27 GMT+02:00 CKehl notifications@github.com:

Op 06-09-16 om 15:19 schreef Jordi Torres:

Hi @CKehl https://github.com/CKehl,

The idea is that you fix yourself your branch, it automatically
appears fixed in the PR.

Thanks.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2 (comment),

or mute the thread
<https://github.com/notifications/unsubscribe-
auth/AFvds9fbp9RetIMCFaKrMrN-aiy_-q0yks5qnWhggaJpZM4Jxzpn>.

Hi,

well, that was not the way I meant it. I guess I understand you
correctly that you want to have a single pull request for each tested
and final version of a feature of my branch. What you now have is one PR
with different commits per feature, tested and working. Sorry for that.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABs8RMTW2FuTjXK7vnYbqevuUv0CnP4Cks5qnWpWgaJpZM4Jxzpn
.

Jordi Torres

jtorresfabra · 2016-09-06T13:38:41Z

+
+                getSize(gc, _width, _height);
+
+                std::cout<<"Window size "<<_width<<", "<<_height<<std::endl;


rather use osg::notify.

already noted.

CKehl · 2016-09-06T13:40:21Z

Well seem's at least I have a little read ahead of me :-) But in
essence: yes, got that.

Op 06-09-16 om 15:37 schreef Jordi Torres:

Hey @CKehl,

https://help.github.com/articles/about-pull-requests/

So "Once you've created a pull request, you can push commits from your
topic branch to add them to your existing pull request. These commits will
appear in chronological order within your pull request and the changes
will
be visible in the "Files changed" tab."

So the only thing you need is to keep adding commits with the fixes to
your
PR. It should be enough.

Does it makes sense for you?

2016-09-06 15:27 GMT+02:00 CKehl notifications@github.com:

Op 06-09-16 om 15:19 schreef Jordi Torres:

Hi @CKehl https://github.com/CKehl,

The idea is that you fix yourself your branch, it automatically
appears fixed in the PR.

Thanks.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub

#2 (comment),

or mute the thread
<https://github.com/notifications/unsubscribe-
auth/AFvds9fbp9RetIMCFaKrMrN-aiy_-q0yks5qnWhggaJpZM4Jxzpn>.

Hi,

well, that was not the way I meant it. I guess I understand you
correctly that you want to have a single pull request for each tested
and final version of a feature of my branch. What you now have is one PR
with different commits per feature, tested and working. Sorry for that.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub

#2 (comment),
or mute the thread

https://github.com/notifications/unsubscribe-auth/ABs8RMTW2FuTjXK7vnYbqevuUv0CnP4Cks5qnWpWgaJpZM4Jxzpn
.

Jordi Torres

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFvds438gtxuSsS7bQpur4xHYTN8tOMLks5qnWyHgaJpZM4Jxzpn.

rafagaitan · 2016-09-08T17:02:09Z

+	if(node != NULL)
+		node->getOrCreateStateSet()->setAttributeAndModes(lw, osg::StateAttribute::ON);
+}
+


I'm wondering if it would be better to have StateSet abstraction with those methods (including the nativeSetMode), maybe, for now this is ok, but I will open a issue for tracking this change soon.

Rafa, I agree. But messing with the StateSet wrapping is something I wanted to avoid for now - especially as I usually rather work with geometry, hence more often work with the node concept.

modified: jni/Android.mk Fixed removing comments from Android.mk

…tter functions

cleared the large TextureFromPose-functions.

…ears

removed underscored variable names, unnecessary comments and notifications for viewers

modified: screenview.h FINAL FIX: adhered to osg::notify mechanism and remove void statements, pointed out by Jordi.

comment on the changes: the problem why the viewer didn't work was due to the new 3rdParty library setup, which now appears to work like a charm. An expected error will be raised ONCE with the OpenSceneGraph head, as it lacks the following functions: inline void set( value_type x, value_type y ) { _v[0]=x; _v[1]=y; } inline void set( const Vec2f& rhs) { _v[0]=rhs._v[0]; _v[1]=rhs._v[1]; } (to be found in <OSG_TOP>/include/Vec2f) this needs to be patched (currently: manually). Changes to be committed: modified: org.openscenegraph.android/jni/Android.mk modified: org.openscenegraph.android/jni/Application.mk modified: org.openscenegraph.osgsimple/src/org/openscenegraph/osgsimple/OSGActivity.java

Changes to be committed: modified: org.openscenegraph.android/jni/Android.mk

jtorresfabra · 2016-10-20T11:31:30Z

Hey @CKehl, is this ready for the last review and merge?

CKehl · 2016-10-20T11:39:14Z

yes, it is. Just tested it yesterday.
There is one function not available in the main OpenSceneGraph trunk
concerning the Vec2(f) - it lacks a setter which I needed to insert locally.

Apart from that, it's dead-simple:
Have an OpenSceneGraph trunk (I used 3.3.7 and 3.3.8) ready, compiled
with plugins for gles1 and gles2 and for armeabi and armeabi-v7a. An
example script that I used (here GLES1 armeabi):

mkdir build_gles1_obj_armeabi && cd build_gles1_obj_armeabi
cmake ..
-DCMAKE_TOOLCHAIN_FILE=../PlatformSpecifics/Android/android.toolchain.cmake
-DDYNAMIC_OPENTHREADS=OFF -DDYNAMIC_OPENSCENEGRAPH=OFF
-DANDROID_NATIVE_API_LEVEL=8 -DANDROID_ABI=armeabi
-DCMAKE_INSTALL_PREFIX=${OSGANDROID_TARGET}/gles1/armeabi
-DOSG_CPP_EXCEPTIONS_AVAILABLE=ON
-DOSG_GL1_AVAILABLE=OFF -DOSG_GL2_AVAILABLE=OFF -DOSG_GL3_AVAILABLE=OFF
-DOSG_GLES1_AVAILABLE=ON -DOSG_GLES2_AVAILABLE=OFF
-DOPENGL_PROFILE="GLES1"
-DOSG_GL_LIBRARY_STATIC=OFF -DCURL_DIR=../3rdparty/curl
-DGDAL_DIR=../3rdparty/gdal/include
-DGIFLIB_INCLUDE_DIR=../3rdparty/giflib
-DJPEG_INCLUDE_DIR=../3rdparty/libjpeg
-DPNG_INCLUDE_DIR=../3rdparty/libpng
-DTIFF_INCLUDE_DIR=../3rdparty/libtiff
-DZLIB_INCLUDE_DIR=../3rdparty/zlib
make -j 6 install
cd ..
rm -Rf build_gles1_obj_armeabi

then modify the paths according to your installation in the Android.mk
of org.openscenegraph.android/jni.
ndk-build

And you can test it with the osgsimple application you already provided
with the whole package.

On 20. okt. 2016 13:31, Jordi Torres wrote:

Hey @CKehl https://github.com/CKehl, is this ready for the last
review and merge?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFvds4FHy3dGuIzwDiZ4m_rMC4fT5YM8ks5q11ETgaJpZM4Jxzpn.

CKehl added 18 commits January 12, 2016 18:37

changed Android.mk to the <path-to-osg-gles1-sdk> variant

ce4db79

added license and contribution information

b690df5

added new compilation files that include 3rdParty libraries and that …

c178f04

…works for GLES1 and GLES2

added ignore rule for temporary gedit backup files (Ubuntu text editor).

3930590

Changes to be committed:

2d6375f

modified: jni/JNIUtils.h Added Interpolate() function

Changes to be committed:

86e4f5f

modified: jni/JNIosgDB.cpp Added support (in the C++ wrappers) for using the plugins automatically. Also provided functions for ReadImage, WriteNode and WriteImage.

Changes to be committed:

044a9e9

modified: src/org/openscenegraph/osg/db/ReadFile.java new file: src/org/openscenegraph/osg/db/WriteFile.java Added Java-callable classes (or functions) to read or write a node or an image.

Changes to be committed:

4c6eb22

modified: jni/Android.mk modified: jni/JNIosgDB.cpp BUG SOLVED: changed the log message of osgDB to avoid conflicts occurring from the recent addition. Also reverted the Android.mk OSG_HEAD definition to the <path-to....> one.

Changes to be committed:

c5d3420

new file: LICENSE new file: Readme.md Changes not staged for commit: deleted: .classpath deleted: .project Added LICENSE and Readme file as given in the main trunk, and added my name to the contributors. MERGE COMPLETE

Changes to be committed:

1cb6a06

modified: Readme.md deleted: org.openscenegraph.android/LICENSE deleted: org.openscenegraph.android/Readme.md removed confusion on doubled LICENSE and Readme.md

jtorresfabra reviewed Sep 6, 2016
View reviewed changes

rafagaitan reviewed Sep 8, 2016
View reviewed changes

CKehl added 10 commits September 15, 2016 15:01

Changes to be committed:

ba1b51e

modified: jni/Android.mk Fixed removing comments from Android.mk

stripped underscored variables and unnecessary comments from JNIosg.cpp

9f9b3a9

Added early-fail termination via "if(var==NULL) return;" for array se…

cf58fa0

…tter functions

removed the as<SuchAndSuch> wrapper functions in JNIosg.cpp

e5e1d41

osg.Core change requests completed.

86d7a82

cleared the large TextureFromPose-functions.

Fixed the in-function defined, which are not needed anymore as it app…

a0bd03e

…ears

modified: JNIosgViewer.cpp

ca7e184

removed underscored variable names, unnecessary comments and notifications for viewers

modified: screenview.cpp

a9f89dc

modified: screenview.h FINAL FIX: adhered to osg::notify mechanism and remove void statements, pointed out by Jordi.

cleared local paths from the Android.mk again

2e67617

Changes to be committed: modified: org.openscenegraph.android/jni/Android.mk


		getSize(gc, _width, _height);

		std::cout<<"Window size "<<_width<<", "<<_height<<std::endl;

Conversation

CKehl commented Aug 31, 2016

Uh oh!

jtorresfabra commented Sep 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtorresfabra commented Sep 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CKehl commented Sep 6, 2016

Uh oh!

jtorresfabra commented Sep 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CKehl commented Sep 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtorresfabra commented Sep 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CKehl commented Sep 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtorresfabra commented Oct 20, 2016

Uh oh!

CKehl commented Oct 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jtorresfabra commented Sep 6, 2016 •

edited

Loading