ResourceSaver does not correctly save ImageTexture

[cmfcmf]

Godot version: 3.0.2

OS/device including version: Ubuntu 18.04

Issue description: The ResourceSaver is not correctly saving ImageTextures (at least I think so). The [resource] section's image is set to null, but should be set to SubResource( 1 ).

Minimal reproduction script: This is a simple script you can run. The asserts will not trigger. However, in the generated .tres files, image is set to null. (I hope it's okay to attach a script instead of a full project. I can also create a project, if you want me to.)

extends SceneTree

func _init():
	test1()
	test2()
	
	quit()

func test1():
	var image = Image.new()
	image.create(2, 2, false, Image.FORMAT_RGBA8)
	assert(image.save_png("test1.png") == OK)
	
	var image_texture = ImageTexture.new()
	image_texture.create_from_image(image)
	assert(ResourceSaver.save("test1.tres", image_texture) == OK)

func test2():
	var image = Image.new()
	image.create(2, 2, false, Image.FORMAT_RGBA8)
	assert(image.save_png("test2.png") == OK)
	
	var image_texture = ImageTexture.new()
	image_texture.load("test2.png")
	assert(ResourceSaver.save("test2.tres", image_texture) == OK)

Generated .tres files:

[gd_resource type="ImageTexture" load_steps=2 format=2]

[sub_resource type="Image" id=1]

data = {
"data": PoolByteArray( 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ),
"format": "RGBA8",
"height": 2,
"mipmaps": false,
"width": 2
}

[resource]

flags = 7
storage = 0
lossy_quality = 0.7
flags = 7
image = null
size = Vector2( 2, 2 )

[guilhermefelipecgs]

Also it is not possible to do this with the inspector. Tested on master (224d537).

[cmfcmf]

Here's the console output when running the code:

ERROR: _write_resource: Resource was not pre cached for the resource section, bug?
   At: scene/resources/scene_format_text.cpp:1368.
ERROR: _write_resource: Resource was not pre cached for the resource section, bug?
   At: scene/resources/scene_format_text.cpp:1368.

[cmfcmf]

I've done some debugging which revealed the root cause of the issue.
Saving the ImageTexture will call

godot/scene/resources/scene_format_text.cpp

Line 1460 in a0ea57b

Error ResourceFormatSaverTextInstance::save(const String &p_path, const RES &p_resource, uint32_t p_flags) {

which calls:

godot/scene/resources/scene_format_text.cpp

Line 1482 in a0ea57b

_find_resources(p_resource, true);

which calls for both the ImageTexture (= Image::get_data()) and the referenced Image object:

godot/scene/resources/scene_format_text.cpp

Line 1417 in a0ea57b

saved_resources.push_back(res);

So far so good. Now back to ResourceFormatSaverTextInstance::save. It iterates through the saved_resources and populates the internal_resources map. This will set internal_resources[<Image Object>] = 1 (line 1585), meaning that it shall be subresource 1 (which is correct).

godot/scene/resources/scene_format_text.cpp

Lines 1555 to 1589 in a0ea57b

	for (List<RES>::Element *E = saved_resources.front(); E; E = E->next()) {
	RES res = E->get();
	ERR_CONTINUE(!resource_set.has(res));
	bool main = (E->next() == NULL);
	if (main && packed_scene.is_valid())
	break; //save as a scene
	if (main) {
	f->store_line("[resource]\n");
	} else {
	String line = "[sub_resource ";
	if (res->get_subindex() == 0) {
	int new_subindex = 1;
	if (used_indices.size()) {
	new_subindex = used_indices.back()->get() + 1;
	}
	res->set_subindex(new_subindex);
	used_indices.insert(new_subindex);
	}
	int idx = res->get_subindex();
	line += "type=\"" + res->get_class() + "\" id=" + itos(idx);
	f->store_line(line + "]\n");
	if (takeover_paths) {
	res->set_path(p_path + "::" + itos(idx), true);
	}
	internal_resources[res] = idx;
	#ifdef TOOLS_ENABLED
	res->set_edited(false);
	#endif
	}

The next for loop now saves the resources' properties. It will first do so for the Image object, which it does correctly. Then comes the ImageTexture.

godot/scene/resources/scene_format_text.cpp

Lines 1594 to 1614 in a0ea57b

	for (List<PropertyInfo>::Element *PE = property_list.front(); PE; PE = PE->next()) {
	if (skip_editor && PE->get().name.begins_with("__editor"))
	continue;
	if (PE->get().usage & PROPERTY_USAGE_STORAGE) {
	String name = PE->get().name;
	Variant value = res->get(name);
	if ((PE->get().usage & PROPERTY_USAGE_STORE_IF_NONZERO && value.is_zero()) || (PE->get().usage & PROPERTY_USAGE_STORE_IF_NONONE && value.is_one()))
	continue;
	if (PE->get().type == Variant::OBJECT && value.is_zero() && !(PE->get().usage & PROPERTY_USAGE_STORE_IF_NULL))
	continue;
	String vars;
	VariantWriter::write_to_string(value, vars, _write_resources, this);
	f->store_string(_valprop(name) + " = " + vars + "\n");
	}
	}

It calls VariantWriter::write_to_string (line 1611) for the image property of the ImageTexture. This is where it should write image = SubResource( 1 ), but instead writes image = null and prints

ERROR: _write_resource: Resource was not pre cached for the resource section, bug?
At: scene/resources/scene_format_text.cpp:1368.

The VariantWriter::write_to_string calls ResourceFormatSaverTextInstance::_write_resource. For the reasons outlined below, the internal_resources.has(<Image Object>) check will return false, even though it should've been set to 1 (see above), resulting in "null" to be returned.

godot/scene/resources/scene_format_text.cpp

Lines 1353 to 1375 in a0ea57b

	String ResourceFormatSaverTextInstance::_write_resource(const RES &res) {
	if (external_resources.has(res)) {
	return "ExtResource( " + itos(external_resources[res] + 1) + " )";
	} else {
	if (internal_resources.has(res)) {
	return "SubResource( " + itos(internal_resources[res]) + " )";
	} else if (res->get_path().length() && res->get_path().find("::") == -1) {
	//external resource
	String path = relative_paths ? local_path.path_to_file(res->get_path()) : res->get_path();
	return "Resource( \"" + path + "\" )";
	} else {
	ERR_EXPLAIN("Resource was not pre cached for the resource section, bug?");
	ERR_FAIL_V("null");
	//internal resource
	}
	}
	return "null";
	}

And here's why: ImageTexture::get_data() uses the VisualServer to return the texture's data:

godot/scene/resources/texture.cpp

Lines 243 to 246 in a0ea57b

	Ref<Image> ImageTexture::get_data() const {
	return VisualServer::get_singleton()->texture_get_data(texture);
	}

which calls

godot/drivers/gles3/rasterizer_storage_gles3.cpp

Line 883 in a0ea57b

Ref<Image> RasterizerStorageGLES3::texture_get_data(RID p_texture, VS::CubeMapSide p_cube_side) const {

which always creates a new Image object

godot/drivers/gles3/rasterizer_storage_gles3.cpp

Lines 959 to 961 in a0ea57b

	Image *img = memnew(Image(texture->alloc_width, texture->alloc_height, texture->mipmaps > 1 ? true : false, img_format, data));
	return Ref<Image>(img);

That means two calls to ImageTexture::get_data() will always return different Image objects, even though the data did not change. That is why the Image can't be found in the internal_resources when looked up the second time.
To verify this is indeed the problem, I replaced

godot/scene/resources/texture.cpp

Lines 243 to 246 in a0ea57b

	Ref<Image> ImageTexture::get_data() const {
	return VisualServer::get_singleton()->texture_get_data(texture);
	}

by

Ref<Image> ImageTexture::get_data() const {
    static auto data = VisualServer::get_singleton()->texture_get_data(texture);

    return data;
}

and now the resource is indeed saved correctly. This "solution" is obviously just a hack and probably breaks 100 other things, but shows that this really is the problem.

[maikramer]

Oh man, 2 days on this and now that i found your work, this bug is REALLY a problem to me, cause its breaking the BakedLightMap generated by code.

I will try your hack

[cmfcmf]

Is there anything else I can do to help fix this? I noticed that this issue hasn't received any labels yet.

[maikramer]

I tried to debug but i'm not able to do so, its simple to test, just create a imagetexture in code and use ResourceSaver to save it.
I have a terrain shader that use a splatmap(ImageTexture), if i create the imageTexture in code, i have almost the same error even without try to save it. my workaround is create the imageTexture in editor and just create the image in code.
My precedural terrain also try to create a baked lightmap but fails in the bake method, also because LightBaker wants a texture saved in disk, this i just gave up and used GIProbe, that bakes by code without problems.
The error you can see in #18954 .
Everything is related to the fact that Godot wants the texture in ResourceCache, this is what i think.
If you Create a ImageTexture in inspector, then try to save it to disk, also doenst work.

If anyone can solve this, the game my team is creating will get very happy.
What i can do to help is test, unfortunatly.
Yes, i'm desperate to have this bug fixed.

[Bauxitedev]

I've also run into this issue in my texture painter. I'm trying to create a save format and I thought I'd just create a tscn file containing a scene tree with all texture slots (albedo, roughness, metalness and emission as 4 different sprites and ImageTextures). Unfortunately the image property of the ImageTexture is null so it cannot be loaded again after saving. It works if I manually replace null with SubResource( 1 ) in the tscn file as you mentioned. Any idea for a workaround or fix? I'd rather not modify Godot's source code.

I was thinking maybe just loading the tscn file afterwards and and automatically replacing all instances of null with SubResource( x ) and keeping a counter x but that won't work if I switch to scn which will save a lot of space when saving big textures.

[cmfcmf]

I was thinking maybe just loading the tscn file afterwards and and automatically replacing all instances of null with SubResource( x ) and keeping a counter x but that won't work if I switch to scn which will save a lot of space when saving big textures.

That's also been my workaround, but, as you said, the tscn files are huge because the images can't be compressed in tscn files.

[rafallus]

I'd the same problem. For me just saving the Image from the TextureImage with
my_image_to_save = my_texture_image.get_data()
worked fine. Then I recover the TextureImage by loading the image and using
my_texture_image.create_from_image(my_loaded_image)

In my case that was enough. If you're using more information from the Texture like flags, you can do
my_image_to_save.set_meta("flags", my_texture_image.flags)
and then on load
my_texture_image.flags = my_loaded_image.get_meta("flags")

[ghost]

possibly related vnen/godot-tiled-importer#97 (but with .tmx)

[AlexHolly]

related #20178

One more example: ResourceSaver saves nothing

	var ts = TileSet.new()
	var id = ts.get_last_unused_tile_id()
	ts.create_tile(id)
	
	var i = Image.new()
	i.create(10,10, true, Image.FORMAT_RGBA4444)
	
	var texture = ImageTexture.new()
	texture.create_from_image(i)
	
	ts.tile_set_texture(id, texture)
	ResourceSaver.save("res://tileset.tres", ts)