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

added handling of numbers for Pixi.js hex format with tests #98

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

weepy
Copy link

@weepy weepy commented Aug 10, 2015

This adds support for raw hex format like in Pixi.js, for example '0xaabbccis the same color as#aabbcc`

@bgrins
Copy link
Owner

bgrins commented Aug 11, 2015

Interesting, I haven't seen that format before

@@ -18,15 +18,19 @@ function tinycolor (color, opts) {
color = (color) ? color : '';
opts = opts || { };


Copy link
Owner

Choose a reason for hiding this comment

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

Nit: can you remove the whitespace changes in this file?

@bgrins
Copy link
Owner

bgrins commented Aug 11, 2015

Does this pixi.js project have some test cases for their hex conversion? Like, how does it handle numbers with > 6 digits?

@bgrins
Copy link
Owner

bgrins commented Aug 11, 2015

There should also be a variation in .toString() for the "number" case that calls .toNumber()

@weepy
Copy link
Author

weepy commented Aug 11, 2015

Pixi uses the numbers internally for the GPU. There's a one-to-one mapping to string hex. e.g 0x0 === "#000000", so I don't think there's really any need for tests.
I didn't add a "number" format, I just converted it to an rgb object, since that was the fastest conversion, so I guess there's no need for a "number" case in .toString() ?

@bgrins
Copy link
Owner

bgrins commented Aug 11, 2015

I didn't add a "number" format, I just converted it to an rgb object, since that was the fastest conversion, so I guess there's no need for a "number" case in .toString() ?

It looks like there was a toNumber function added to the prototype. Usually I have added a matching toString() definition for each of the to* functions added to the prototype but I guess it isn't really necessary for this

@bgrins
Copy link
Owner

bgrins commented Aug 11, 2015

Pixi uses the numbers internally for the GPU. There's a one-to-one mapping to string hex. e.g 0x0 === "#000000", so I don't think there's really any need for tests.

So 0xffffffffffff000000 just ends up becoming #000000? What about -0x00ff00? Does it depend on the GPU?

@weepy
Copy link
Author

weepy commented Aug 11, 2015

Not sure about the big one. 0x00ff00 just becomes 0xff00 which is
interpreted as green. Doesn't depend on gpu as it's using webgl which is
standard across architectures.

On Tue, 11 Aug 2015 18:00 Brian Grinstead notifications@github.com wrote:

Pixi uses the numbers internally for the GPU. There's a one-to-one mapping
to string hex. e.g 0x0 === "#000000", so I don't think there's really any
need for tests.

So 0xffffffffffff000000 just ends up becoming #000000? What about
-0x00ff00? Does it depend on the GPU?


Reply to this email directly or view it on GitHub
#98 (comment).

@bgrins
Copy link
Owner

bgrins commented Aug 11, 2015

0x00ff00 just becomes 0xff00 which is interpreted as green

I had a negative in front of it :). So, -0x00ff00.

@weepy
Copy link
Author

weepy commented Aug 12, 2015

I'm not really sure. Maybe ultraviolet?! ;-)
It probably just comes out as black which is what happens if it's NaN

On Tue, 11 Aug 2015 22:21 Brian Grinstead notifications@github.com wrote:

0x00ff00 just becomes 0xff00 which is interpreted as green

I had a negative in front of it :). So, -0x00ff00.


Reply to this email directly or view it on GitHub
#98 (comment).

@weepy
Copy link
Author

weepy commented Sep 30, 2015

did u decide on this PR ?

On Wed, Aug 12, 2015 at 7:38 AM Jonah Fox jonahfox@gmail.com wrote:

I'm not really sure. Maybe ultraviolet?! ;-)
It probably just comes out as black which is what happens if it's NaN

On Tue, 11 Aug 2015 22:21 Brian Grinstead notifications@github.com
wrote:

0x00ff00 just becomes 0xff00 which is interpreted as green

I had a negative in front of it :). So, -0x00ff00.


Reply to this email directly or view it on GitHub
#98 (comment).

@alesdotio
Copy link

I would love to see this merged in. It's very useful when using libraries like pixi.js

@bgrins
Copy link
Owner

bgrins commented Mar 20, 2016

Sorry, for the slow response. This pixi.js library is the only place I've seen this format used so far, so I'm hesitant to add support for it into the main library.

Maybe what we need is some kind of extension API where someone could register id, shouldHandleInput(input) -> bool, toRgb(input) -> {r,g,b,a} and toString({r,g,b,a}) -> string then the lib could loop through extensions and send tinycolor(0xaabbcc) into the toRgb method if shouldHandleInput is true. Then when tinycolor(0xaabbcc).toString("number") is called it would defer to the extension's method.

@danwad
Copy link

danwad commented Apr 19, 2016

I would find this useful, either built in or as an extension.

For now, I have been passing colours through pixi's utility functions for converting between hex and rgb/string.

https://github.com/pixijs/pixi.js/blob/master/src/core/utils/index.js#L24-L65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants