这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@charliergi
Copy link

So here I am with my first contribution, as asked in #985
There is a lot of optimization that must be done, but first I need to understand the logic of the code.
The code don't run because I can't extract properly the special SLP data from an SLP, and pass it to determine_rgba_matrix()

Thanks in advance for your feedback.

@simonsan simonsan added area: assets Involved with assets (images, sounds, ...) nice new thing ☺ A new feature that was not there before lang: python Done in Python code labels Dec 9, 2019
@TheJJ
Copy link
Member

TheJJ commented Dec 21, 2019

Did you have a chance to look at our feedback, @charliergi? :)

@charliergi
Copy link
Author

Yes of course ! I've been busy the two last weeks. I've began to improve the code. I'll finish it as soin as possible :)

@TheJJ
Copy link
Member

TheJJ commented Dec 21, 2019

Oh no worries, just making sure it's nothing on our end that hinders your fun journey through the code 😉

Copy link
Author

@charliergi charliergi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I have to good logic this time. I've managed to add the command on the doc to execute the code for terrains. (look at the singlefile doc). The code still needs to be cleaned, and optimized with Cython.
If you have ideas about why the merge method fails, or if you have a better idea, let me know.
(If you don't have time to run the code, basically, the method "merge" returns a transparent png).

Comment on lines +28 to +34
while yd < overlayed.shape[1]:
# if the overlayer is not transparent, then we can push it on the overlayed
# without the if, we would overwrite non-transparent pixels with transparent ones.
if xr < overlayer.shape[0] and yr < overlayer.shape[1] and overlayer[xr,yr][3]>0:
to_ret[xd,yd] = overlayer[xr,yr]
yd+=1
yr+=1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yd is never reset, so the inner loop is only executed once.

Also, what is the purpose of the xr < overlayer.shape[0] and yr < overlayer.shape[1] conditions?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow I feel stupid now haha, of course I have to reset them!
Because the size of the merge_img is a bit smaller than the original SLP terrain texture, I had to make sure I wasn't pasting any pixel out of the merge_img. Basically, these conditions are there to avoid out of bound indexes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! However, I still wonder what the inner and outer loop accomplish exactly. Wouldn't the current logic operate on the whole image in the worst case (xd, yd = 0) instead of just one tile?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I corrected, thanks to your advices. It's now merging the tiles but the coordinates are wrong. I have to correct them.


#matrix = determine_rgba_matrix(image, None, 0) ???
image_new = Image.fromarray(image)
image_new = texture_tomod.image_data.get_data().transpose(1,0,2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why transpose the array here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everytime I use the function Image.fromarray(image_array, "RGBA"), the array is transposed (x and y are swapped). If I remember well my computer architecture courses, this is because it depends on the array representation in memory. (https://github.com/python-pillow/Pillow/issues/2619#issuecomment-315333880) I don't know if I'm right, but we should be able to identify which array representation is used in memory.

Copy link
Member

@heinezen heinezen Dec 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are unit SLPs swapped like this too? This bug was fixed some time ago and we've been using Image.fromarray() to save the textures without problems for a while.

@heinezen
Copy link
Member

I would still recommend to have the merge begin even sooner with a custom terrain packer in merge_frames of the texture.py module. Right now, the array/image operations are the following:

  1. All tile arrays are appended/merged into a single array (texture_tomod.image_data) by merge_frames in texture.py
  2. You then merge the tiles a second time by extracting the tiles again and overlaying them in another merge array
  3. The result is transformed into the "flat" representation of the texture.

Appending the tiles in Step 1 is basically redundant right now since the tiles are extracted again anyway. You should aim to integrate Step 2 into Step 1 by having the tiles be merged directly into the first single array (texture_tomod.image_data). This can be done via a new packer specifically for terrains.

@charliergi
Copy link
Author

I'll continue when I'll have time. Thanks for the feedback :)

@heinezen
Copy link
Member

Sure, take your time. If it merges then you're already halfway done :)

@heinezen
Copy link
Member

@charliergi Any assistance needed here? :)

@simonsan simonsan added the waiting for author This issue/PR is waiting for feedback of the author label Apr 26, 2020
@heinezen
Copy link
Member

Implemented by me in #1151, sorry @charliergi

@heinezen heinezen closed this Aug 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: assets Involved with assets (images, sounds, ...) lang: python Done in Python code nice new thing ☺ A new feature that was not there before waiting for author This issue/PR is waiting for feedback of the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants