Unnecessary temp tables in SpatialHash

[kikito]

Hi there,

I've started browsing the HC code, to create a my own collider version (I only need AABBs, with no rotation). I've started with SpatialHash, so I'll start with it. I might have other comments as I read other sections.

It seems to me that SpatialHash's API & implementation require the creation of temp tables that are probably not really needed.

On the API side, consider SpatialHash:insert definition:

function Spatialhash:insert(obj, ul, lr)

ul and lr are tables. But then, on the method's implementation, they are used only in terms of their components: ul.x, ul.y, lr.x, lr.y. On the other hand, the invocation looks like this:

self._hash:insert(shape, {x=x1,y=y1}, {x=x2,y=y2})

I suggest changing insert's signature to 5 parameters:

function Spatialhash:insert(obj, ulx, uly, lrx, lry)

And the call would be:

self._hash:insert(shape, x1, y1, x2, y2)

(Aside note: I personally prefer specifying bounding boxes as l, r, w & h, but suit yourself)

The same change could be done in remove, update & getneightbors.

On the implementation side, I think cells_meta's implementation is again forcing the creation of unnecessary tables.

Probably I'm missing something, but it seems rather overcomplicated; for example, I don't think cells_mt.newIndex is invoked at all (everything is set via rawset). And it doesn't seem like index is doing anything useful - appart from requiring an extra function call.

I think you could get away with something simpler - along these lines:

...
self.cells = setmetatable({}, {__mode = "kv"}) -- cells_mt is no more
...
function Spatialhash:cell(wx, wy) -- world coordinates
    local x,y = self:getCoords(wx, wy) -- cell coordinates
    self.cells[x] = self.cells[x] or setmetatable({}, {__mode = "kv"})
    self.cells[x][y] = self.cells[x][y] or setmetatable({}, {__mode = "kv"})
    return self.cells[x][y]
end

Intuitively, these changes should bring some benefits in memory and speed; but I confess I have not made any tests at all.

Regards!

[vrld]

This is basically a relic from the time when HC used hump.vector instead of hump.vector-light. It's already fixed, but will have to sit on my hard drive until I have access to a fast and stable internet connecton again. :/

[vrld]

Done. I did some profiling but could not find any relevant performance differences. But the API is much cleaner now :)

[kikito]

Nice.

How do you do profile? On my machine, doing it with LÖVE I'm stuck with 60fps, so I can't tell if a change improves or degrades the performance. Maybe you are using native Lua only?

[vrld]

I used luaprofiler to profile this code. It measures some performance differences in hash:update() and hash:getNeighbors(), but the difference in average time per call is so small (order of magnitude 0.0001) that this could also be attributed to different system loads when I took the measurements.

[kikito]

ok, thanks!