SFTtech / openage

Free (as in freedom) open source clone of the Age of Empires II engine 🚀
http://openage.dev
Other
12.6k stars 1.11k forks source link

Speed up converter terrain merging with Cython #1369

Open heinezen opened 3 years ago

heinezen commented 3 years ago

Required skills: Cython, Python

Difficulty: Easy

The terrain textures in AoC and SWGB are stored as isometric* tiles (see https://github.com/SFTtech/openage/issues/141 for an example). In the converter, we merge all these tiles together to get the cartesian projection of the texture (also used in the HD Edition: see https://github.com/SFTtech/openage/issues/141#issuecomment-62250621). The cartesian projection has many advantages, the main one being that the image size is halved and the generated texture file smaller.

An algorithm for terrain merging has already been implemented here using Cython. You main goal is to improve the speed of the conversion by reducing the Python overhead of the function and adjusting the algorithm. Cython will generates a file that shows the overhead of the source file in bin/openage/convert/processor/export/texture_merge.html when you build the project. We want the Python overhead of the merge function to be as small as possible.

For your conveniance,we have made a list of things that could be improved. You should probably have a look at the code first though.

You can test your changes with the singlefile media converter.

Further reading:


* Technically, the projection is a dimetric (or game isometric) projection. Keep that in mind, when you search the internet for it.

zehuilu commented 3 years ago

Hello, can I take this?

TheJJ commented 3 years ago

Sure, you can just submit an early pull request and get help in the chatroom :)

ranbir7 commented 1 year ago

can i help with this?

heinezen commented 1 year ago

@ranbir7 Sure :)

Do you need some directions on where to start?

ranbir7 commented 1 year ago

Yes please ..

heinezen commented 1 year ago

@ranbir7 Alright.

Old AoC versions store every terrain tile individually and already projected to how they look ingame (i.e. they have a diamond shape). What our converter does is

  1. merging all these tiles together
  2. "rotating" the diamond shape so that we get a flat rectangular texture

This is just for context. You don't really need to understand how it works since it's already implemented. What we interested in for this task is speed. For this purpose, it's important that the tiles merge code (written in Cython) gets fast access to the individual tiles that it operates on.

The terrain tiles for the whole terrain are fetched from the files starts here:

https://github.com/SFTtech/openage/blob/35064788daad55d2f07a64bc9860306374dbbdb8/openage/convert/entity_object/export/texture.py#L106-L121

_to_subtextures() is a method that creates a TextureImage object that references the pixel data for single tile in a numpy array and also some metadata like the hotspot (the latter is unimportant for terrains). All TextureObjects are put into a list; the self.frames member of the texture.

The frames are accessed by the tiles merge code starting here:

https://github.com/SFTtech/openage/blob/35064788daad55d2f07a64bc9860306374dbbdb8/openage/convert/processor/export/terrain_merge.pyx#L44

Since we are accessing Python objects here, Cython cannot use it's full potential for speeding up some of the following calculations. Here, a more Cython-friendly solution would be nice. For example, instead of creating TextureImage objects for each tile, we could directly store the numpy arrays.

ranbir7 commented 1 year ago

Okay this is something that's gonna take time for me to understand But I'll let you know if I'll be able to do it Just give some few good days Alright?

heinezen commented 1 year ago

@ranbir7 Okay!

Feel free to ask more questions in our chatroom.

ranbir7 commented 1 year ago

im getting stuck up alot in https://github.com/SFTtech/openage/tree/master/openage/convert/processor/export/terrain_merge.pyx what shall I do here though? like I'll use numpy to do what?

heinezen commented 1 year ago

@ranbir7

In the terrain merge function, the numpy arrays store the RGBA pixel data for each tile. Individual tiles are merged into one big tile (which is also a big numpy array).

Currently, the tiles are stored as TextureImage objects. The TextureImage.data member stores the RGBA data for this tile.

You can see how the data is accessed in lines 44 and 75-76. In line 44, we first retrieve all TextureImage objects for the terrain and store them in the frames variable:

https://github.com/SFTtech/openage/blob/35064788daad55d2f07a64bc9860306374dbbdb8/openage/convert/processor/export/terrain_merge.pyx#L44

Later, we access the numpy arrays (the data member) for each frame in the for loop:

https://github.com/SFTtech/openage/blob/35064788daad55d2f07a64bc9860306374dbbdb8/openage/convert/processor/export/terrain_merge.pyx#L75-L76

What we want to change in this for loop is that it should iterate over the numpy arrays instead of calling TextureImage.data in each iteration. I don't know how familiar you are with Cython, but the reason for this is that in Cython, access to Python objects is slow, while access to numpy arrays is fast. Therefore, we want to minimize access to Python objects in loops and the function itself.

ranbir7 commented 1 year ago

yes, I am not familiar in cython, matter of fact im learning cython on the go so its a lil bit difficult for me to get through Ill be working on this, if I find any difficulties later I'll ask ,Thanks for helping though😄