Skip to content

Conversation

@Jerozgen
Copy link
Contributor

Before:
before
After:
after

@IMB11
Copy link
Member

IMB11 commented Sep 14, 2025

I think we decided to keep support for alpha within skin textures as there are mods which users can install that allow for the alpha to work in-game (even if it doesnt work in vanilla) - will leave the decision to @AlexTMjugador though.

@IMB11 IMB11 requested a review from AlexTMjugador September 14, 2025 13:57
@IMB11 IMB11 added app Relates to Modrinth App backend Involves work from the backend team labels Sep 14, 2025
@Jerozgen
Copy link
Contributor Author

I created this PR because my skin has transparency utilized by Ears Mod for wings texture, so the head was transparent in the launcher:

Copy link
Contributor

@AlexTMjugador AlexTMjugador left a comment

Choose a reason for hiding this comment

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

For the Modrinth App, I think it makes the most sense to render skins as closely as possible to how the vanilla client does, which is a baseline most users will find reasonable. Since mods can change many aspects of skin rendering in countless ways, trying to account for those variations sounds both impractical and outside our scope.

When I first wrote this code, I chose not to implement transparency handling because the time required to fully understand Mojang's code and test the edge cases related to transparency didn't seem worth it, especially given that many third-party tools get it wrong anyway (so user expectations weren't clear to me, as @IMB11 noted, albeit for different reasons). However, the fact that someone submitted a PR to adjust the behavior to match the vanilla client is a strong signal that we should handle it that way 😄

@Jerozgen
Copy link
Contributor Author

A skin texture buffer is now converted to RGBA. This simplifies other code and allows "Notch transparent hack" to add transparency even if the original skin image is RGB or Grayscale. Also I've added 2 more skins to the normalize_skin_texture_works test

Copy link
Contributor

@AlexTMjugador AlexTMjugador left a comment

Choose a reason for hiding this comment

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

Nice work, thank you! ❤️

I think we can merge this after a few remaining nitpicks are addressed 🚀

@Jerozgen
Copy link
Contributor Author

Done!

@AlexTMjugador AlexTMjugador added this pull request to the merge queue Sep 18, 2025
Merged via the queue into modrinth:main with commit ff50964 Sep 18, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Relates to Modrinth App backend Involves work from the backend team

Development

Successfully merging this pull request may close these issues.

3 participants