Skip to content

Fix rotated frames not rendering on Hashlink #3454

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

Merged
merged 1 commit into from
Jul 30, 2025

Conversation

rifxii
Copy link
Contributor

@rifxii rifxii commented Jul 28, 2025

Closes #3433

This seems to solve the previously mentioned issues I had with rotated frames not rendering on Hashlink.

Judging by what I saw while testing, it seems that the HL compiler is inlining MatrixVector.set by using pointers of the given variable. So instead of the set function in MatrixVector.copyFrom performing this[0] = -b; this[1] = a;, it's instead doing this[0] = -this[1]; this[1] = this[0]; and messing up the whole matrix.

I also found setting a reference to the variables before set() also fixes the issue WITHOUT the removal of the inline but I'm not sure which one is better to deal with and if there's any sort of performance loss from removing the inline.

Example:

#if hl
final a:Float = a; 
final b:Float = b;
#end

set(-b, a, ...);

@MaybeMaru
Copy link
Contributor

If you haven't already this should probably be reported to the Hashlink or Haxe repo too. Seems like a very important issue with the target.

@rifxii rifxii marked this pull request as draft July 29, 2025 01:09
@rifxii
Copy link
Contributor Author

rifxii commented Jul 29, 2025

After a bit more testing I've discovered that the odd behavior with Hashlink is able to be reproduced on the other platforms that I've tested. I created an issue on the Haxe repo regarding this: HaxeFoundation/haxe#12331.

I'll close the PR for now since this seems to be a non-flixel issue.

@rifxii rifxii closed this Jul 29, 2025
@Geokureli Geokureli reopened this Jul 29, 2025
@Geokureli
Copy link
Member

Geokureli commented Jul 29, 2025

Reopening this issue, in case other people have this issue and don't know who to blame, or want to track when it's fixed.

Thanks for tracking this down and making a new issue, I'll monitor and close this when it's fixed

Edit: Oh wait, this is a PR, I'll post the link on the actual issue

@Geokureli
Copy link
Member

Geokureli commented Jul 29, 2025

Also, I've been extremely busy and forgot about this PR, if this prevents the error, I'm okay with merging this, since the haxe change won't fix haxeflixel's minimum supported haxe versions.

Do you still think this should be merged?

@Geokureli Geokureli reopened this Jul 29, 2025
@rifxii rifxii marked this pull request as ready for review July 29, 2025 16:43
@rifxii
Copy link
Contributor Author

rifxii commented Jul 29, 2025

I've been using this change for the past day while testing my project and I haven't had any problems happen so I'm not against merging it. Would this require another PR in the future to update the conditional to check the user's haxe version if the issue is fixed in a future haxe release?

@Geokureli
Copy link
Member

Geokureli commented Jul 29, 2025

I've been using this change for the past day while testing my project and I haven't had any problems happen so I'm not against merging it. Would this require another PR in the future to update the conditional to check the user's haxe version if the issue is fixed in a future haxe release?

We could make it #if (haxe5 || !hl) now, or we can wait until it's fixed to add it. If now, people using haxe5 won't be able to use rotated frames until it's fixed in haxe. IMO, using either haxe5 or rotated frames are both very uncommon... up to you

@rifxii
Copy link
Contributor Author

rifxii commented Jul 29, 2025

When typing that reply I completely forgot that this issue in particular only affects Hashlink, whoops. I think leaving the current conditional alone is fine since I'm not sure if anything would need to be fixed on Hashlink's side and if so, their updates seem to be on a yearly schedule so that wouldn't be anytime soon.

@Geokureli Geokureli added this to the 6.1.1 milestone Jul 30, 2025
@Geokureli Geokureli merged commit c8f35b6 into HaxeFlixel:dev Jul 30, 2025
10 checks passed
Redar13 added a commit to TwistTeam/flixel-twist that referenced this pull request Jul 31, 2025
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.

Rotated frames not rendering on Hashlink
3 participants