Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(777)

Issue 141440043: code review 141440043: go.mobile/f32: float32 math library (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 6 months ago by crawshaw
Modified:
9 years, 6 months ago
Reviewers:
nigeltao
CC:
nigeltao, bryanturley, adg, alsodave, golang-codereviews
Visibility:
Public.

Description

go.mobile/f32: float32 math library I apologise for the poor state of this library. I hope to improve it significantly soon. Right now it exists to support example/gopher and the debug library.

Patch Set 1 #

Patch Set 2 : diff -r 36bf6272cbe7bd0809df03e7d39ad2a45bf65bf4 https://code.google.com/p/go.mobile #

Patch Set 3 : diff -r 36bf6272cbe7bd0809df03e7d39ad2a45bf65bf4 https://code.google.com/p/go.mobile #

Total comments: 36

Patch Set 4 : diff -r 3bf2681432a0cf478ca360817016c82468781050 https://code.google.com/p/go.mobile #

Total comments: 6

Patch Set 5 : diff -r 547bbce45af3444f8b5d9a209501de569c9fd460 https://code.google.com/p/go.mobile #

Unified diffs Side-by-side diffs Delta from patch set Stats (+446 lines, -0 lines) Patch
A f32/f32.go View 1 2 3 1 chunk +34 lines, -0 lines 0 comments Download
A f32/f32_test.go View 1 2 3 1 chunk +185 lines, -0 lines 0 comments Download
A f32/mat4.go View 1 2 3 4 1 chunk +131 lines, -0 lines 0 comments Download
A f32/vec3.go View 1 2 3 1 chunk +49 lines, -0 lines 0 comments Download
A f32/vec4.go View 1 2 3 1 chunk +47 lines, -0 lines 0 comments Download

Messages

Total messages: 10
crawshaw
Hello nigeltao (cc: adg, alsodave, golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.mobile
9 years, 6 months ago (2014-09-11 14:48:01 UTC) #1
bryanturley
https://codereview.appspot.com/141440043/diff/40001/f32/f32.go File f32/f32.go (right): https://codereview.appspot.com/141440043/diff/40001/f32/f32.go#newcode56 f32/f32.go:56: v.X = v0.Y*v1.Z - v1.Y*v0.Z This looks wrong to ...
9 years, 6 months ago (2014-09-11 18:38:55 UTC) #2
nigeltao
https://codereview.appspot.com/141440043/diff/40001/f32/f32.go File f32/f32.go (right): https://codereview.appspot.com/141440043/diff/40001/f32/f32.go#newcode56 f32/f32.go:56: v.X = v0.Y*v1.Z - v1.Y*v0.Z On 2014/09/11 18:38:54, bryanturley ...
9 years, 6 months ago (2014-09-12 06:27:53 UTC) #3
crawshaw
I've moved this to gl/f32 and introduced a dependency on gl, as this is not ...
9 years, 6 months ago (2014-09-13 17:41:48 UTC) #4
bryanturley
I would not make it depend on the gl package. I believe gl4.5 an gles3.1 ...
9 years, 6 months ago (2014-09-13 18:15:42 UTC) #5
bryanturley
Must have missed a save on one of the comments... https://codereview.appspot.com/141440043/diff/40001/f32/f32.go File f32/f32.go (right): https://codereview.appspot.com/141440043/diff/40001/f32/f32.go#newcode171 ...
9 years, 6 months ago (2014-09-13 18:26:19 UTC) #6
crawshaw
On Sat, Sep 13, 2014 at 2:15 PM, <bryanturley@gmail.com> wrote: > I would not make ...
9 years, 6 months ago (2014-09-13 19:58:47 UTC) #7
nigeltao
LGTM. https://codereview.appspot.com/141440043/diff/60001/f32/mat4.go File f32/mat4.go (right): https://codereview.appspot.com/141440043/diff/60001/f32/mat4.go#newcode53 f32/mat4.go:53: m.Mul(src, &scaler) It seems just as easy to ...
9 years, 6 months ago (2014-09-14 23:33:25 UTC) #8
crawshaw
https://codereview.appspot.com/141440043/diff/60001/f32/mat4.go File f32/mat4.go (right): https://codereview.appspot.com/141440043/diff/60001/f32/mat4.go#newcode53 f32/mat4.go:53: m.Mul(src, &scaler) On 2014/09/14 23:33:25, nigeltao wrote: > It ...
9 years, 6 months ago (2014-09-17 22:07:20 UTC) #9
crawshaw
9 years, 6 months ago (2014-09-17 22:11:37 UTC) #10
*** Submitted as
https://code.google.com/p/go/source/detail?r=892f52b66f29&repo=mobile ***

go.mobile/f32: float32 math library

I apologise for the poor state of this library.
I hope to improve it significantly soon. Right
now it exists to support example/gopher and the
debug library.

LGTM=nigeltao
R=nigeltao, bryanturley
CC=adg, davidday, golang-codereviews
https://codereview.appspot.com/141440043
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b