-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[ASTextNode] Fix ASTextNode shadow is not rendering #2042
[ASTextNode] Fix ASTextNode shadow is not rendering #2042
Conversation
| if (shadowColor != NULL) { | ||
| CGColorRetain(shadowColor); | ||
| } | ||
| _shadowColor = shadowColor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you're already in this code, could you add if (_shadowColor != NULL) { CGColorRelease(_shadowColor); } so we stop leaking them 🚰
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On it 👷
|
Love this, let's land it ASAP. Thoughts about comment @maicki ? |
|
@Adlai-Holler Fixed the memory leak ready to go |
| if (_shadowColor != shadowColor) { | ||
| if (shadowColor != NULL) { | ||
| CGColorRetain(shadowColor); | ||
| CGColorRelease(_shadowColor); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... I think we need to move this out of the if statement and wrap it in a new one: if (_shadowColor != NULL) because no matter what the new value is, if we have an old value we must release it
|
Sweet! |
* Passing through shadow in renderer attribute * Fix memory leak setting shadow color
…2042) * Passing through shadow in renderer attribute * Fix memory leak setting shadow color
* Passing through shadow in renderer attribute * Fix memory leak setting shadow color
We didn't pass through shadow properties via the renderer attributes. That meant that the renderer shadower didn't get those and so setting a shadow on the text node did not have any effect.