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. ;)

[ftomassetti]

Ok, so let's merge #161 first

[tcld]

EDIT: Oops, this was supposed to be a reply to https://github.com/Mindwerks/worldengine/pull/163#discussion_r43624596

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 https://github.com/Mindwerks/worldengine/pull/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 (https://github.com/Mindwerks/worldengine-data/pull/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).

[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.)

[psi29a]

It was fixed here: https://github.com/dezhin/pygdal/blob/master/setup.py#L61

just waiting for new release to drop.

[tcld]

Oh ... I also feel like I read that before.^^

[tcld]

I rebased #185 on this since before I wasn't able to have that PR pass it's new tests.

Since there are quite a lot of changes in here, I'll give a short rundown:

• The image-drawing class (PNGWriter) operates on a numpy-array and provides the same interface as the old ImagePixelSetter; pixels can be accessed/modified via the numpy-interface, if wanted.
• Pillow has been completely replaced by PyPNG.
• All access to the image-module happens inside _imageio.py, the image-module could be swapped out easily.
• All direct access to data-carrying arrays (as well as images) happens via [y, x]; there were inconsistencies before where [x, y] was used instead. (Orientation of the images stays the same.)
• PNGReader reads PNGs and stores them in a numpy-array; the reader is used by the tests and later might want to be used in other parts of the code to circumvent the need for directly accessing the underlying image-module.
• At the moment RGBA-, RGB- and grayscale-PNGs can be created easily within a single line of code; palette-support was skipped for now.

[psi29a]

Do we need Pillow at all anymore? If we default to PyPNG (purely python) for all output, then use GDAL to support other things that Pillow normally supported, then we don't need Pillow.

[tcld]

We don't need Pillow, I tried to remove it. Did I overlook something?

[psi29a]

also in appveyor, tox, etc for the pillow->pypng switcheroo ?

[tcld]

tox.ini, requirements.txt and setup.py should be taken care of. appveyor? I don't know what to change in there.

[psi29a]

This is super! :)

[psi29a]

Do we need new blessed images?

[tcld]

Some for sure. I don't remember which ones, though - I created a full set of new worlds and images; non-square because I needed some feedback whether [y, x] etc. were misused. (I think the blessed worlds shouldn't be square so the tests are a little stronger.)

EDIT: Grayscale heightmap and ancient map needed to be updated for sure.

[tcld]

This is still my next PR, some (if not all) of my current PRs are based on this. Maybe somebody finds some time to check the code; the x-y-replacements have been done using a regular expression, so there are no typos to be expected (and thus most of the changes in drawing_functions.py won't have to be reviewed in depth).

[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: https://github.com/Mindwerks/worldengine-data/pull/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. https://github.com/Mindwerks/worldengine-data/pull/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.