Closed TehShrike closed 7 years ago
hmm. I'm not sure why this pull request passes on node 4, but #23 and https://github.com/toish/chromatism/commit/71f2c1c882b81cd70e8c585eeee259e00dc5d32d didn't pass when they ran.
I've since made some tweaks to the current conversion chain, I'll make the tweaks! This should ease out the performance hit from my search a bit.
Actually, just noticed you have RGB>HSV, would prefer RGB>HSL>HSV, as HSV is just a value-shifted version of HSL, so a lot of the code is the same. RGB>HSL>HSV would cut down on repeated code
I believe that the current implementation exactly matches the conversion steps that would have been taken before. That said, I have no problem implementing improvements in this pull request.
Made that change, saves a nice little bit of code. While making that change, I wondered: do we need both HSV->RGB and HSL->RGB conversion functions? Could we eliminate one of them?
@TehShrike Nope, don't need those either! Other then the over head to switch modes, should be about the same speed to back the same way
hmm, if I try to eliminate either of those conversions, the output values change enough that the tests don't pass. I'll open up an issue for that, and leave that out of this pull request.
Sorry for the absence on this! Working on the HSL/V => RGB conversion fix at the moment, merging this in to keep things organized
Using the
getChain
function, I generated a map of type strings that let you look up, for any from->to combination, the conversion steps that were between you and that type. I got this map:Then I shortened each list of steps to only give you the next step (since you can look back at the map to figure out what to do after that). That gets you:
I shortened that data structure down to this slim version by hand.
Unlike the currently-committed conversion function, or my original breadth-first search implementation, It doesn't involve iterating over any arrays.
I merged #23 into this branch to pre-emptively fix the merge conflicts.