16 Bit grayscale heightmaps

[tcld]

This is the setup I am using to generate 16 Bit grayscale heightmaps, extracted from the (now dead) #141 . There is a big chunk of code here that is basically a drop-in replacement of the ImageWriter-class used so far (I gave it its own file). Technically that class could be used to write all images (which would remove the Pillow-dependency) but I didn't want to go in too deep again. Maybe later.

The tests won't be passed until I also update the grayscale heightmap in worldengine-data. That can happen once the code here is acceptable, I think.

EDIT: I had to base this off of #161 , currently only the last two commits here are actually new.

PS: I will try hard to keep this my last PR until all others are handled. ;)

[tcld]

I decided to just write out the array. I know this circumvents all modifications usually done in draw_grayscale_heightmap() but since this file is usable to import data into other programs I didn't like the idea of shifting around elevations.
If this is a bad idea, we can probably add in another parameter to decide which route to take - although I am not sure if a modified heightmap would prove useful at all.

[tcld]

When changing draw_simple_elevation(), @esampson gave a reason for why it might be preferable to manipulate the elevation map.
Maybe you have a comment for this change?

[ftomassetti]

In general the idea is to have a function which do all the work on an "abstract image" (e.g., draw_grayscale_heightmap) and then a simple wrapper (draw_grayscale_heightmap_on_file). The reason is that the abstract image could either end up on an image file on disk OR it could be used to draw on the screen (as we do in worldengine-gui). This is the basic principle to follow to ensure that the user see on the screen what it will be later saved on disk.

If we need exceptions for performance reason I would just suggest to document that and keep them as exceptions.

[tcld]

PNGWriter can take any numpy-array and write it to a PNG-file.* In the case of world.elevation['data'] there is no immediate reason to perform any kind of processing before that. I don't see the need for any kind of change - so draw_grayscale_heightmap(world, img) could technically be an empty function.

This was not due to performance reasons, just because I wouldn't want to see the heightmap tampered with (since when using it as foundation for a 3D-landscape that would probably lead to visual irregularities).

*= The "abstract image" here is a full copy of the elevation data.

[tcld]

To sum this up and finish it:
The image for the grayscale-heightmap is now generated basically by directly writing the corresponding numpy-array into an image-file. There is no middleman necessary.

[ftomassetti]

Ok, so let's merge #161 first

[tcld]

EDIT: Oops, this was supposed to be a reply to #163 (comment)

Ok, I think I understand what you mean. Since draw_grayscale_heightmap() is still around and in working order (although it will produce output that slightly differs from the raw elevation map) all of this should not cause problems right now.

Here it doesn't look like the functions in draw.py are actually used, though:
https://github.com/Mindwerks/worldengine-gui/blob/master/worldengine-gui/view.py

[ftomassetti]

that could be because Bret reimplemented the GUI, we dropped the code I had from lands. I think in the long run we should use the same functions here and there but the GUI is still in alpha state for now I think. Probably @psi29a is the best person to comment on this

[tcld]

Still, me not calling draw_grayscale_heightmap() in draw_grayscale_heightmap_on_file() would not stop the GUI from using draw_grayscale_heightmap(). Right now it might just not be necessary to do that extra step in draw_grayscale_heightmap_on_file().

[tcld]

About the three failing tests:
The grayscale-test obviously fails until a new (16 Bit) grayscale heightmap is pushed to worldengine-data.
The ancient-map tests might need a slightly extensive rewiring within drawing_functions.py. I think I will take care of it later today, but there will be a lot of (very trivial) changes, as far as I can tell.

[psi29a]

Couldn't this already be done via GDAL export? It was the reason why I dropped pypng support to begin with as GDAL can handle this for us.

You tell it to use png with 16-bit unsigned.

[tcld]

Yes, GDAL can do it. But creating a proper heightmap without GDAL seems like a good idea to me, the "normal" fake-grayscale heightmap isn't too useful anyway.
GDAL can still handle all possible other formats ( #159 ), including raw data.

[tcld]

Additionally, Pillow can handle several different formats but doesn't fully support the PNG-standards. And since worldengine uses Pillow to exclusively write PNG-files, I thought that it might be a good idea to switch to a library that fully supports PNG, and only that.

[psi29a]

It is a pity that 16-bit greyscale png isn't widely supported.

I know... been there with pypng as well. :)

https://github.com/psi29a/worldsynth/blob/2b2e9f8d8b42f34f512b923a96e1fdc636f795b1/worldsynth.py#L635

[tcld]

Well, I haven't found a program that couldn't at least open the output. And the file could be directly imported to Unreal Engine, which (I think) is a good example use-case for grayscale heightmaps.

Also, the class I wrote to handle PNG does support all formats (except the ones using palettes) so the output could be changed by switching just a small piece of code:

img = PNGWriter.grayscale_from_array(world.elevation['data'], filename, scale_to_range=True)

becomes

img = PNGWriter.rgba_from_dimensions(world.width, world.height, filename, scale_to_range=True)
draw_grayscale_heightmap(world, img)

So at the very least nothing is lost.

[tcld]

So, do you think I should pursue and finish this PR? Or do you really not like the idea of creating 16 Bit heightmaps by default?

[psi29a]

Are you sure that gdal can't create 16-bit greyscale pngs? I thought I tested this, which is the reason I removed pypng since it would be redundant.

https://trac.osgeo.org/gdal/ticket/2806
^-- uint16 png works

[tcld]

GDAL can do it for sure, I said that before. ;) But it is also a library that isn't too easy to install (for me, at least) and won't automatically come with the WorldEngine release. And I am not sure what uses the fake grayscale (it's actually RGBA made to look gray) 8 Bit heightmap as default output actually has.

I did design the PNGWriter-class to be a full replacement for the Pillow-writer - I didn't know that would kind of be a step backwards. I will think about this tomorrow.

[tcld]

I decided to just finish this and let you decide. With leftover code from #141 there wasn't much work left to do, so I just went all the way - Pillow is gone, I even regenerated all the data (Mindwerks/worldengine-data#11, mostly because I needed the tests to hiccup in case x- and y-coordinates of arrays/images were accidentally swapped; since all test-maps were square before that was not assured). Python 2 and 3 passed all tests with that data on my system.

There are some more PyPNG abilities that could be taken advantage of, but I tried to leave output and code as unchanged as possible (there already are enough changes in this PR, even though most of them are tiny).

[tcld]

I started noticing in erosion.py that a couple of arrays were created by using (width, height) as dimensions - while all "major" arrays were created using (height, width). So I changed that and one thing lead to another...All changes in this file swap x- and y-coordinates. I ran a couple of tests to make sure the output stayed exactly the same, so there are some (~2) lines that do slightly more than just switching x and y.

[psi29a]

I'm still curious as to what the problem was with GDAL on linux... it works here just with pip install pygdal (provided you have libgdal-dev package installed).

Any releases I upload (binary releases) will include gdal, that includes windows/osx/linux.

[tcld]

It was pygdal that didn't work for me, I don't remember the exact reason, though - I switched PCs since so I don't have an immediate way to reproduce the problem either.

[tcld]

A fresh environment (well, I did the things recommended in README.md):

(venv3)tcl@worldengine$ pip3 install pygdal
Collecting pygdal
  Using cached pygdal-1.11.2.1.tar.gz
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 20, in <module>
      File "/tmp/pip-build-4b773ez_/pygdal/setup.py", line 61
        print 'GDAL prefix from environment variable %s' % ENV_GDALHOME
                                                       ^
    SyntaxError: Missing parentheses in call to 'print'

    ----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-build-4b773ez_/pygdal

I feel like I posted this before. pygdal really doesn't seem to be compatible with Python 3.

I also switched PCs again and still cannot get a proper Python 2 venv going. On the laptop it worked fine - but I will just go ahead and assume that pygdal will work fine with Python 2. It has to work somewhere, I guess.^^

On top of all those errors, I have the perfect solution for this PR - no need to include two ways to produce 16 Bit heightmaps, we can just remove draw_grayscale_heightmap() completely! :P (I really feel that the output isn't exactly useful in 8 Bit.)

[tcld]

I added a small comment to image_io.py regarding the [y, x]-notation, rebased the code to the current master and made some very small improvements. Is this ready to be merged yet?

Do you think that _dynamic_draw_a_mountain() should be removed? It isn't used yet and seemingly was never finished. But should esampson get done with his current work, the function would be already obsolete.

[tcld]

Data used to pass the tests: Mindwerks/worldengine-data#11
I didn't have to regenerate all data (ancient maps, rivers and grayscale heightmap would have been enough) but I wanted to have non-square maps to make sure the changes to the xy-notation in this PR are alright.

If you don't like that, I can change the data-PR to only change the data that absolutely has to be updated.

[psi29a]

The maps can be square and rectangular. That's fine.

Do you consider this PR ready?

[tcld]

I think it's ready, yes.

(But should you want to merge it, don't forget about the updated worldengine-data.)

[psi29a]

I don't see a PR for the WE-data?

[tcld]

This one.
Mindwerks/worldengine-data#11

[psi29a]

Weird, I didn't get that an hour ago... I need sleep.

[tcld]

Tests succeeded. :)
By the way: Will the Travis-tests be updated before a release? Or is that too much work?

[psi29a]

What do you mean?

[tcld]

All the tests that usually fail.

EDIT: https://travis-ci.org/Mindwerks/worldengine/builds/90341120
There is a second pyflakes-test (?), a second manifest-test (?), a second py27-test (?), two pypy-tests and two py34-tests.
(But again, that isn't related to this PR. I was just wondering.)

[psi29a]

all the tests are green, otherwise it wouldn't say so.

do you mean environments? python3, and the osx py2/py3?

[tcld]

I meant all the optional tests that don't seem to work properly.
https://travis-ci.org/Mindwerks/worldengine/jobs/90341134
https://travis-ci.org/Mindwerks/worldengine/jobs/90341141
https://travis-ci.org/Mindwerks/worldengine/jobs/90341123
[...]

Do only I see those?

EDIT: Maybe they aren't called "tests", so I am probably referring to environments.

[psi29a]

Yes, those are environments... not tests. And no, we can't fix that until Travis upgrades ubuntu 12.04 to something more recent and we have someone with osx experience to help get that running. Any volunteers? ;)

There are other factors to, pypy (on ubuntu 12.04)... we sit in a catch22 and I haven't figured out yet how to handle that.

[tcld]

Ah, thanks for the explanation.

[rbb]

What kind of experience/skills are needed for the OS X environment testing? I have an OS X box, and I run python on it via MacPorts. What else would I need?

[psi29a]

Well, travis allows us to use osx in addition to linux (ubuntu 12.04).

I got travis to setup up the osx instances, but now we need to figure out how to actually get python (2.7 and 3.whatever) installed on them along with virtualenv and tox. Likely we can do that with brew/taps.

That would get us coverage on osx up and running. :)

[psi29a]

btw.. merging this branch.