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.

Sign up for GitHub

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

Jump to bottom

Convert TileSet Atlas Merge input images to RGBA8 to match output, if needed #80943

Merged
merged 1 commit into from
Aug 28, 2023
Merged

Convert TileSet Atlas Merge input images to RGBA8 to match output, if needed #80943

merged 1 commit into from
Aug 28, 2023

Conversation

rakkarage
Copy link
Contributor

closes: #80870

@Calinou Calinou added bug topic:editor topic:2d cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release labels Aug 23, 2023
@Calinou Calinou added this to the 4.2 milestone Aug 23, 2023
@rakkarage rakkarage force-pushed the tileset-atlas-merge-rgb8 branch 2 times, most recently from 4de6bf6 to 9d4a492 Compare August 23, 2023 20:48
@rakkarage rakkarage force-pushed the tileset-atlas-merge-rgb8 branch from 9d4a492 to dc2aa69 Compare August 23, 2023 23:27
@jonathandw743
Copy link

This fixes a problem that has been bugging me for a while.

@AThousandShips AThousandShips requested a review from a team August 27, 2023 11:12
groud
groud approved these changes Aug 28, 2023
View reviewed changes
Copy link
Member

@groud groud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I was a bit worried that using the result of get_image() could end up in the modification of something internal to the atlas source. However, it seems like get_image() always return a newly allocated Image object, so that should be fine.

@akien-mga akien-mga merged commit 6636ccc into godotengine:master Aug 28, 2023
@akien-mga
Copy link
Member

Thanks!

@jonathandw743
Copy link

Yay

@jonathandw743 jonathandw743 mentioned this pull request Aug 28, 2023
Closed
@kleonc kleonc mentioned this pull request Oct 8, 2023
Closed
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 24, 2023
@Griphen116
Copy link

@YuriSizov is this not included in 4.1.3?
I see you removed the cherrypick label. I'm on 4.1.3 stable and i am still having issues with the atlas merge tool.

@jonathandw743
Copy link

Idk why the cherrypick has been removed, the fix was working very well for me.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 11, 2023

The label is removed when after the commit is cherry-picked, so it is included in the release. Are the issues you experience the same as fixed by this PR?

@Griphen116
Copy link

@KoBeWi Yes, just experienced the same issue today that was demonstrated in the videos linked as clones of this issue. Trying to merge 2 atlases resulted in a blank atlas.

@KoBeWi
Copy link
Member

KoBeWi commented Nov 12, 2023

I tested the MRP from the issue. The bug happens in 4.1.2, but it's fixed in 4.1.3. Your problem is something different, open a new issue if it doesn't exist.

@jonathandw743
Copy link

Are you sure your actually using 4.1.3?

@KoBeWi
Copy link
Member

KoBeWi commented Nov 12, 2023

Yes.

@jonathandw743
Copy link

I meant that for griphen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathandw743 jonathandw743 jonathandw743 approved these changes

@groud groud groud approved these changes

Assignees
No one assigned
Labels
bug topic:editor topic:2d
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.

RGB8 images not shown or copied when TileSet Atlas Merge
8 participants
@rakkarage @jonathandw743 @akien-mga @YuriSizov @Griphen116 @KoBeWi @groud @Calinou