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

Interpolating arrays and support for rgba values #136

Closed
hoomanaskari opened this issue Nov 27, 2021 · 9 comments
Closed

Interpolating arrays and support for rgba values #136

hoomanaskari opened this issue Nov 27, 2021 · 9 comments

Comments

@hoomanaskari
Copy link
Contributor

hoomanaskari commented Nov 27, 2021

Hi,

I really love what you have done here.

I have 1 small issue with Shifty, however. I cannot interpolate an array of numbers, and if I use Array.join to use the string interpolation, I can only use an array of length <= 3 (which I think is related to RGB support and the fact that RGBA is not yet supported) I think it would be a nice addition to this library.

If for example, I use an array of size 4 I get this in the state of render function ['39.5417', '39.5417', '39.5417', ''] as you can see, after the 3rd number it returns empty strings.

@hoomanaskari hoomanaskari changed the title Interpolating arrays and rgba Interpolating arrays and support for rgba values Nov 27, 2021
@jeremyckahn
Copy link
Owner

Hi @hoomanaskari, thanks for checking out Shifty and opening this issue! I didn't realize that Shifty didn't support RGBA colors, but after some quick testing I can see that you're right. Sorry about that! I don't have much time for active Shifty development these days, but I will try to get in a fix for that soon. Feel free to make a PR if you don't want to wait though, and I'll work with you to get it merged.

As for array interpolation, what's the use case you have in mind for that? Was it just RGBA interpolation?

@hoomanaskari
Copy link
Contributor Author

Thanks for responding back @jeremyckahn.

The array interpolation in my case is for a rectangle with 4 corner radius values, I would like to be able to animate those values separately (or all at once) in the canvas.

I am using Rekapi to make timelines, so right now the only way to do it is by adding 4 separate keyFrames, one for each corner. While it works, it's not the ideal solution for it, especially if I want to have more values in the array (like in a rounded polygon shape)

I may be able to help with a pull request if you can point me in the right direction, which function in which file needs to be modified to support the array interpolation.

For RGBA support, I believe we need to add another regex for it?

Thanks for the help.

@jeremyckahn
Copy link
Owner

The array interpolation in my case is for a rectangle with 4 corner radius values, I would like to be able to animate those values separately (or all at once) in the canvas.

Ah, I see. Would it be feasible to tween an object that looks something like:

{
  top_left: 0,
  top_right: 0,
  bottom_left: 0,
  bottom_right: 0,
}

And apply those values as necessary in your render function? If necessary, I think a custom Shifty filter could be implemented for your project to support arrays as you're describing, but I feel that might be overkill for your needs.

I am using Rekapi to make timelines, so right now the only way to do it is by adding 4 separate keyFrames, one for each corner. While it works, it's not the ideal solution for it, especially if I want to have more values in the array (like in a rounded polygon shape)

That makes sense. It sounds like you're working with an arbitrary number of enumerable properties. Although it's a little awkward, could you structure things around a set of keyframe/tween properties that are named something like pointX, where X is the index number of the value? It's a little hard to give good guidance without seeing the code myself, but that's what comes to mind for me.

BTW, I'd love to see what you're building with Shifty/Rekapi, if that's something you can share! :)

I may be able to help with a pull request if you can point me in the right direction, which function in which file needs to be modified to support the array interpolation.

For RGBA support, I believe we need to add another regex for it?

Thank you for offering to help! I think the only file we'll need to change is https://github.com/jeremyckahn/shifty/blob/develop/src/token.js. That file provides all the string interpolation support for Shifty and works as a plugin via the filter API I mentioned above:

Tweenable.filters.token = token

There's also a lot of special case logic in that file to support color strings efficiently. Here's the specific RGB prefix:

shifty/src/token.js

Lines 4 to 9 in 199774d

const R_RGB = (() => {
const number = R_UNFORMATTED_VALUES.source
const comma = /,\s*/.source
return new RegExp(`rgb\\(${number}${comma}${number}${comma}${number}\\)`, 'g')
})()

I think we'll need more than simply updating that RegExp to support 'a' values. Specifically, I think this function will need to be revisited to support both variants as well:

shifty/src/token.js

Lines 80 to 95 in 199774d

