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

Fix the fetching of images in CF_DIB format in DisplayServerWindows::clipboard_get_image #88220

Conversation

FaycalElOuariachi
Copy link
Contributor

@FaycalElOuariachi FaycalElOuariachi commented Feb 11, 2024

A bug was occuring when trying to import an image from the windows clipbpoard, if it's in CF_DIB format (e. g. by taking screenshots). No image was returned.

It comes from a tiny error : in DisplayServerWindows::clipboard_get_image, Image::create_from_data was used with an instance of Image, but since it's a static function, it returns a new instance instead of modifying the 'called' one.
Correcting this fixes the importation.

Testing with a GDScript

extends TextureRect

func _ready() -> void:
	if DisplayServer.clipboard_has_image() :
		var image = DisplayServer.clipboard_get_image()
		texture = ImageTexture.create_from_image(image)

This script was attached to a TextureRect node in the testing scene.
(At the start, the clipboard must contains an image in CF_DIB format, for instance a screenshot, or some images coming from a web navigator, in the right format, copied with the right-click context menu.)

PS : I apologies if my PR isn't conform to godot contributor's workflow, or for any other problem.

@FaycalElOuariachi FaycalElOuariachi requested a review from a team as a code owner February 11, 2024 22:07
@AThousandShips AThousandShips added this to the 4.3 milestone Feb 12, 2024
@AThousandShips AThousandShips changed the title Fix the import of images in CF_DIB format in DisplayServerWindows::clipboard_get_image Fix the fetching of images in CF_DIB format in DisplayServerWindows::clipboard_get_image Feb 12, 2024
@AThousandShips
Copy link
Member

Your commit seems not to be linked to your GitHub account. See: Why are my commits linked to the wrong user? for more info.

@akien-mga akien-mga changed the title Fix the fetching of images in CF_DIB format in DisplayServerWindows::clipboard_get_image Fix the fetching of images in CF_DIB format in DisplayServerWindows::clipboard_get_image Feb 12, 2024
@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Feb 12, 2024
@FaycalElOuariachi FaycalElOuariachi force-pushed the display_server_windows-clipboard_get_image-fix-DIB-image-import branch from 0d9d5f8 to 3b18d71 Compare February 12, 2024 11:36
@FaycalElOuariachi
Copy link
Contributor Author

Thank you for your help. I tried to amend my commit to fix this. Is it correct now ? Must I completely redo the commit ?

@AThousandShips
Copy link
Member

Seems good now, it's got two authors though, unsure if it matters, could do git commit --amend --reset-author

@FaycalElOuariachi FaycalElOuariachi force-pushed the display_server_windows-clipboard_get_image-fix-DIB-image-import branch from 3b18d71 to 4e990cd Compare February 12, 2024 12:13
…clipboard_get_image

Fix the fetching of images from windows clipboard, if they're in CF_DIB format (e. g. by taking screenshots).

Image::create_from_data was used with an instance of Image, but it's a static function, returning a new instance.
@FaycalElOuariachi
Copy link
Contributor Author

The amend was creating a new commit and a merge commit... Unfortunalty to avoid that, I did something bad by force pushing. Sorry for that... (I'll make sure to be cautious next time)

@akien-mga
Copy link
Member

It looks good now!

@akien-mga akien-mga merged commit f317cc7 into godotengine:master Feb 12, 2024
16 of 24 checks passed
@akien-mga
Copy link
Member

akien-mga commented Feb 12, 2024

Thanks! And congrats for your first merged Godot contribution 🎉

@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.

@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants