search

nicklockwood / Euclid

A Swift library for creating and manipulating 3D geometry
MIT License
644 stars 53 forks source link

Test for wasm32/WASI on CI #89

Closed MaxDesiatov closed 2 years ago

MaxDesiatov commented 2 years ago

I'm not 100% sure why some of the values produced by tests slightly deviate, maybe because Double on wasm32 is 32-bit, so there's some loss of precision?

nicklockwood commented 2 years ago

This is awesome, thanks @MaxDesiatov!

MaxDesiatov commented 2 years ago

Looks like SwiftWasm 5.4/5.5 CI is failing due to the lack of support for NSKeyedUnarchiver in those versions, I've removed them from the matrix.

codecov-commenter commented 2 years ago

Codecov Report

Merging #89 (c6b1789) into master (b08ed5e) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #89   +/-   ##
=======================================
  Coverage   77.60%   77.60%           
=======================================
  Files          24       24           
  Lines        4916     4916           
=======================================
  Hits         3815     3815           
  Misses       1101     1101

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b08ed5e...c6b1789. Read the comment docs.

nicklockwood commented 2 years ago

Double on wasm32 is 32-bit

Seriously?!

MaxDesiatov commented 2 years ago

My bad, Double is 64-bit. I thought of Int, which indeed is 32-bit on wasm32. I don't have any other possible explanations then. I haven't yet exercised any of the code exposed by these tests that behaves differently, so can't say how big of an impact that is. Would have to debug thoroughly to say more. Let me know if these naive #if os(wasm32) checks in tests are blockers for approving and merging this, I'll prioritise debugging it in that case.

nicklockwood commented 2 years ago

I don't have any other possible explanations then

The CSG stuff is pretty sensitive to slight differences in rounding, so I'm not terribly surprised by it tbh. I wouldn't worry about it for now in any case.