const hexToRGBArray = hex => {
hex = hex.replace(/#/, '')
// If the string is a shorthand three digit hex notation, normalize it to
// the standard six digit notation
if (hex.length === 3) {
hex = hex.split('')
hex = hex[0] + hex[0] + hex[1] + hex[1] + hex[2] + hex[2]
}
return [
hexToDec(hex.substr(0, 2)),
hexToDec(hex.substr(2, 2)),
hexToDec(hex.substr(4, 2)),
]
}

There may be more that's needed here, but this should at least be enough to get started with. Sorry about the messiness of some of this code; I haven't touched it in quite a while! 😅 There are tests for this plugin at https://github.com/jeremyckahn/shifty/blob/develop/src/token.test.js which should help to validate a potential solution.

Let me know how it goes! Sorry I don't have time to fix this myself right now, but my hands are quite full with other projects and I only have bandwidth to support others with Shifty PRs at the moment. That said, let me know if I can answer any questions or otherwise help here!

@hoomanaskari
Copy link
Contributor Author

hoomanaskari commented Nov 29, 2021

Wow, thanks for the tweet, and thanks for the extensive answer.

I poked around the code a bit, and you were right, the RGBA support might need a little bit of time to set it up and running. I will come back to it if I needed it in my project. (I may be able to get away with it by doing some color conversion tricks).

After having a look at the tokens.js I realized all colors are being converted to hex before being enumerated? Or did I miss something? If that is the case, then adding support for hex with alpha (#RRGGBB[AA]) might do the job?

Also, I could not find where the numerical values are being enumerated? In tokens.js I could only find the string interpolations. Finding that function might help me with adding support for Arrays.

For the array enumeration, I ended up creating 4 different keyFrames:

if (action === 'cornerRadius') {
  if (typeof target.cornerRadius === 'number') {
    addOrUpdateKeyframes(actor, millisecond, target.cornerRadius, 'cornerRadius');
  } else {
    addOrUpdateKeyframes(actor, millisecond, target.cornerRadius[0], 'cornerRadius0');
    addOrUpdateKeyframes(actor, millisecond, target.cornerRadius[1], 'cornerRadius1');
    addOrUpdateKeyframes(actor, millisecond, target.cornerRadius[2], 'cornerRadius2');
    addOrUpdateKeyframes(actor, millisecond, target.cornerRadius[3], 'cornerRadius3');
  }
}

/**
 *
 * @param {Rekapi.Actor} actor - Rekapi actor
 * @param {number} millisecond
 * @param {number|string} value
 * @param {string} property - the property to be animated on the target
 */
function addOrUpdateKeyframes(actor, millisecond, value, property) {
  if (actor.hasKeyframeAt(millisecond, property)) {
    actor.modifyKeyframeProperty(property, millisecond, { value });
  } else {
    const leftProperty = new KeyframeProperty(millisecond, property, value, 'linear');
    actor.addKeyframeProperty(leftProperty);
  }
}

// inside the render function
const settings = {};

if (track.includes('cornerRadius')) {
  if (typeof object.cornerRadius === 'number') {
    settings.cornerRadius = state[track];
  } else {
    settings.cornerRadius = object.cornerRadius || [0, 0, 0, 0];
    const index = track.split('cornerRadius')[1];
    settings.cornerRadius[Number(index)] = state[track];
  }
}

As you can see it's not the most elegant solution out there, but it does the job for now.

I tried your approach for passing the following:

{
  top_left: 0,
  top_right: 0,
  bottom_left: 0,
  bottom_right: 0,
}

But in Rekapi I have one option to pass the value, and if I pass anything other than a number or a string, then it won't enumerate it correctly. For array and object, I get the [Object Object]NaN while rendering the animation.

I would love to work on both Shifty and Rekapi to fix these, and will as soon as I get some free time, there is a ton of potential in these two libraries.

We are bringing our Animation feature out of Beta in Artboard V3.0 and I ended up rewriting the whole thing, it's gonna be much much better than what it is right now. You can animate multiple frames at the same, there are so many other values that can now be animated, etc.

V3.0 is right now in closed Alpha, but I will definitely send you an invite to have a look as soon as it's presentable :)

I will keep digging into Shifty and Rekapi and will get back to you for more questions :))

