Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display object clipping #1070

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

tconkling
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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.

  • you can now both skew and rotate at the same time
  • I moved the skewing to a different position in the order of matrix operations (see http://goo.gl/dFWif ), which makes it react in a more intuitive way when you use scaling and skewing simultaneously

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
Copy link
Owner

(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
Copy link
Contributor Author

Great to hear, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants