lmas / opensimplex

This repo has been migrated to https://code.larus.se/lmas/opensimplex
MIT License
241 stars 29 forks source link

Cleanup and a few optimizations #2

Closed CreamyCookie closed 8 years ago

CreamyCookie commented 8 years ago
  1. Methods and attributes that were referenced multiple times were replaced by a local variable.
  2. Removed unnecessary initializations.
  3. Made gradiens$D tuples instead of lists to save memory. (and renamed them as they're constants)
  4. Cleaned up the code in a few places (removed unneeded brackets and docstrings now follow PEP8)
  5. Renamed variables to use underscores (PEP8)
  6. Made extrapolate methods more efficient by using Python's ability to access a range of a sequence.
  7. Replaced fastFloor with real floor which is actually much faster.
lmas commented 8 years ago

Wow, what a massive PR, thanks man! :+1: Gimme a couple of days to review/test it and I'll pull it in.

lmas commented 8 years ago

Added some test, but it fails on your code on python2:

Traceback (most recent call last):
  File "/home/lmas/code/opensimplex/tests/test_opensimplex.py", line 21, in test_samples
    actual = simplex.noise2d(s[0], s[1])
  File "/home/lmas/code/opensimplex/opensimplex/opensimplex.py", line 169, in noise2d
    value += attn1 * attn1 * extrapolate(xsb + 1, ysb + 0, dx1, dy1)
  File "/home/lmas/code/opensimplex/opensimplex/opensimplex.py", line 105, in _extrapolate2d
    index = perm[(perm[xsb & 0xFF] + ysb) & 0xFF] & 0x0E
TypeError: unsupported operand type(s) for &: 'float' and 'int'

Traced it back to xsb/ysb turning into floats after a call to that new floor() function.

I've been away from python for too long, you think it's safe/performant to just wrap the floor() calls inside an int(floor(..))?

CreamyCookie commented 8 years ago

@lmas Ah, no problem. =)

Cool that you added tests! Oops ..didn't even check Python 2.

I've added a check for Python 2 that wraps floor as you suggested.

lmas commented 8 years ago

Looks great :+1: Also made a simple benchmark, the results from it (for both python 2 and 3) didn't indicate any really performance loss/gain so I think we're pretty good.

Thanks a bunch, pal

lmas commented 8 years ago

dammit, wrong button.