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

avm2: Make PerspectiveProjection.focalLength more precise #19748

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cookie-s
Copy link
Contributor

@cookie-s cookie-s commented Mar 8, 2025

Small follow up to #19532

The previous focalLength getter had a certain level of difference. This change reduces the difference from Flash Player.

Also, I found Ruffle test.toml has the configuration section for number approximations, so move the logic from Test.as.

@cookie-s cookie-s force-pushed the avm2-pp-focallength-precise branch 3 times, most recently from ebd8291 to 5daa9d6 Compare March 8, 2025 04:10
cookie-s added 2 commits March 8, 2025 04:10
The previous calculation for focalLength was correct in semantics, but
the difference from FP was not tiny. The calculation involves
trigonometric functions in floating point number and this semantically-equivalent
refactoring makes the result of focalLength calculation closer to
that of FP.
Move approximation logic from ActionScript to Ruffle test framework
@@ -40,7 +40,7 @@ pub fn get_focal_length<'gc>(
let fov = this.get_slot(pp_slots::FOV).coerce_to_number(activation)?;

let width = get_width(activation, this);
let focal_length = (width / 2.0) / f64::tan(fov / 2.0 * DEG2RAD);
let focal_length = (width / 2.0) as f32 * f64::tan((PI - fov * DEG2RAD) / 2.0) as f32;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After many trials and observations, I found this code. It doesn't seem to be perfect yet (for edge values like 1e-10), though.

  • 1 / tan x = cot x = tan (pi/2 - x)
  • _ as f32 * f32::tan(_) is not good. This produces better result.

Comment on lines +6 to +7
'fieldOfView = ([0-9.]+)',
'FOV [0-9]+ ([0-9.]+)',
Copy link
Contributor Author

@cookie-s cookie-s Mar 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

field of view still has some errors.

var pp: PerspectiveProjection = new PerspectiveProjection();
pp.fieldOfView = 179.99999;
trace(pp.fieldOfView);

I found the code above gives 179.99999000000003, so even pp.fieldOfView is not a simple storage variable.

@cookie-s cookie-s marked this pull request as ready for review March 8, 2025 04:34
@adrian17 adrian17 self-requested a review March 8, 2025 17:28
@Lord-McSweeney Lord-McSweeney added A-avm2 Area: AVM2 (ActionScript 3) T-compat Type: Compatibility with Flash Player labels Mar 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-avm2 Area: AVM2 (ActionScript 3) T-compat Type: Compatibility with Flash Player
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants