Adding TileStitcher as optional postprocessing tool

[thiloSchlemmer]

Hi Loic,

sorry for the confusing pull requests... :) i try to make future requests be more cleaner

[coveralls]

Coverage increased (+2.8%) to 94.582% when pulling 83fb68a2a7d0801a24b46e34e7ba56e2a0f137a1 on thiloSchlemmer:master into d4251b2c9c955e4c09eb73dfc597101a4bce3dd4 on loicgasser:master.

[loicgasser]

won't have time to finish the review today, this will take some time

[thiloSchlemmer]

I try today and tomorrow to make the first changes / updates of your comments (typo, codestyle, etc.). On Friday I'm almost offline for 2 weeks and can only comment a little bit. In the first week of September I will make the bigger code-changes. Is that ok for you?

[loicgasser]

Yes of course take your time! We'll try to do it bit by bit. Thank you!

[loicgasser]

I like the methods you added in editable terrain. I would try to put as many methods and properties as we can on the TerrainTile class, thus avoiding code duplication and improving the code base.

[thiloSchlemmer]

Hi Loic,

back again, next steps i could make, are:

• move EditableTerrainTile.boundingBox to TerrainTile.boundingBox
• simplify usage of TileStitcher
• move EditableTerrainTile._quantize*/_dequantize-Methods to TerrainTile

... and adding content to docs.

Any favorites or something i miss?

[loicgasser]

@thiloSchlemmer thanks! Looks already better.

We're gona have to break it down into smaller PRs. It seems to me, we still have a lot of duplicated code.

Can you try to make smaller PRs? Identify code you copy pasted and refactor the base code.

A good place to start: In the terrain tile code we see in _computeVerticesCoordinates https://github.com/loicgasser/quantized-mesh-tile/blob/master/quantized_mesh_tile/terrain.py#L298

...
                self._heights.append(
                    lerp(
                        self.header['minimumHeight'],
                        self.header['maximumHeight'],
                        old_div(float(h), self.MAX)
                    )
                )
...

and in your code we have:

    def _dequantizeHeight(self, h):
        """
        Private helper method to convert quantized tile (h) values to real world height
        values
        :param h: the quantized height value
        :return: the height in ground units (meter)
        """
        return lerp(self.header['minimumHeight'],
                    self.header['maximumHeight'],
                    old_div(float(h), TerrainTile.MAX))

and the list goes on.

I am not against adding a private helper method to do that.

The current code base already has some duplicated code, but we should try to avoid making it worst than it is.

What do you think about breaking it down?

[thiloSchlemmer]

Hi Loic,

yes there is a lot of duplication, mainly caused by the first attempt to split the tilestitcher out to a seperate package. after refactorings the copy-pasted (and duplicated) code is not always easy to identify, but thats not a problem. So i agree with you to make smaller PR, only with the tileStitcher-additions. First of all i have to figure out, how to make such a smaller PR, then i would name the suggested parts.

[thiloSchlemmer]

Hmmm... I 'rebase' somehow (..and accidently close this PR).

To get smaller PR, i make branches for every step. I start with the quantized/dequantize helper methods and create/refactor them in terrain.py out. Ok?

Next step could be to integrate a cleaned version of editable_terrain.py.

[loicgasser]

yes perfect