avm1: Implement MovieClip.setMask - #17 #263

[davidar]

This fixes some (but not all) of the rendering bugs in #1098

@Dinnerbone offered to make some tests as I have no idea where to even start with that

[Toad06]

👍

This also makes Bill The Demon playable (just remains an issue with yellow background on some objects, but this may be unrelated to this PR?).

[Herschel]

Thank you and welcome!

Some issues from me eyeballing it:

• A masker clip should no longer be visible. (i.e. after mc1.setMask(masker), masker should no longer render normally).
• A masker clip can be anywhere in the display hierarchy, but needs to render at the correct position. For example, mc1.setMask(_root.mc2.mc3.masker); works. I expect there should be something like this to render the mask at the proper place:

  context.transform_stack.push(Transform {
         matrix: masker.global_to_local_matrix() * self.local_to_global_matrix();
         ..Default::default(),
     });
  m.render(context);
  context.transform_stack.pop();
  

• A clip may only setMask one other clip, i.e. after mc1.setMask(masker); mc2.setMask(masker);only mc2 will be masked. So I suppose DisplayObject should also have a maskee field to be able to remove itself from its previous maskee.
• Will need to do some testing to understand how these interact with normal timeline masks; I believe if mc2 was used as a timeline mask, mc1.setMask(mc2) will disable the timeline masks (changes clip_depth to 0?)

(Take this with a grain of salt because I haven't tested this yet! 😄 )

[davidar]

I think those first three issues should be fixed now, but I'll need someone else to do the testing for the last one

[Dinnerbone]

So I did some basic testing and the lesson for today is that you may only have one script-set mask, and it's AND'ed onto any authored mask.

Here's a test swf: snip

Description	Flash	Ruffle
Authored (left) mask only
setMask(right)
setMask(bottom)
setMask(null)

[davidar]

Hm, that test.swf doesn't seem to be working for me (in either flash or ruffle), ruffle gives this error:

[2020-11-05T22:40:50Z ERROR ruffle_core::avm1] Prototype recursion limit has been exceeded
[2020-11-05T22:40:50Z ERROR ruffle_core::avm1] No more actions will be executed in this movie.

I think the problem is that I forgot to remove the maskee from the old masker when a new one is set, should be an easy fix.

[Dinnerbone]

I think my file uploader got confused and linked to an old test for something else. Here's a zip: avm1_set_mask.zip

I'll work on getting this test inside our visual test suite when this PR is ready to land!

[davidar]

Thanks, fixed!

[Toad06]

I'm getting a crash in this old Mario game (web and desktop):

panicked at 'already borrowed: BorrowMutError', /rustc/04488afe34512aa4c33566eb16d8c912a3ae04f9/src/libcore/cell.rs:867:31

I don't know why it panicks but I can tell that without the latest commit (1c8ca7c), the game works as intended.

[davidar]

I'm not sure why it was being triggered by that game in particular, but removing the redundant write to the maskee field of the new mask seems to have fixed it.

[Toad06]

👍

[Herschel]

Thanks for your work on this!

• So that the GC doesn't explode, be sure to trace masker and maskee in impl Collect: self.masker.trace(cc);
• In AS3 land, you can set a mask on any DisplayObject (https://help.adobe.com/en_US/FlashPlatform/reference/actionscript/3/flash/display/DisplayObject.html#mask), so let's move masker, maskee, set_mask to DisplayObject in anticipation of that.
  • For now, it's okay that only MovieClip::render renders using masker, since that's the only case in AS2. Eventually we'll want to reorganize things so there's a base render on DisplayObject that takes care of this consistently for all display objects (this might link up with the DisplayObjectContainer PR).

[Herschel]

Fixed these two issues:

• clip1.setMask(clip2): If clip1 was a timeline masker, these masks should be disabled (clip.set_clip_depth(0);).
  • Sample: setmask-masker.zip
• The mask was not rendered at the correct position in the case of nested clips, as described above: clip1.clip2.clip3.setMask(_root.clip4.clip5); needs to render clip5 with the proper position.
  • Sample: setmask-nested-clips.zip

Could use some testing on these fixes.

This is still an issue but will take a little more work:

• Non-movieclips should be allowed as maskers: clip.setMask(button);
  • Sample: setmask-button.zip
  • Need to split render into something like render and render_self
  • render does the common stuff; visibility checks, push transform, dynamic mask
  • render_self does the specific rendering for that subclass

[davidar]

Fixed:

The only thing I'm not sure about is whether the fact that EditText pushed a clone of the transform is a detail that should be retained?

Also when you say visibility checks, do you mean the culling? (I avoided moving that as not all of the classes do that, not sure if that's deliberate)

[Herschel]

Holding off on this until #1347 is fixed, because it will probably change how masks are rendered a bit (i.e. the context.allow_mask = true/false lines will probably change).

Re: the visibility checks, I think this should include both culling and checking display_object.visible(), and probably all objects should do it; but lets keep it simple for this PR and not move it, and keep this PR only focused on masks.

[Herschel]

Sorry for the delay on this one; I'll get back to this, clean up the masking code, and merge this in.

[Herschel]

Ok, rebased this into the latest changes. I still have to fix a few things with the masking in general, but I've been slow and I'd rather get this in now so it's not just sitting in the PR queue. Thanks for the patience and work, @davidar !