PrimaryFeather / Sparrow-Framework

The Open Source Game Engine for iOS
http://www.sparrow-framework.org
Other
538 stars 173 forks source link

Display object clipping #1070

Open tconkling opened 12 years ago

tconkling commented 12 years ago

Hi Daniel,

I've added clipRect support to SPDisplayObjectContainer. It's partly based on the work that Shilo did with his SHClippedSprite class (https://gist.github.com/1346513), but since it's properly integrated with Sparrow, it's more efficient. There's no API or performance impact for users that don't use clipRects.

For those that do, using a clipRect is as simple as setting the clipRect property on a SPDisplayObjectContainer. The performance impact for clipping should be negligible.

It's pretty basic - it just uses GL_SCISSOR_TEST, which means that it doesn't support non-axis-aligned clipRects, so clipped sprites that are rotated won't have their rotation applied to the clipping. (The docs make a note of this.)

PrimaryFeather commented 12 years ago

Hi Tim, wow, thanks! As always, a great implementation! Really very well done. I especially love the way you handled nested clipRects. This is a very welcome addition!

One suggestion, though: I think we could even take this one step further and add that property to "SPDisplayObject" instead of "SPDisplayObjectContainer". From skimming over your code, I don't see any reason why this shouldn't be possible! What do you think -- does this make sense?

tconkling commented 12 years ago

I started going down that path, but the boundsInSpace: method complicates things a bit, since you want it to return the intersection of an object's clipRect with its unclipped bounds. We could either require that all implementations of boundsInSpace do the intersection, or we could make a "boundsInSpaceInternal" method that subclasses implement, and have boundsInSpace call through to that and then do the intersection.

I don't have a strong opinion either way - I do think it would be handy to set a clipRect on display list leaf nodes, but I was hesitant to complicate the API. I'm happy to make changes, though, if you have a preference! (Since there aren't that many display object subclasses, I think the first option -- requiring everyone who implements boundsInSpace: to do the intersection -- makes a bit more sense, because it's just a few lines of additional code in a couple classes and doesn't change the API.)

PrimaryFeather commented 12 years ago

Hm ... no, you're right, that would make things needlessly complicated. Let's stick with the original implementation for now. We could still make that change later.

tconkling commented 12 years ago

Hey Daniel - just curious what your thoughts are on merging the displayObjectClipping and skewing pull requests into sparrow/master, now that it seems Sparrow is being actively worked on again.

PrimaryFeather commented 12 years ago

You're right, I should add those while I'm working at the other changes!

I even considered to add skewing in version 1.4, but then I remembered that I have changed it a little bit from your implementation, when I added it to Starling.

So I'll do it the same way in Sparrow, too. At the same time, I will make sure that rendering will use the transformation matrix right from the display object, not the "glRotate ..." etc. methods, for efficiency.

PrimaryFeather commented 12 years ago

(BTW, the reason I didn't add them to Sparrow 1.4 is that I wanted to concentrate on Sparrow 2.0; that latest release was just for maintenance, I wanted to make sure I don't break anything.)

tconkling commented 12 years ago

Great to hear, thanks!