Skip to content
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

Saving a texture as png with save_png() gives corrupted image #18109

Closed
garyo opened this issue Apr 10, 2018 · 5 comments
Closed

Saving a texture as png with save_png() gives corrupted image #18109

garyo opened this issue Apr 10, 2018 · 5 comments
Assignees
Milestone

Comments

@garyo
Copy link
Contributor

garyo commented Apr 10, 2018

Saving a texture with save_png() saves what looks like a corrupted mipmap, or maybe something overwrote the image data; it's not the full-res texture image.
This is with godot 3.0.1 master 3ad9e47 on Windows. No GDnative or anything fancy. Here's the main.gd that does the saving:

func _ready():
	var filename = "texture-image-saved.png"
	print("Saving PNG for ", $Floor, $Floor.get_surface_material(0))
	var img = $Floor.get_surface_material(0).albedo_texture.get_data()
	var x = img.save_png(filename)
	print("Saved.")

Minimal project attached; just run it and look at the resulting texture-image-saved.png file.

Minimal repro project:
save-tex-test.zip

Source texture:
picture08_basecolor

What you get from save_png():
texture-image-saved

The bottom 3/4 of the main image is OK; the top 1/4 is quarter res vertically and doubled horizontally (could be a half-res image in the same memory), then quarter of that etc.

@akien-mga akien-mga added this to the 3.1 milestone Apr 10, 2018
@Vega-KH
Copy link

Vega-KH commented Apr 10, 2018

I haven't found the solution to this yet, but this may be useful: This error only happens if you import the texture using the "VIDEO RAM" compression method. If the texture compression is set to lossless, lossy, or uncompressed, the saved image file is correct.

Probably related to #16079

EDIT: It seems this may not be a bug, as the get data method will return the complete image including mipmaps, and "Video Ram" mode currently forces mipmaps on. Lines 223 & 224 of resource_importer_texture.cpp are:
if (p_mipmaps || p_compress_mode == COMPRESS_VIDEO_RAM) //VRAM always uses mipmaps format |= StreamTexture::FORMAT_BIT_HAS_MIPMAPS; //mipmaps bit
But it may be useful to have a new function in the Texture class called get_image (in addition to get_data) that would get the highest resolution mip and stretch it to the correct dimensions.

@Vega-KH
Copy link

Vega-KH commented Apr 10, 2018

On second thought, it seems the get_data method is buggy when writing a file saved as video ram. With the other formats, the image writes fine even if MipMaps are on.

As a work around, you can clear mipmaps before you save like this:

func _ready():
	var filename = "texture-image-saved.png"
	print("Saving PNG for ", $Floor, $Floor.get_surface_material(0))
	var img = $Floor.get_surface_material(0).albedo_texture.get_data()
	img.clear_mipmaps()
	var x = img.save_png(filename)
	print("Saved.")

This will produce a correct image.

@volzhs
Copy link
Contributor

volzhs commented Apr 17, 2018

should clear_mipmaps() be done in save_png()?

@Vega-KH
Copy link

Vega-KH commented Apr 17, 2018

On a brief look at the code, it appears that the save_png function works on a reference to the texture image (rather that creating a copy like the code above does.) So running clear_mipmaps() inside save_png() would delete the mipmaps from the original texture.

So one solution would be to alter the save_png function to make a copy of the image, then clear mipmaps, then save it. This would temporarily use up some memory, but it could be immediately disposed.

@garyo
Copy link
Contributor Author

garyo commented Apr 19, 2018

reduz says on irc "fault is mostly in image_compress_squish.cpp".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants