🤖 avm1: Implement MovieClip.hitArea (part of #21569) by FIM43-Redeye · Pull Request #24227 · ruffle-rs/ruffle

FIM43-Redeye · 2026-07-16T06:43:27Z

Description

Implements the AVM1 half of #21569: MovieClip.hitArea was completely
unimplemented, so assignments had no effect on mouse picking, and the
long-standing from_shumway/avm1/hitarea test was known_failure
because of it.

Flash Player resolves hitArea as an ordinary script property read at
pick time: whenever a button-mode clip is picked, the property is
re-fetched (running any user getter), and a value that currently
resolves to a display object has its shape tested in place of the
clip's own shape, at its own stage position. The engine stores no
hit-area state of its own — an assignment just writes the ordinary
script property, and a user setter can consume it entirely. Because a
getter runs during picking, it can call stopDrag() mid-drag (handled
with a reentrancy guard in update_drag) or remove clips mid-pick (in
self-removal captures, removal takes effect on the next pick). The
commit
messages and fixture headers describe the captured cases in full, all
established against the Flash Player 32 Linux debug projector.

(An earlier revision instead mirrored assignments out of the property
machinery into the display object; review feedback and further
captures showed that model is wrong. The branch was rewritten in
place, so the old approach remains visible in the outdated review
diffs.)

On performance: the old owner-bounds early-out is gone. Hit areas may
lie outside the owner's bounds, getters are side-effectful, and
captures show FP itself re-resolves the property on every pick, so the
result cannot be cached or bounds-gated.

No SWF5 fixture is included because these script-only fixtures rely on
the drawing API introduced in SWF6. The core diff is +78/-5:
movie_clip.rs (the helper and pick-time logic), player.rs (the
reentrancy guard), and one interned string; the rest is test fixtures.

Testing

Four SWF regression tests under tests/tests/swfs/avm1/hitarea_*,
down from sixteen in the previous revision per review feedback:
hitarea_sweep (eleven former single-shot fixtures consolidated into
one SWF and one choreography, plus transform-in-getter coverage added
during review), hitarea_lazy_getter,
hitarea_remove_sibling, and hitarea_remove_owner_drag. Every
fixture's expected output is real FP32 debug-projector output,
captured by scripting the fixture's input.json choreography with
xdotool in a virtual X server. Each fixture's header documents its
mouse path in stage coordinates (so input.json can be reproduced by
hand) and its reason for being a separate fixture.

from_shumway/avm1/hitarea now passes and its known_failure flag is
removed. Against master, all four fixtures fail.

cargo test --all passes (regression suite: 4162 passed, 0 failed,
332 ignored, at this branch rebased onto current master)
cargo fmt --all -- --check clean; cargo clippy --all --tests
zero warnings

Checklist

I, a human, have self-reviewed this PR and fully understand the changes within.
I have made or updated tests where possible.
All of my commits are properly scoped, compile successfully, and pass all tests.
This PR does not make sense to split up into smaller PRs.
An LLM was involved in the authoring of this code.

Copilot

Pull request overview

Implements AVM1 MovieClip.hitArea side effects so that assigning a MovieClip redirects mouse picking to the assigned clip’s shape (while reads still echo the originally assigned value), and adds a comprehensive regression-test suite that matches Flash Player behavior.

Changes:

Mirror processed AVM1 hitArea assignments/deletions from script property machinery into MovieClip display-object state, respecting case-sensitivity and read-only rejection rules.
Update AVM1 mouse picking so button-mode shape tests use the assigned hit area (including special cases like invisible hit areas and mask-in-use behavior).
Add 13 new Rascal-compiled AVM1 regression SWFs covering verified behavioral clauses, and unmark the legacy Shumway-derived test as known-failing.

Reviewed changes

Copilot reviewed 58 out of 71 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
core/src/display_object/movie_clip.rs	Redirect AVM1 button-mode picking to an assigned hit area and handle degenerate-owner-transform ordering.
core/src/avm1/object/stage_object.rs	Add helpers to mirror AVM1 `hitArea` assignments and clear redirects on delete.
core/src/avm1/object/script_object.rs	Hook hitArea mirroring into processed data-property assignments and deletions.
core/src/avm1/object.rs	Ensure inherited virtual-setter assignments also update mirrored hitArea side effects.
tests/tests/swfs/from_shumway/avm1/hitarea/test.toml	Remove `known_failure` now that behavior is implemented.
tests/tests/swfs/from_shumway/avm1/hitarea/output.ruffle.txt	Remove Ruffle-specific expected output override (test now matches baseline output).
tests/tests/swfs/avm1/hitarea_zero_scale/test.toml	Add regression fixture for hitArea with zero-scale owner transform.
tests/tests/swfs/avm1/hitarea_zero_scale/test.as	Exercise hitArea picking when owner transform is degenerate.
tests/tests/swfs/avm1/hitarea_zero_scale/output.txt	Expected trace output for zero-scale owner test.
tests/tests/swfs/avm1/hitarea_zero_scale/input.json	Mouse-move script to trigger rollover via hit area.
tests/tests/swfs/avm1/hitarea_setter/test.toml	Add regression fixture for instance-level `addProperty` setter interaction.
tests/tests/swfs/avm1/hitarea_setter/test.as	Verify redirect updates even when assignment is consumed by an instance setter.
tests/tests/swfs/avm1/hitarea_setter/output.txt	Expected trace output for instance-setter case.
tests/tests/swfs/avm1/hitarea_setter/input.json	Mouse-move script to trigger rollover via hit area.
tests/tests/swfs/avm1/hitarea_removed/test.toml	Add regression fixture for hitArea clip removed from stage.
tests/tests/swfs/avm1/hitarea_removed/test.as	Verify redirect stops when hit-area clip is removed.
tests/tests/swfs/avm1/hitarea_removed/output.txt	Expected trace output for removed-hit-area behavior.
tests/tests/swfs/avm1/hitarea_removed/input.json	Mouse-move script to trigger rollover before/after removal.
tests/tests/swfs/avm1/hitarea_readonly/test.toml	Add regression fixture for read-only rejection via ASSetPropFlags.
tests/tests/swfs/avm1/hitarea_readonly/test.as	Verify rejected assignment does not change redirect.
tests/tests/swfs/avm1/hitarea_readonly/output.txt	Expected trace output for read-only rejection.
tests/tests/swfs/avm1/hitarea_readonly/input.json	Mouse-move script to distinguish which region triggers rollover.
tests/tests/swfs/avm1/hitarea_proto_setter/test.toml	Add regression fixture for prototype-chain `addProperty` setter interaction.
tests/tests/swfs/avm1/hitarea_proto_setter/test.as	Verify redirect updates when assignment is consumed by a prototype setter.
tests/tests/swfs/avm1/hitarea_proto_setter/output.txt	Expected trace output for prototype-setter case.
tests/tests/swfs/avm1/hitarea_proto_setter/input.json	Mouse-move script to trigger rollover via hit area.
tests/tests/swfs/avm1/hitarea_proto_readonly/test.toml	Add regression fixture for inherited getter-only property rejection.
tests/tests/swfs/avm1/hitarea_proto_readonly/test.as	Verify inherited getter-only property rejects assignment and preserves picking.
tests/tests/swfs/avm1/hitarea_proto_readonly/output.txt	Expected trace output for inherited-readonly case.
tests/tests/swfs/avm1/hitarea_proto_readonly/input.json	Mouse-move script to confirm rollover stays on original clip.
tests/tests/swfs/avm1/hitarea_overwrite/test.toml	Add regression fixture for overwriting hitArea with non-MovieClip value.
tests/tests/swfs/avm1/hitarea_overwrite/test.as	Verify overwriting with number clears redirect while reads echo new value.
tests/tests/swfs/avm1/hitarea_overwrite/output.txt	Expected trace output for overwrite-clears-redirect behavior.
tests/tests/swfs/avm1/hitarea_overwrite/input.json	Mouse-move script to test post-overwrite rollover region.
tests/tests/swfs/avm1/hitarea_mask/test.toml	Add regression fixture for hitArea clip that is used as a mask.
tests/tests/swfs/avm1/hitarea_mask/test.as	Verify hitArea does not register hits when the hit-area clip is masking another clip.
tests/tests/swfs/avm1/hitarea_mask/output.txt	Expected trace output for mask-in-use behavior.
tests/tests/swfs/avm1/hitarea_mask/input.json	Mouse-move script attempting rollover over the hit area.
tests/tests/swfs/avm1/hitarea_invisible/test.toml	Add regression fixture for invisible hit-area idiom.
tests/tests/swfs/avm1/hitarea_invisible/test.as	Verify invisible hit area still registers hits.
tests/tests/swfs/avm1/hitarea_invisible/output.txt	Expected trace output for invisible-hit-area behavior.
tests/tests/swfs/avm1/hitarea_invisible/input.json	Mouse-move script to trigger rollover over invisible hit area.
tests/tests/swfs/avm1/hitarea_duplicate/test.toml	Add regression fixture for `duplicateMovieClip` copy semantics.
tests/tests/swfs/avm1/hitarea_duplicate/test.as	Verify duplicate does not copy hitArea property or redirect.
tests/tests/swfs/avm1/hitarea_duplicate/output.txt	Expected trace output for duplicateMovieClip behavior.
tests/tests/swfs/avm1/hitarea_duplicate/input.json	Mouse-move script to compare original vs duplicate rollover regions.
tests/tests/swfs/avm1/hitarea_delete/test.toml	Add regression fixture for `delete mc.hitArea`.
tests/tests/swfs/avm1/hitarea_delete/test.as	Verify deleting hitArea clears redirect and readback becomes undefined.
tests/tests/swfs/avm1/hitarea_delete/output.txt	Expected trace output for delete-clears-redirect behavior.
tests/tests/swfs/avm1/hitarea_delete/input.json	Mouse-move script to test rollover before/after deletion.
tests/tests/swfs/avm1/hitarea_chain/test.toml	Add regression fixture ensuring redirects don’t chase hitArea chains.
tests/tests/swfs/avm1/hitarea_chain/test.as	Verify only the assigned clip’s own shape is used, not its hitArea’s hitArea.
tests/tests/swfs/avm1/hitarea_chain/output.txt	Expected trace output for non-chaining behavior.
tests/tests/swfs/avm1/hitarea_chain/input.json	Mouse-move script to distinguish c vs b vs btn regions.
tests/tests/swfs/avm1/hitarea_case/test.toml	Add regression fixture for SWF6 case-insensitive assignment behavior.
tests/tests/swfs/avm1/hitarea_case/test.as	Verify `HitArea` assignment affects `hitArea` in a case-insensitive SWF.
tests/tests/swfs/avm1/hitarea_case/output.txt	Expected trace output for SWF6 case-insensitivity.
tests/tests/swfs/avm1/hitarea_case/input.json	Mouse-move script to trigger rollover via hit area.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kjarosh · 2026-07-16T23:30:55Z

Since you mentioned that you've used LLM to generate some code, and I can see that you're using it extensively, please describe how you used it exactly and how you made sure that the LLM didn't produce low quality code or code that is too specific to the API you're implementing. What is your contribution here except generating the code by an LLM?

I personally reviewed the behavior of the fix on each of the tests (mouse automation is broken under Wayland so I hand-captured those). I also verified the code locally, checked the tests for vacuousness against master (12/13 fail), and reviewed the final diff before submission. Most importantly in my opinion, I categorized the behavior by hand with Flash Player captures to begin with.

On being too specific to the API, the property hooks follow the notify_property_change pattern for text-field binding mirroring in the same file, the picking change reuses the storage field AVM2 already added, and no general machinery changed behavior.

I used Claude to write the Rust and test scripts, as well as to help me understand the architecture. If there is a more specific bar for what level of LLM-assisted contributions are accepted in this repository, please let me know.

no general machinery changed behavior

Yes, it did. You're changing the generic code of properties for injecting behavior for a specific property, which is without precedent.

I'm worried that the code generated by Claude is just too specific for the use case, it looks like you potentially encountered some bugs, but the code was generated with this specific API in mind instead of focusing on how AVM1 works in general.

It is possible there are just bugs in ruffle that cause the difference in behavior, but the proper way is to address them separately instead of hacking around a specific property. (Assuming that was the case.)

Thanks for correcting me on the property machinery hook- still learning the architecture here. I did some more captures on FP and found a few things worth citing:

A getter's call counter climbs continuously during picking, even with the mouse stationary.

An assignment consumed by a setter (nothing stored) never redirects, there's no write-time snapshot to hook.

A getter returning a clip that was NEVER assigned DOES redirect.
So as far as I understand, hitArea is a normal lazily-read property. In view of this I changed the code to resolve at pick time with a helper shaped like get_avm1_boolean_property, so all the avm1/object*.rs files are unchanged now. Test suite is also reworked, cut some useless fixtures and added one that any write-time implementation should fail.

Found a bunch of interesting things along the way too. _droptarget ignores hitArea entirely, so I gated that. Using a TextField as hitArea kills the owner (no redirect or own-shape fallback) so I handled that too. A getter removing clips mid-pick breaks FP itself, so instead of emulating a broken state, removed clips are never pickable. There was also a crash by recursion in my implementation if a getter called stopDrag() mid-drag, leading to it reentering update_drag, which is why I put in that guard. Without the guard it recurses until the stack overflows. I patched update_drag because that function shouldn't reenter itself afaik.

Thanks for your time reviewing this. I lean on LLM assistance heavily, so I verified all the FP behavior above against real FP captures. Happy to change anything here.

Thanks, that looks a lot better now, and I like your approach with extensive tests!

I lean on LLM assistance heavily, so I verified all the FP behavior above against real FP captures.

That's good to know, although you have to remember that (at least in my experience) LLMs will do everything to make tests pass without thinking much whether the code makes sense. That's why it's important (not only in the case of LLMs, but in general) to have tests more complex than the solution, because otherwise instead of reverse engineering properly and finding patterns, you'd just hard code specific cases you've tested. It's also important to try to falsify any claims you had while reverse engineering.

BTW You're welcome to join our Discord server and chat with us, this way you could run your ideas / questions though us quickly or ask for help if you're stuck somewhere.

Went over all my claims to break as much as I could and pulled out four of them:

A TextField hitArea does NOT disable picking entirely. The original fixture was silent because a selectable textField above the owner stole the mouse by basic occlusion. No TextField special case, objects are just objects.

The getter MAY NOT BE polled continuously even with the mouse stationary, that was an estimation based on manual captures and failed under script-driven capture with a virtual frame buffer. Dropped the fixture, no claims about idle-polling anywhere. I can dig deeper on this if it's worth roping into this PR.

FP mouse handling SOMETIMES breaks after a mid-pick removal, but this isn't a universal behavior in my captures. Depends on sibling-removal vs self-removal.

Removed clips are returned if the clip is removing itself mid-pick, addressed in more detail elsewhere. Sibling-removal captures froze so I've got nothing there.

Thank you so much, I really appreciate the help! I understand that LLMs are very good at sliding out from under you, that's why I try to verify so hard. I'm still learning the ropes of rigorous engineering so this is really useful. Also realized I joined the Ruffle server ages ago to lurk and it disappeared under other servers, so I'll be sure to pop in when I've got ideas/questions!

kjarosh · 2026-07-18T08:21:01Z

+hit.lineTo(150, 150);
+hit.lineTo(50, 150);
+hit.endFill();
+hit._visible = false;


You can also test what if hit is visible, but btn isn't.

Did so, doesn't pick, pinned as a part of the sweep test. Ruffle already had this one down pat.

kjarosh · 2026-07-18T08:33:15Z

+            ActivationIdentifier::root("[AVM1 hitArea]"),
+            self.avm1_root(),
+        );
+        let value = object.get(istr!("hitArea"), &mut activation).ok()?;


I'm not sure if you've already tested it (I don't think so), in order to make sure what to do with an error here, just throw from inside the property getter, e.g.

addProperty("hitArea", function() { throw "test"; });

(it's non-blocking though, as it's a pretty niche path, that's something that can be covered in the future)

Went ahead and tested, it's the btn_throw case in hitarea_sweep now. In FP, throwing doesn't disturb picking, the clip's own shape is tested like if there's no hitArea at all. The only visible effect is an uncaught-exception warning in the debug projector's flashlog, not even a dialog. Ruffle's existing .ok() fallback already did this correctly, now it has a test.

kjarosh · 2026-07-18T08:39:38Z

+                    // The getter may have removed this clip itself; likewise,
+                    // never pick a removed clip.
+                    if self.avm1_removed() {
+                        return None;


Is that something covered by tests?

It is now. Tested, determined the check was actually WRONG. Two captures in two layouts show that a getter removing its own owner still has the owner returned from the removing pick (_droptarget names it), and removal only takes effect on the next pick. The check is deleted, and if it ISN'T deleted hitarea_remove_owner_drag fails. We lose the dt: /btnRm line because the old check suppresses the removing pick's result. The entry check above it is NOT discriminated by any test because removal already removes the clip from its parent's child list, and player.rs filters event delivery. We disabled said entry check and everything passed anyway, but kept it as is nonetheless. This should only matter for clips removed earlier in the SAME pick's traversal snapshot (where FP froze in our attempts) so I don't have any usable capture to pin it against.

The coverage boundary is that _droptarget and event delivery are pinned while hover/cursor state and context-menu selection aren't covered. Noting this because both come from the same pick result, but the uncovered elements would need test framework expansion and that is VERY out of scope. I'm trying to keep my work here to adding single coverage elements one by one.

kjarosh · 2026-07-18T08:59:30Z

This is looking very good now thank you! Let's polish details and improve tests and we'll be ready to merge.

Flash Player resolves hitArea as an ordinary script property read at pick time: the property is re-fetched (running any user getter) on every button-mode pick, including each drag update's _droptarget pick, and a value that currently resolves to a display object is hit-tested in place of the owner's own shape, at its own stage position. Assignment stores nothing beyond the ordinary script property — there is no dedicated engine state, and a setter can consume a write entirely. This mirrors how other engine-consulted properties like `enabled` are read, and the lookup inherits SWF-version case sensitivity from the property system. Capture-verified against the FP32 debug projector: - The hit area applies only to button-mode picking; captured _droptarget values follow physical shapes and ignore the hit area, though the property is still resolved on those picks (a getter calling stopDrag() ends the drag, as in Flash, without crashing or hanging). - An invisible hit area still registers hits, but an invisible owner's hit area does not; a removed hit area falls back to the clip's own shape. A mask-serving MovieClip hit area never hits, while a mask-serving TextField hit area still does — each type keeps its own hit-test semantics. - Display-object values use their normal shape test; the fixtures cover MovieClip and TextField. Non-display-object values fall back to the owner's own shape. - A getter that throws is treated as no hit area (the owner's own shape picks). Running user code during picking has two consequences that need handling. A getter can call stopDrag() and re-enter Player::update_drag mid-pick; a reentrancy guard makes the nested call a no-op. A getter can also remove clips mid-pick. Captures show removal takes effect on the NEXT pick: a getter removing its own owner still has the owner returned from the removing pick (visible as a _droptarget value naming the removed clip), while later picks exclude it. Ruffle matches this. In captures of a mid-pick SIBLING removal, Flash Player froze _droptarget and stopped delivering release events; Ruffle emulates none of that and simply excludes the removed clip for the rest of the pick. Un-marks from_shumway/avm1/hitarea as a known failure, since this change makes it pass.

Four fixtures pinning capture-verified Flash Player behavior, with deterministic mouse choreography via input.json. - sweep: one consolidated SWF, one choreography along guide lines drawn on the stage, with every stop documented in the fixture header so a human can reproduce input.json by hand. Covers: a hit area's own hitArea being ignored; an assigned hit area replacing the owner's own shape; an invisible hit area still hitting while an invisible owner's does not; a mask-serving MovieClip hit area never hitting while a mask-serving TextField hit area still does; a zero-scaled owner still picking via its hit area; a TextField hit area picking like a MovieClip one when not occluded; a getter that throws meaning no hit area (the own shape picks); deleting the property, overwriting it with a number, or removing the hit-area clip returning picking to the own shape; a transform mutated inside the getter taking effect on the pick that ran the getter — a moved hit area and a moved owner are both tested at their new positions immediately, unlike removal, which takes effect on the next pick; _droptarget following physical shapes and ignoring hit areas during a drag; a getter calling stopDrag() mid-drag ending the drag without hanging the player; and a getter removing its own owner mid-pick, after which no rollover for it is delivered. Silent zones are pinned by the trace lines around them, and roll-out gap stops precede any same-owner zone that must fire again. The owner-removal finale is choreographed last and its readback fires on a frame delay instead of a click: captures showed Flash Player's mouse input can die after the mid-pick removal. - lazy_getter: resolution is a full lazy property get — a prototype addProperty getter supplies the hit area even though its return value was never assigned to the property, picking follows the getter's changing return value, and a setter that consumes assignments stores nothing. Separate because its prototype-level setter would consume every plain hitArea assignment in a shared SWF. - remove_sibling: no rollover is delivered for a clip removed by a sibling's getter mid-pick. Separate as a precaution: Flash Player froze after a mid-pick sibling removal in earlier captures, and a second removal case in the sweep's capture would compound that risk. - remove_owner_drag: _droptarget across a getter removing its own owner mid-pick — the removing pick still reports the removed clip, the next pick lands on the clip underneath, and a later hover reaches the underlying clip. Suppressing the removing pick's result loses the "dt: /btnRm" line, as an earlier revision of the implementation did. Separate for the same precaution as remove_sibling.

Lord-McSweeney · 2026-07-29T08:59:52Z

Seems like there's a difference in the hitarea_remove_sibling between Ruffle with this PR and Flash Player. In Ruffle, the blue box disappears when the mouse is pressed down (this line in input.json):

    { "type": "MouseDown", "pos": [0.0, 0.0], "btn": "Left" },

However, in FP, the blue box disappears when the mouse is moved back over it, at the end of the test (this line in input.json):

    { "type": "MouseMove", "pos": [100.0, 100.0] },

Do you know the reason for this discrepancy?

Copilot AI review requested due to automatic review settings July 16, 2026 06:43

Copilot started reviewing on behalf of FIM43-Redeye July 16, 2026 06:43 View session

Copilot AI reviewed Jul 16, 2026

View reviewed changes

kjarosh reviewed Jul 16, 2026

View reviewed changes

Comment thread core/src/avm1/object/script_object.rs Outdated

kjarosh reviewed Jul 16, 2026

View reviewed changes

FIM43-Redeye force-pushed the avm1-hitarea branch 2 times, most recently from 0b0e2b3 to a602b56 Compare July 18, 2026 00:22