-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
draft for convert_terrain #1204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Did you have a chance to look at our feedback, @charliergi? :) |
|
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 :) |
|
Oh no worries, just making sure it's nothing on our end that hinders your fun journey through the code 😉 |
There was a problem hiding this 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).
| 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
I would still recommend to have the merge begin even sooner with a custom terrain packer in
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 ( |
|
I'll continue when I'll have time. Thanks for the feedback :) |
|
Sure, take your time. If it merges then you're already halfway done :) |
|
@charliergi Any assistance needed here? :) |
|
Implemented by me in #1151, sorry @charliergi |
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
SLPdata from anSLP, and pass it todetermine_rgba_matrix()Thanks in advance for your feedback.