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 Sprite's getColor() behavior #6863

Merged
merged 2 commits into from
May 4, 2022
Merged

Conversation

Raxorg
Copy link
Contributor

@Raxorg Raxorg commented May 4, 2022

As described in issue 6862 getColor() returns a vertex's modified color instead of the color variable stored in the Sprite.

Even if the reason for the current implementation is to keep sync when the user modifies the vertices' colors directly, current getColor() implementation only takes C1 into account.

Thanks to @tommyettinger, @mgsx-dev, @Frosty-J & @MobiDevelop for helping me understand key concepts

@obigu
Copy link
Contributor

obigu commented May 4, 2022

This was changed on aa0d30e2dadbe36d4b549aca3c99c3725333815c.

Pinging @NathanSweet

@NathanSweet
Copy link
Member

I can't remember why I thought it was a good idea back in 2018. I'm OK with just returning color and it is nicer that the alpha doesn't get scaled from float conversion.

Copy link
Member

@tommyettinger tommyettinger left a comment

Choose a reason for hiding this comment

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

I like this a lot; if we track a Color object inside each Sprite, we should use it.

color.r = (intBits & 0xff) / 255f;
color.g = ((intBits >>> 8) & 0xff) / 255f;
color.b = ((intBits >>> 16) & 0xff) / 255f;
color.a = ((intBits >>> 24) & 0xff) / 255f;
Copy link
Member

Choose a reason for hiding this comment

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

I definitely like this removal; the scaling NathanSweet was referring to happens here and in floatToIntColor().

NumberUtils.floatToIntColor() does have some oddities because it tries to create an 8-bit alpha from a 7-bit value. My preference when doing that is to copy the MSB of alpha into the previously-vacant LSB of alpha, like:

    public static int floatToIntColor (float value) {
        int intBits = Float.floatToRawIntBits(value);
        intBits |= (intBits >>> 31) << 24;
        return intBits;
    }

This also avoids some float math.

@tommyettinger tommyettinger merged commit 6e979c3 into libgdx:master May 4, 2022
@Raxorg Raxorg deleted the issue-6862-fix branch May 4, 2022 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants