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

fix offset math in ColorTransform.concat #2653

Closed
wants to merge 5 commits into from

Conversation

Geokureli
Copy link
Contributor

in this thread Thomas outlined the expected offset formula:

concat_offset = multiplier2*offset1 + offset2;

Joshua replied that it was implemented, however the formula used was:

concat_offset = multiplier1*offset2 + offset1;

the doc states

the effect is the same as applying the second color transformation after the original color transformation.

but the previous code has this backwards

Came across this when I was implementing flixel tools for adobe animate's tint and brightness color transforms, I noticed that concatenating a full brightness or full tint on top of another transform did not completely override the initial transforms values.

@Geokureli
Copy link
Contributor Author

Geokureli commented Jul 31, 2023

I'm not very well versed in unit testing visual utilities like this. For test_concat1, I clearly just took the new values and put then as the expected value, I feel like this is not great practice, and I'm open to suggestions

Edit: I see, so the test's purpose is to ensure that openfl behaves identical to air/flash, and it appears that my changes do not, perhaps concat's doc is incorrect?

Is my interpretation of the doc, wrong?

@Geokureli Geokureli closed this Jul 31, 2023
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

Successfully merging this pull request may close these issues.

None yet

1 participant