@jeremyckahn
Copy link
Owner

After having a look at the tokens.js I realized all colors are being converted to hex before being enumerated? Or did I miss something? If that is the case, then adding support for hex with alpha (#RRGGBB[AA]) might do the job?

It's actually the other way around. Here's where the token plugin starts its lifecycle when a tween is created:

shifty/src/token.js

Lines 363 to 371 in 199774d

export function tweenCreated(tweenable) {
const { _currentState, _originalState, _targetState } = tweenable
;[_currentState, _originalState, _targetState].forEach(
sanitizeObjectForHexProps
)
tweenable._tokenData = getFormatSignatures(_currentState)
}

It runs sanitizeObjectForHexProps to preprocess the state data:

shifty/src/token.js

Lines 149 to 157 in 199774d

const sanitizeObjectForHexProps = stateObject => {
for (const prop in stateObject) {
const currentProp = stateObject[prop]
if (typeof currentProp === 'string' && currentProp.match(R_HEX)) {
stateObject[prop] = sanitizeHexChunksToRGB(currentProp)
}
}
}

As you can see, it's actually converting from hex to rgb, since working with those numerical values is more efficient. Here's how the conversion works:

shifty/src/token.js

Lines 140 to 141 in 199774d

const sanitizeHexChunksToRGB = str =>
filterStringChunks(R_HEX, str, convertHexToRGB)

shifty/src/token.js

Lines 117 to 132 in 199774d

const filterStringChunks = (pattern, unfilteredString, filter) => {
const patternMatches = unfilteredString.match(pattern)
let filteredString = unfilteredString.replace(pattern, VALUE_PLACEHOLDER)
if (patternMatches) {
patternMatches.forEach(
match =>
(filteredString = filteredString.replace(
VALUE_PLACEHOLDER,
filter(match)
))
)
}
return filteredString
}

shifty/src/token.js

Lines 103 to 104 in 199774d

const convertHexToRGB = hexString =>
`rgb(${hexToRGBArray(hexString).join(',')})`

I think the first part of the solution for this bug is to change this logic flow to generically handle both rgb and rgba formats. the second part is to revisit the afterTween logic flow, which basically reverses the preprocessing outlined above. Once a tween completes, it triggers this filter hook:

shifty/src/token.js

Lines 388 to 401 in 199774d

export function afterTween(tweenable) {
const {
_currentState,
_originalState,
_targetState,
_easing,
_tokenData,
} = tweenable
;[_currentState, _originalState, _targetState].forEach(state =>
collapseFormattedProperties(state, _tokenData)
)
collapseEasingObject(_easing, _tokenData)
}

Which calls collapseFormattedProperties on the optimized data:

shifty/src/token.js

Lines 287 to 298 in 199774d

const collapseFormattedProperties = (stateObject, formatSignatures) => {
for (const prop in formatSignatures) {
const { chunkNames, formatString } = formatSignatures[prop]
const currentProp = getFormattedValues(
formatString,
getValuesList(extractPropertyChunks(stateObject, chunkNames), chunkNames)
)
stateObject[prop] = sanitizeRGBChunks(currentProp)
}
}

As we can see, this calls the rather suspect-looking sanitizeRGBChunks:

shifty/src/token.js

Lines 180 to 181 in 199774d

const sanitizeRGBChunks = formattedString =>
filterStringChunks(R_RGB, formattedString, sanitizeRGBChunk)

Which again uses filterStringChunks as before, but this time with sanitizeRGBChunk:

shifty/src/token.js

Lines 165 to 170 in 199774d

const sanitizeRGBChunk = rgbChunk => {
const numbers = rgbChunk.match(R_UNFORMATTED_VALUES).map(Math.floor)
const prefix = rgbChunk.match(R_RGB_PREFIX)[0]
return `${prefix}${numbers.join(',')})`
}

Just like the first half of the solution, I think this logic flow will also have to be changed to generically handle both rgb and rgba values. At least, that's how I'd likely approach it. I'll give this approach a try when I can as you've found a legitimate bug with Shifty, but again I am happy to let you run with this if it's something you need sooner rather than later.


Also, I could not find where the numerical values are being enumerated? In tokens.js I could only find the string interpolations. Finding that function might help me with adding support for Arrays.

I think this is the function you're looking for:

shifty/src/token.js

Lines 195 to 217 in 199774d

/**
* @param {Object} stateObject
*
* @return {Object} An Object of formatSignatures that correspond to
* the string properties of stateObject.
* @private
*/
const getFormatSignatures = stateObject => {
const signatures = {}
for (const propertyName in stateObject) {
const property = stateObject[propertyName]
if (typeof property === 'string') {
signatures[propertyName] = {
formatString: getFormatStringFrom(property),
chunkNames: getFormatChunksFrom(getValuesFrom(property), propertyName),
}
}
}
return signatures
}

It determines the data structure for each string property from the tweenCreated filter hook. The inverse is the afterTween cleanup phase that calls collapseFormattedProperties, which was shown above.

Let me know if that answers your question!

As you can see it's not the most elegant solution out there, but it does the job for now.

Your sample code seems as reasonable as anything! Animation timeline data structures are so intricate that code like this is unavoidable when you're working with more complex animations. I wouldn't sweat it. :)

