Gamua / Starling-Framework

The Cross Platform Game Engine
http://www.starling-framework.org
Other
2.82k stars 821 forks source link

The Inverted Mask Feature #970

Closed EhsanMarufi closed 7 years ago

EhsanMarufi commented 7 years ago

I've implemented the Inverted Mask feature into the Starling API and put together a live demo of the feature here.

As the name suggests, the inverted mask marks the pixels where the object shouldn't be drawn, contrary to the normal mask where it marks the desired portion to be drawn.

The implementation supports any arbitrary combination of nesting or stacking of objects having normal or inverted masks. The only pushed restriction compared to the current masking routine is the maximum nesting count; through which, the stencil masking works. Before introducing the Invertible Masks, the maximum count was 255 levels of nesting on the Starling framework; and now, it's 127, half of which is dedicated to the inverted masks. A new property of isMaskInverted is added to the DisplayObject for the feature. The mask drawing methods in the Painter class are completely rewritten to support the feature. An almost non-destructive argument of isMaskInverted is also added to the method signatures (it's almost non-destructive to the public, as the methods are almost never invoked independently!) The support also causes the DisplayObjectContainer.as and the RenderTexture.as files to be slightly modified. The initial uniform stencil buffer value (when the buffer gets cleared) is pushed up to the value of 127; the change is reflected in the both of the Paint.as and the RenderUtil.as files.

The added code in the class of Painter could've been more concise and maybe even more "pretty!" using the ternary operator and some in-place conditional statements; but, since this is a drawing method, it needs to be as optimized as posible. Thus, the new code is organized into a single if-else statement instead of checking the state of the isMaskInverted parameter repeatedly.

b005t3r commented 7 years ago

Just came here to say: brilliant!

EhsanMarufi commented 7 years ago

Just came here to say: brilliant!

@b005t3r Thanks, it's very kind of you :)

PrimaryFeather commented 7 years ago

Wow! :astonished:

I can only second @b005t3r's comment: this is brilliant! The change is extremely clean and well integrated into the rest of the code. (Some tab-issues aside, but don't worry about that, I can fix that when merging.)

The demo alone is amazing. I've never seen a new feature advertised as well as that! Frankly, that alone deserves a "merge". :wink:

(I hope you are okay if I add that demo to a future blog post? With your credentials included, of course.)

I admit I need a little time to think through exactly how you are achieving this — so far, I understand that you are making use of the depth buffer in addition to the stencil buffer, but I need to study the rest of the code in a little more detail. :bowtie:

One small thing that's still missing (I think!) is that hit-testing should probably be inverted as well. But that shouldn't bee too difficult.

In any case: thanks a lot for all your efforts! I'll make it a priority to merge this into the master branch.

b005t3r commented 7 years ago

I admit I need a little time to think through exactly how you are achieving this

Yeah, @EhsanMarufi, would you mind explaining how does it work internally?

EhsanMarufi commented 7 years ago

I can only second @b005t3r's comment: this is brilliant!

@PrimaryFeather Thank your :) It's so nice of you, it's just an extension of your awesome framework at the first place :)

Some tab-issues aside..

I actually inserted the code via the github web page, the inconstancy arose there!

you are okay if I add that demo to a future blog post? With your credentials included, of course..

It's an honor :)

..hit-testing should probably be inverted as well. But that shouldn't bee too difficult..

I didn't think of that..

would you mind explaining how does it work internally?

@b005t3r No, of course not :)

TL;DR! (with a simple sample :)


Some explanations:

Actually, when I done typing the details, I came up with a huuuge! wall of text, most of which not really necessary!! So, I just go through the routine to convey the idea, to avoid building a bigger "wall of text," than what we have now!! The preparations (like introducing the new properties and their respective fields, or the new arguments, ...) are so obvious so I just skip them :) The considerable part, is the mask drawing methods in the class of Painter; i.e: the drawMask() and the eraseMask() methods. Firstly, the depth test is set to always fail, Context3DCompareMode.NEVER, prohibiting the mask shape to get drawn into the destination color buffer; it's rational, the concept of the mask is to "only" determine where the maskee should be drawn. The challenge is to utilize a single stencil buffer for the masking operations of the whole display list hierarchy. The normal mask drawing routine is the same as before, in short: drawing the mask into the stencil buffer and incrementing the respective entry value in the buffer. The incrementation effectively allows us to restore the stencil buffer to the previous state by decrementing the values. A normal mask gets written to the stencil buffer by incrementing the entry value in the buffer and when we want to "erase" that mask, we simply decrement the respective values in the stencil buffer. The opposite happens to the inverted masks; i.e: the stencil buffer values get decremented when an inverted mask is written, and the buffer values get incremented when "erasing" the inverted mask from the stencil buffer. The values in the stencil buffer are limited to 8-bit entries (possible values: 0..255). Since we want to be able to increament the values or decrement them, an initial value of 127 is chosen for the entire of the stencil buffer. The initial reference value is also set to 127. The pixels of the maskee get drawn to the destination color buffer, only when the corresponding entries in the stencil buffer are equal with the stencil reference value. This rule always applies, regardless of the mask mode being used. Now, it's simplified, we do need to control the stencil reference value. As another key factor, having the mask object as a "single" mesh (MeshBatch) is vital, which is already being take care of internally and flawlessly. Without that, the overlapping sections in the masks would repeatedly update the common regions, voiding the validity of the stencil reference value we have in control. The whole masking mechanism is the same as before. In the normal mask mode, the stencil buffer values and the reference value get incremented. When normal masks get nested, the final outcome is the same as in the common region in the a Venn diagram. When an inverted mask is encountered for drawing, the "whole shape" of the mask (Context3DCompareMode.ALWAYS) causes the corresponding stencil buffer values to be decremented, but the reference value remains intact. Having the stencil reference value (that is not changed) bigger than the buffer values (that are just decremented) would effectively cause the "undesirable" parts to be skipped when drawing the maskee. That's how it works :)

PrimaryFeather commented 7 years ago

Thanks a lot for the detailed description!

So that I understand this correctly: the depth buffer changes you are making are only to avoid the mask from actually being drawn, right? If so, that's not even necessary: in the renderMask method, the alpha value of the render state is set to zero, which already has this effect.

So what you're actually doing is decrementing the value instead of incrementing it for an inverse mask, and not change the stencil reference value. The funny thing is: I tried this myself, too, when I added the mask feature; however, I ended up with certain nesting scenarios where this didn't work. I couldn't reproduce any such issues with your sample, though, so it seems to be solid.

What I cannot remember doing in my experiments, though, is changing the default stencil buffer value to 127. So maybe my problem was actually not really the nesting (what I thought at the moment), but the fact that my stencil value reached zero. 😵

I'll play around with it a little more, but maybe that was the real issue back then.

PrimaryFeather commented 7 years ago

I'm still playing around with the code, and I've got a few ideas:

(1) First, we can actually leave the parameters of drawMask and eraseMask unchanged, since they can check for isMaskInverted themselves via maskee.isMaskInverted.

(2) In theory, we wouldn't even need the isMaskInverted property on DisplayObject. We could also do it just as it was done in classic Flash and treat the mask as being inverted when its blendMode is set to ERASE. It's by far not as obvious, so I'm not saying that this would be better — but we would spare DisplayObject from requiring even more memory (it's already quite a big object), for a feature that's needed only on a tiny, tiny (!) percentage of objects. If it's documented well on the mask property's API reference, people would find that, too.

Again, I'm really not sure about (2), I just wanted to bring it up. Any comments, anyone?

PrimaryFeather commented 7 years ago

BTW, @EhsanMarufi, no need to update your pull request with any of those changes. I've already got them on my local copy of your branch. :wink:

EhsanMarufi commented 7 years ago

Thanks a lot for the detailed description!

I do appreciate your efforts on making the Starling even better, you're awesome :)

..the depth buffer is to avoid the mask from actually being drawn..

Yes, actually setting the alpha value of the render state to zero looks wasteful! I mean, the the pixels are sent to have their alpha value of "zero" being computed producing exactly the same "effect" with the cost of some processor cycles, where you could've stopped the operations at the first place.

.. I ended up with certain nesting scenarios where this didn't work..

Also besides that, you might had the "single mesh" issue back then that I've described above :)

.. I'll play around with it a little more..

Please consider playing with various scenarios of nesting and stacking.

(1) First, we can actually leave the parameters of drawMask and eraseMask unchanged..

I understand, it seems better.

I'm really not sure about (2),

Me too!!

no need to update your pull request with any of those changes

You do know how to deal with these stuff! tnx :)

PrimaryFeather commented 7 years ago

Thanks a lot for your comments, Ehsan! 😄

Yes, actually setting the alpha value of the render state to zero looks wasteful! I mean, the the pixels are sent to have their alpha value of "zero" being computed producing exactly the same "effect" with the cost of some processor cycles, where you could've stopped the operations at the first place.

At least on the ActionScript side, it doesn't make much of a difference to set alpha to zero. That alpha value is a vertex constant that's sent to the vertex shader in any case. And the vertex- and pixel-programs are executed in both versions (i.e. whether you use the depth test to disable rendering or an alpha value of zero), which happens e.g. when you use the Texture Mask extension, so it seems that the render pipeline is not significantly shortened, either.

So, in the end, I think both versions are probably quite equivalent, performance wise.

Also besides that, you might had the "single mesh" issue back then that I've described above :)

Indeed, that might also have been it!

Please consider playing with various scenarios of nesting and stacking.

Will do!

I'm really not sure about (2),

Me too!!

After sleeping over it, I still think that your property is cleaner — but perhaps somebody else wants to comment on this, too.

b005t3r commented 7 years ago

After sleeping over it, I still think that your property is cleaner — but perhaps somebody else wants to comment on this, too.

Why not both? :) You can actually implement the property, that it internally sets blendMode to ERASE or AUTO.

EhsanMarufi commented 7 years ago

I think both versions are probably quite equivalent, performance wise.

I know the "Stage3D" is a bit more abstract than the native APIs like the "OpenGL ES", but I'm sure the rendering pipelines are somewhat similar. I think, when you prohibit the depth buffer from being updated, the pixel is guaranteed to be discarded instead of most probably being written to the color buffer (even if it's fully transparent), hopefully a tiny little of performance is gained through the prohibition and maybe some other operations like the "Blending operations" get skipped. As you know, there is not much documentation about the Stage3D to be precise.

PrimaryFeather commented 7 years ago

I know the "Stage3D" is a bit more abstract than the native APIs like the "OpenGL ES", but I'm sure the rendering pipelines are somewhat similar.

You are perfectly right. Since Stage3D builds upon OpenGL and DirectX (depending on the platform), the underlying pipeline must be the same. Thus, any saved step is a plus! I removed that "alpha" trick and let depth testing take care of it.

As for the isMaskInverted property: for now, I left that just as you implemented it, too. It's just easier to understand, and that should be the primary concern. Using both "BlendMode" and that property simultaneously would introduce a side effect (using a mask changes the blend mode) that might be confusing.

In other words, you made all the right choices!

All I added was correcting the "hitTest" (see the method with the same name) and I made sure that an inverted mask prevents the scissor-optimization from getting applied.

Thanks again for your help with this!! Great job, Ehsan. :smile: :+1:

EhsanMarufi commented 7 years ago

@PrimaryFeather I'm so glad that I helped this amazing framework to take another step forward and to be even better; I've uploaded the source for the demo project on my web host here, in case you may need it for some fine tuning. You made all this possible, Daniel, you're so awesome and thank you :) :1st_place_medal:

PrimaryFeather commented 7 years ago

Ah, thanks a lot for uploading the demo! That will come in very handy for the blog post for Starling 2.2! :smile:

PrimaryFeather commented 7 years ago

I just renamed the property isMaskInverted to a simple maskInverted. The advantage: auto-completion will group the two properties (mask and maskInverted) together, making it easier to find for developers.

EhsanMarufi commented 7 years ago

It's as great and subtle as ever, thank you Daniel :1st_place_medal: :)