ungerik / go3d

A performance oriented 2D/3D math package for Go
MIT License
310 stars 49 forks source link

Include copy of fmath with fix for archs other than amd64 #28

Closed hubues closed 3 years ago

hubues commented 3 years ago

Imported package github.com/barnex/fmath does not compile on other architectures than amd64:

$ GOOS=linux GOARCH=arm go build
# github.com/barnex/fmath
/Users/hugues/go/pkg/mod/github.com/barnex/fmath@v0.0.0-20150108074215-ec9671f295c2/sqrtf_decl.go:7:6: missing function body

fmath package provides float32 wrappers for standard math package float64 functions. It as (only) one optimization in assembly code for sqrt function, for architecture amd64. This optimization is implemented in two files: sqrtf_amd64.s and sqrtf_decl.go. That later file must be compiled only for amd64 arch because its function body is empty and implemented in sqrtf_amd64.s. Currently it is included in builds for all archs, leading to the error reported above. To fix that, it should be renamed or include a build flag:

// +build amd64

This pull request proposes to include (copy) fmath package code from github.com/barnex/fmath into go3d, and remove the dependency to github.com/barnex/fmath. One could argue that github.com/barnex/fmath could be fixed instead, but that project shows no activity since 6 years. In addition, it is very simply made of generated wrapper code from a simple bash script (see codegen.bash). The only optimisation is within sqrtf function, that works only for amd64. Hence a pragmatic approach is to embed the fixed fmath functions into go3d (just my humble opinion).

ungerik commented 3 years ago

Thanks