But in Rekapi I have one option to pass the value, and if I pass anything other than a number or a string, then it won't enumerate it correctly. For array and object, I get the [Object Object]NaN while rendering the animation.

Ah, my suggestion was in terms of Shify's API, not Rekapi's. Rekapi's KeyframePropertys will only handle plain Numbers and Strings; you'd need a separate KeyframeProperty for each of these.

I would love to work on both Shifty and Rekapi to fix these, and will as soon as I get some free time, there is a ton of potential in these two libraries.

Yay I'm so glad to hear that! Both libraries are at a point where they meet my needs, so I'm not actively developing either of them right now. However, I take project maintenance pretty seriously, so I do my best to fix and address things as they come up. Shifty has gotten a lot more community interaction over the years, so that's had more frequent updates. That being said, I don't consider either project "dead" and am happy to work with you on Issues and Pull Requests. So just let me know what you need from me! :)

We are bringing our Animation feature out of Beta in Artboard V3.0 and I ended up rewriting the whole thing, it's gonna be much much better than what it is right now. You can animate multiple frames at the same, there are so many other values that can now be animated, etc.

V3.0 is right now in closed Alpha, but I will definitely send you an invite to have a look as soon as it's presentable :)

That sounds awesome! I'm really impressed with the current version of Artboard and can't wait to see what you've got in the works. Thanks for offering me an invite!

I will keep digging into Shifty and Rekapi and will get back to you for more questions :))

Sounds good! 👍

@hoomanaskari
Copy link
Contributor Author

Thanks as always for the extensive explanation.

I have some good news. The bug with RGBA was in my codes not in yours. I tried again today to compare the results with Shifty and RGBA is working perfectly fine. I think both Shifty and Rekapi are more perfect than we thought :D

I think we can close this issue now. I will keep on working on the array/object enumeration, if I come up with something I will definitely create a PR for it.

That sounds awesome! I'm really impressed with the current version of Artboard and can't wait to see what you've got in the works. Thanks for offering me an invite!

Thank you for your heartwarming words :)))

@jeremyckahn
Copy link
Owner

I have some good news. The bug with RGBA was in my codes not in yours. I tried again today to compare the results with Shifty and RGBA is working perfectly fine.

I'm so glad to hear that! I made a Codepen to test the functionality, and indeed it's tweening the rgba values correctly: https://codepen.io/jeremyckahn/pen/BawBjbR

I'm actually a little surprised to see this working as well as it is since rgb values are handled as a special case in the code whereas rgba values are not. The token filter supports arbitrary string formats, so perhaps explicit rgb support isn't even needed. I'm going to leave this open as a reminder to myself to investigate this and maybe clean up the code a little bit. It's been a while since I took a close look at token.js!

At least there's no bugs that need to be urgently fixed! 😄

@jeremyckahn
Copy link
Owner

@hoomanaskari I realized why the RGB code was needed: It was preventing rgb strings from having decimal values. However, Shifty was not handling rgba values appropriately. That's been fixed by #137 and released as v2.16.1!

Thanks for bringing this to my attention! 🙂

@hoomanaskari
Copy link
Contributor Author

Oh wow, that was fast, thank you for fixing it :)

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

No branches or pull requests

2 participants