Bug 743 - Quaternion patch
Summary: Quaternion patch
Status: RESOLVED FIXED
Alias: None
Product: Jogl
Classification: JogAmp
Component: opengl (show other bugs)
Version: 2
Hardware: All all
: --- enhancement
Assignee: Sven Gothel
URL:
Depends on:
Blocks:
 
Reported: 2013-06-01 22:20 CEST by Petr Skramovsky
Modified: 2013-06-12 12:49 CEST (History)
2 users (show)

See Also:
Type: ---
SCM Refs:
jogl f5fe8439f52acaf4bc2afe04531f1aa8ca30390b jogl eaf15f8020bdd97f7d919721c9cc74eb72e6c493 jogl 0a4defdfe478c3e231f3b1255cba6989aa2c489e jogl 08bfa5797069cb5757620e74b8befaa079d04ddb jogl a368548b20321ea5cdace6cc495e632ca9d5c99c jogl 85b96a891bb81776faf0a6dd1783443a045f74fb
Workaround: ---


Attachments
patch (18.65 KB, application/octet-stream)
2013-06-01 22:20 CEST, Petr Skramovsky
Details
demo (4.93 KB, application/zip)
2013-06-01 22:23 CEST, Petr Skramovsky
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Petr Skramovsky 2013-06-01 22:20:41 CEST
Created attachment 469 [details]
patch

i would like to submit this patch, because i have tried to use quaternion class for my camera but i wasn't able to do it.
so sum of changes:
- reformatted
- method for init from axis vector and angle
- method for transform vector
- few minor things
what i'm not sure of:
- naming
- maybe more/better comments
Comment 1 Petr Skramovsky 2013-06-01 22:23:17 CEST
Created attachment 470 [details]
demo

.zip containing a very simple quaternion camera class + demo program
Comment 2 Xerxes Rånby 2013-06-02 22:55:10 CEST
Thank you Peter for pushing this Quaternion enhancement patch as 4 individual commits to github:
https://github.com/petr-s/jogl/commit/ba8f322de06204afe4c7cb81e51a1d46477bb586 - new method for vector multiplication, new copy constructor 

https://github.com/petr-s/jogl/commit/1fe8fa0765f13980cd4498cb65ab2e36908e75b1 - ixed isIdentity method, deprecated isEmpty method (quaternion doesn't have such a property + method do same thing as isIdentity), new setIdentity method, default constructor sets this quaternion to identity, new fromAxis method/costructor

https://github.com/petr-s/jogl/commit/0f9e158d89d2971d505373409e4ab56699e66eea -  removed unnecessary castings, removed unnecessary methods for vector operations changed to VectorUtil instead

https://github.com/petr-s/jogl/commit/81c911541690f573b631974212f0e49d4d83daba - reformatted to same style
Comment 3 Harvey Harrison 2013-06-04 20:49:40 CEST
Petr,

Your patches look fine, I had some trivial comments that I made in your github repo, reproduced below.

+    public void fromAxis(float[] vector, float angle) {
+        float sin = FloatUtil.sin(angle *= 0.5f); 



Please don't hide the assignment in the argument here, would appreciate a temp
var.

Maybe:
double halfangle = angle * 0.5f;

And then use it below as well.

Also, I'd prefer (but not require) a real name in the committer/author field 'petrs' is not so helpful.

In any event, looks pretty good, will recommend pulling with the above fixed.
Comment 4 Petr Skramovsky 2013-06-04 22:28:42 CEST
Harvey thank you for your review and suggestions, the latest revision fixes the issue and i set my user.name for jogl repo as well.
Comment 5 Harvey Harrison 2013-06-04 22:39:38 CEST
Thanks, looks good to me, I wouldn't mind seeing the fixup folded back into
the other commit, but it's not a huge deal.

Would you mind terribly fixing up your commit name in the other commits?

I'm not sure how Sven feels about it, so don't feel it is a must-do.  From a code standpoint, I feel this is pullable.

Harvey
Comment 6 Sven Gothel 2013-06-12 12:49:54 CEST
Thank you - merged.

I also took the liberty to add qualifier 'final' to Quaternion and VectorUtil.