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

Can not hydrate an image from data and load into a TextureRect #42346

Closed
xarses opened this issue Sep 25, 2020 · 8 comments
Closed

Can not hydrate an image from data and load into a TextureRect #42346

xarses opened this issue Sep 25, 2020 · 8 comments

Comments

@xarses
Copy link
Contributor

xarses commented Sep 25, 2020

Godot version:
v3.2.3.stable.flathub

OS/device including version:
Linux Ubuntu (PopOS)

Issue description:
I'm trying to replace the image used in a TextureRect with a Image that has been created from Image.new() (which has been regurgitated from from a json file containing a base64 encoded png data (similar to #25571 (comment)).

Proof the data isn't corrupt
We can prove the image data is fine if we marshal the data to disk, however this is only usable if the engine editor is brought back into focus to do "some magic" with the file (the first call to write and read the image from a res:// will always fail)

func _on_FromSave_pressed():
	var file = File.new()
	file.open("res://temp.png", File.WRITE)
	file.store_buffer(Marshalls.base64_to_raw(ref_data))
	file.close()
	
	$Icon.texture = load("res://temp.png")
	$Sprite.texture = load("res://temp.png")

Method 1: try to attach an image to the TextureRect
I figured I'd give it a shot with whatever magic the editor does for us. I know its most likely is wrong, but raises no errors so 🤷

func _on_ImageTexture1_pressed():
	var image = Image.new()
	image.load_png_from_buffer(Marshalls.base64_to_raw(ref_data))
	
	$Icon.texture = image

Method 2: try some version of creating an ImageTexture and attaching that
you can create a new ImageTexture and do an set_data() on it directly or calling create_from_image() neither seem to work. Also as an aside why do you have to instance ImageTexture just to create a new one from an image??

func _on_ImageTexture2_pressed():
	var image = Image.new()
	image.load_png_from_buffer(Marshalls.base64_to_raw(ref_data))

	var it = ImageTexture.new()
	var texture = it.create_from_image(image)

	$Icon.texture = load("res://temp.png")```

I've tried some other ways like spawning other texture resources, but they result in errors similar to #23574 which gives no context to the error or how to avoid it other than "use load()"

Steps to reproduce:

Minimal reproduction project:

BrokenImport.zip

@Calinou
Copy link
Member

Calinou commented Sep 25, 2020

Also as an aside why do you have to instance ImageTexture just to create a new one from an image??

GDScript doesn't allow exposing static methods in built-in classes (unless they're singletons, in which case they appear to be static but aren't).

@xarses
Copy link
Contributor Author

xarses commented Sep 26, 2020

Ok, update time, I dug through the code and found a reference to texture.is_valid() before rendering the update. So I decided to ad copious print statements to see what may be going on.

Method 1:

func _on_ImageTexture1_pressed():
	var image = Image.new()
	image.load_png_from_buffer(Marshalls.base64_to_raw(ref_data))
	print(str(image))
	
	$Icon.set_texture(image)
	print("TextRect Texture is: " + str($Icon.get_texture()))
	$Sprite.texture = image
	print("Sprite Texture is: "+ str($Sprite.get_texture()))

which results in

[Image:1541]
TextRect Texture is: [Object:null]
Sprite Texture is: [Object:null]

So there appears to be difficulty setting the texture from an Image variant, which I though maybe should have worked. (Jury is still out if this should work in the first place)

Method 2:

func _on_ImageTexture2_pressed():
	var image = Image.new()
	image.load_png_from_buffer(Marshalls.base64_to_raw(ref_data))

	var it = ImageTexture.new()
	var texture = it.create_from_image(image)
	it.set_data(image)
	print(str(it))
	print(str(texture))

	$Icon.set_texture(texture)
	print("Texture is: " + str($Icon.get_texture()))
	$Sprite.texture = it
	print("Sprite Texture is: "+ str($Sprite.get_texture()))

This results in


[ImageTexture:1501]
Null
Texture is: [Object:null]
Sprite Texture is: [ImageTexture:1501]

Interestingly calling create_from_image(image) from an instance of ImageTexture results in a Null.

BUT If we set_data(image) we do get an texture back yay! And if you imagine, it also works if we assign it to a texture attribute. YAY!!

Open problems:

  • Setting a texture attribute from a Image variant doesn't work. Expected?
  • imagetexture.create_from_image(image) doesn't return a texture from a image created in this manner
  • Writing an image into the resource, and then trying to loading it again results in a file-not-found error until the IDE comes back into focus or the game is relaunched.

Updated repo project: BrokenImportv2.zip

@xarses
Copy link
Contributor Author

xarses commented Sep 26, 2020

Back again, the working call isn't set_data(image) that doesn't work. the working call was in fact create_image_from_data(image), also, some noob (me) didn't realize that it returns void x.x.

A working call is

	var image = Image.new()
	image.load_png_from_buffer(Marshalls.base64_to_raw(ref_data))

	var texture = ImageTexture.new()
	texture.create_from_image(image)

	$Icon.set_texture(texture)
	print("Texture is: " + str($Icon.get_texture()))

@Xrayez
Copy link
Contributor

Xrayez commented Sep 26, 2020

  • Setting a texture attribute from a Image variant doesn't work. Expected?

Yes, this is expected, the Image class acts as a container for image data and provides image processing methods on that data. The Texture is closer to GPU terms which can be actually rendered on the screen.

And please lets not rename Image to ImageData because of this. 😛

The current documentation seems to be explicit about this, but likely just lacks examples for those common conversion operations.

  • imagetexture.create_from_image(image) doesn't return a texture from a image created in this manner

If I understand correctly, Godot follows single-responsibility principle for methods, see example discussions in #30628 and #33926. It would just return the current texture (imagetexture), so I don't see the point in this case.

  • Writing an image into the resource, and then trying to loading it again results in a file-not-found error until the IDE comes back into focus or the game is relaunched.

I think I've seen a bug report/enhancement proposal for this, yes.

It also depends, if you embed the Image resource as an exported property of a scene and set it at run-time and save it (also possible with tool scripts), it will be serialized into the scene as an array of bytes. If you really need to do that, you can explicitly re-import the image files as Image resources as well via the Import tab, and load calls will return Image rather than Texture.

@xarses
Copy link
Contributor Author

xarses commented Sep 30, 2020

@Xrayez the new documentation you posted clearly closes out method 1 as invalid, and clarifies that the texture.create_from_image(image) method is the correct path. Thanks, that should be very helpful for others.

This leaves texture.set_data(image) doesn't appear to work from the image data I have, yet create_from_image does 🤷

Lastly, if you happen to update the docs for create_from_image

Create a new ImageTexture from an Image

I think this is what threw me expecting a new object to be returned from the call instead of listening to the plainly explain just above that it returns void

@Xrayez
Copy link
Contributor

Xrayez commented Sep 30, 2020

This leaves texture.set_data(image) doesn't appear to work from the image data I have, yet create_from_image does 🤷

In Godot master, there's texture.update(image) and/or texture.update(image, true), maybe those help to alleviate the problem, I don't know. I think you may still need to initialize the texture with create_from_image first, and then perhaps set_data exists to allow you optimize frequent updates of the existing texture without allocating another one.

Create a new ImageTexture from an Image

I think this is what threw me expecting a new object to be returned from the call instead of listening to the plainly explain just above that it returns void

Yes, I suspected that this wording is what lead to the confusion. 🙂

I've looked up ImageTexture.create_from_image source in latest version (master):

void ImageTexture::create_from_image(const Ref<Image> &p_image) {
ERR_FAIL_COND(p_image.is_null());
w = p_image->get_width();
h = p_image->get_height();
format = p_image->get_format();
mipmaps = p_image->has_mipmaps();
if (texture.is_null()) {
texture = RenderingServer::get_singleton()->texture_2d_create(p_image);
} else {
RID new_texture = RenderingServer::get_singleton()->texture_2d_create(p_image);
RenderingServer::get_singleton()->texture_replace(texture, new_texture);
}
_change_notify();
emit_changed();
image_stored = true;
}

The implementation is somewhat different when comparing to 3.2 version:

void ImageTexture::create_from_image(const Ref<Image> &p_image, uint32_t p_flags) {
ERR_FAIL_COND(p_image.is_null());
flags = p_flags;
w = p_image->get_width();
h = p_image->get_height();
format = p_image->get_format();
VisualServer::get_singleton()->texture_allocate(texture, p_image->get_width(), p_image->get_height(), 0, p_image->get_format(), VS::TEXTURE_TYPE_2D, p_flags);
VisualServer::get_singleton()->texture_set_data(texture, p_image);
_change_notify();
emit_changed();
image_stored = true;
}

So, this can be documented as:

Initializes the texture by allocating and setting the data from an [Image].

I think this shouldn't matter much because those are implementation details.

Sounds good?

@xarses
Copy link
Contributor Author

xarses commented Oct 1, 2020

Thanks for reminding me to look at the code. We can see in 3.2 that set_data doesn't do the texture_allocate so ya, it must only work as an update function. in master, I don't see it at all. I see _set which probably works out to _set("image", <image variant>) which will in turn call create_from_image

This probably implies that we should annotate the 3.2 set_data is an update function, and will only work after create_from_image and will not resize the texture, and that they probably don't want to use this function, its deprecated anyways

@xarses
Copy link
Contributor Author

xarses commented Oct 2, 2020

Documentation updates in #42412 resolve this issue

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

No branches or pull requests

4 participants