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 transparency in skin icon #1513

Merged

Conversation

lumiscosity
Copy link
Contributor

QPainter has a bug where drawing transparency to a freshly initialized, empty QPixmap causes garbage data to be drawn. This broke the rendering of the skin icon. The fix is simply to fill the QPixmap with empty transparent pixels beforehand.

Signed-off-by: lumiscosity averyrudelphe@gmail.com

Copy link
Member

@TheKodeToad TheKodeToad left a comment

Choose a reason for hiding this comment

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

Just a suggestion, if you want to keep it the same please format it

launcher/SkinUtils.cpp Outdated Show resolved Hide resolved
launcher/minecraft/auth/MinecraftAccount.cpp Show resolved Hide resolved
@lumiscosity
Copy link
Contributor Author

sorry, i wasn't aware such a constant existed! it's been added now

@TheKodeToad
Copy link
Member

TheKodeToad commented Aug 10, 2023

You forgot to sign off your commits :P
Edit: I don't know what that is about

@Trial97
Copy link
Member

Trial97 commented Aug 10, 2023

Maybe its because of some GitHub shenanigans maple! vs lumiscosity :
in the first commit the signoff is with maple! but for the rest is lumiscosity

From DCO error message:
image

@lumiscosity
Copy link
Contributor Author

lumiscosity commented Aug 10, 2023

could be; i manually amended the first commit to include the signoff text, and the latter two were accepted with github's "Sign off and commit suggestion" button. sorry, i've never made a pr before

@Scrumplex Scrumplex added this to the 8.0 milestone Aug 10, 2023
@Scrumplex Scrumplex added bug Something isn't working changelog:fixed A PR that appears under "Fixed" in the changelog labels Aug 10, 2023
@Scrumplex
Copy link
Member

sorry, i've never made a pr before

Don't worry about it :D

It is quite unintuitive honestly

Thanks for your contribution, either way :D

@Scrumplex
Copy link
Member

when taking a look at the log, your first commit was authored by maple! while the sign off says lumiscosity. The latter should also be maple!, so ideally amend it again and change lumiscosity to maple!

QPainter has a bug where drawing transparency to a freshly initialized, empty QPixmap causes garbage data to be drawn. This broke the rendering of the skin icon. The fix is simply to fill the QPixmap with empty transparent pixels beforehand.

Signed-off-by: maple! <averyrudelphe@gmail.com>
@lumiscosity
Copy link
Contributor Author

alright, i asked a friend who's more knowledgeable about this stuff who told me to just wipe the whole thing and recommit it. should be fine now

@TheKodeToad TheKodeToad removed the request for review from Scrumplex August 10, 2023 15:50
@TheKodeToad
Copy link
Member

I think we're waiting for #1486 to be merged before merging any prs targetting develop

@Scrumplex
Copy link
Member

this should fix the build on the older Qt 5 version used in one of the Linux builds

@Scrumplex
Copy link
Member

cough

@TheKodeToad
Copy link
Member

oh, I forgot we were using older qt versions than that

Signed-off-by: Sefa Eyeoglu <contact@scrumplex.net>
@Scrumplex Scrumplex merged commit 8f5bb98 into PrismLauncher:develop Aug 15, 2023
25 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working changelog:fixed A PR that appears under "Fixed" in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants