scrollRect implementation

[jwt-sgg]

This is the first pass at implementing scrollRect. I have been using it for about a week and it appears to work well (as expected anyway), but I have not tested it with cacheAsBitmap.
Eventually we should make sure that when caching as bitmap is in use, applying a scrollRect should also provide a performance boost since it should only render child objects that are in the clip region (you may know if this is already the case, but I have not looked into it).

[bp74]

The code looks great and it's nice that the API is the same as in Flash. From this point of view i would immediately merge your pull request. But i have one concern. Someone would need the scrollRect feature for a very specific problem (mainly scrollable lists). The same thing could be achieved by using the currently available features. Granted that the scrollRect feature is nicer to use and easier to understand, but at the same time you add lots of code. Especially the additional code in RenderState.renderObject concerns me. This is by far the hottest method in the whole library! Only a few DisplayObjects will ever use the scrollRect feature but the code needs to check every DisplayObject.

Do you think that a simple implementation like the one below would achieve the same thing? It's a dedicated DisplayObject that provides the scrollRect feature, without changing any of the other code. The code is not really tested, just a quick prototype: https://gist.github.com/bp74/576d04003326f783a8b82b6457a77438

Btw. i think i will take a look at an optimization for stage-aligend rectangle masks. For such mask we don't need to use the stencil buffer, but use the scissors feature from WebGL. I haven't used it before but i think this would be faster!

[jwt-sgg]

It seems to me that we could easily optimize RenderState.renderObject to quickly branch for objects using a scrollRect rather than crippling/removing the feature. That would add only a single boolean test to the rendering of objects not using a scrollRect. I can update the pull request if that would work with you.

If we really can't use the current implementation, I could be OK with a single object type that implements scrollRect, but I would rather see it derive from DisplayObjectContainer than Sprite. It would make a scrollable 'group' object, without the overhead of the sprite graphics. BTW, I didn't run your example, but where in your ScrollableSprite does the clipping to the rect come from?

I think you're selling scrollRect short in the number of use cases though, and several use cases will have extra overhead by having to wrap in a clipping container. We use it for many scrollable objects: lists (as you suggested), text boxes, zoomed in views, drawing canvas for tools, and very importantly as an optimization in showing&rendering only the part of a complex object or large bitmap currently visible (we use this case frequently). I'm sure there are more uses that I'm not currently thinking about as well. Also, when exporting objects from Flash it will be easier to support the feature if it works the same way.

[jwt-sgg]

BTW, masks are similarly expensive in RenderState.renderObject, and we could optimize that overhead away as well by early branching in the same way. We use scrollRect much more frequently than we use masks, so if this is that hot of a function, I might just want to have that overhead removed either way.

[bp74]

Hi, you are right about the overhead of the currenct mask property of DisplayObject in RenderState.renderObject. The same argument i use for the ScrollContainer (yes, a container is better than a Sprite) would be valid for a MaskContainer. This would move the generic mask property of DisplayObject to a dedicated class.

The good thing is that dart2js removes the mask related code in RenderState.renderObject completely if the mask property is not used in the application. So if someone only has a few masks (maybe just one?) he would be better of using a MaskContainer instead of the generic DisplayObject.mask property. This way dart2js would optimize the RenderState.renderObject method.

The clipping in ScollContainer happens in it's render method. I uploaded a new gist ... https://gist.github.com/bp74/84047ef0de36c3d2ee54832c01b52b50

[jwt-sgg]

The more I think about it, I believe it would be best to leave the scrollRect available on any object, as well as the mask. If we create a specialized container for each feature that could add performance overhead, we will end up with a bunch of features that can't be used together (not easily anyway). It will also make creating a tool to manipulate art, like a level editor, much more difficult to use for both the developer and the creative users.

I would actually say your point about the tree shaking in dart2js is a good point in favor of simply optimizing the RenderObject with a quick branching for masks and scrollRects. The overhead for both features being used would be minimized to less than the current masking support, while any app not using the features wouldn't see any overhead in release code anyway.

[bp74]

No i would not remove the mask property, this was just because you mentioned it. For me the scrollRect feature is just a more specialized mask feature, therefore i don't like to add it.

[bp74]

Fun story, i just had a chat with @primaryfeather (Daniel, the creator of the Startling Framework) about the scrollRect property. He also thinks that it's not a good idea to add this property since there is already a mask property.

[PrimaryFeather]

I really don't want to interfere in this discussion, but maybe my input can help in some way.

I didn't implement the scrollRect property in Starling, either. Not because it's not useful — of course, scrolling lists and the samples you mentioned are perfectly valid use cases. But just like Bernhard, I think that the mask property already provides all the building blocks for this feature, and scrollRect is somehow a duplication of that logic, plus scrolling. As Bernhard's implementation shows, it's easy to add that functionality via a separate class.

DisplayObject is arguably the most important class of a display list framework, so it requires really exceptional care. It should be as lightweight as possible, and provide really just the minimum. Even that mask is part of it is a decision that was not taken lightly. You must also consider the side effects, i.e. how other objects that manipulate display objects will handle the new property. (Sprite3D? Filters?)

That StageXL is such a great engine is not only a result of the features it has — but also of those it has not, decidedly (which might sound counter-intuitive)! I'm probably famous for being overly conservative when it comes to core features of a framework, so I'm definitely not the best guide on this topic — but that's my opinion. :wink:

[jwt-sgg]

Bernhard, your point about tree shaking removing an unused feature is exactly why I can't understand why we would not just add the feature. That same argument can be said for Daniel's point about keeping a framework lightweight, because the code gets removed if it's not used. And yes, scrollRect is just a special case of masking, but the fact that it's a special case (always a rect) means it can be optimized much more than the general masking case. Obviously the first implementation didn't go there, but you've also pointed out we could use scissors for screen aligned rects, and eventually we could use clipPlanes for unaligned rects. Clearly scrollRect is a very special case for masking that could have minimal performance impact compared to general masking.

On a more macro level, it seems to me that the whole point of StageXL is to make it easy for Flash developers to port and/or continue using Flash as a content creation tool. So implementing features to match the Flash API as close as reasonably possible makes a lot of sense. If that's not the goal, I don't understand why the Flash API was the basis. The Flash API mimicking certainly is the reason we're using StageXL. I realize that's just our use case and my opinion, but it also means leaving the feature in will make it much more easy for my teams to port our current code and assets. That said, clearly you can choose to add scrollRect, or not, to StageXL. If not, I will simply maintain the feature in our fork to keep my devs as efficient as possible.

[devlfm]

Just my 2 cents,

Why not extend like InteractiveObject? a "ViewPortObject" that would be a core component with more features.. Then you can create a scrollbars that listen to the changes on the viewPort and vice-versa.

The ones who need it, use it.. otherwise just use the base DisplayObject.

[bp74]

@devlfm Yes exactly, i think this is the right thing to do. I like the name you suggested!

[bp74]

Please check out my commits: https://github.com/bp74/StageXL/commit/0501bd0facaaf6845da4b57661bfb9923173dd51 https://github.com/bp74/StageXL/commit/6d5aab6b4852fa75f3d108923f324288b3146b17

I've added a new DisplayObjectContainer called ViewportContainer and optimized stage-aligned rectangular masks in WebGL.