pixijs / pixijs

The HTML5 Creation Engine: Create beautiful digital content with the fastest, most flexible 2D WebGL renderer.
http://pixijs.com
MIT License
43.86k stars 4.78k forks source link

Bug: Array color used to tint a Sprite it changes the original color array values #10882

Closed rperello closed 1 month ago

rperello commented 2 months ago

Current Behavior

Setting the tint property of Sprite if the color is an array, because it is ._normalize method is applied directly to the passed color but not the cloned one, it causes the original color to be modified causing a side effect on the application using Pixi.js

Expected Behavior

I expect that the color passed is not modified. After looking at the Color.ts file I guess that the clone intends to avoid that color is modified from outside Pixi.js once it has been set. Aligned to that intention normalize should be applied to the cloned color.

Steps to Reproduce

I posted a pixiplayground link so it is clear how to reproduce it.

https://pixiplayground.com/#/edit/xX09VNY0_G1XCeHTd5iRr

Environment

Possible Solution

On value setter of Color class just call the normalize method on the already cloned color.

         else if (this._value === null || !this._isSourceEqual(this._value, value))
         {
-            this._normalize(value);
             this._value = this._cloneSource(value);
+            this._normalize(this._value);
         }

Additional Information

No response

bigtimebuddy commented 2 months ago

Makes sense and I agree that seems like a bug. Your solution sounds perfectly reasonable. Fancy making a PR for that?

rperello commented 2 months ago

Of course. I will target dev branch for the PR