Gamua / Starling-Framework

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

Two cases of isStateChange call that might not be properly checking numQuads hits max #735

Closed jamikado closed 9 years ago

jamikado commented 9 years ago

Looks like there might be two cases where a call to isStateChange is not properly checking the actual numQuads max reach limit when checking for a state change when adding a QuadBatch.

In RenderSupport, line 577, the call: if (mQuadBatches[mCurrentQuadBatchID].isStateChange( quadBatch.tinted, parentAlpha, quadBatch.texture, quadBatch.smoothing, mBlendMode)) needs to be: if (mQuadBatches[mCurrentQuadBatchID].isStateChange( quadBatch.tinted, parentAlpha, quadBatch.texture, quadBatch.smoothing, mBlendMode, quadBatch.numQuads))

also, in QuadBatch, line 519, the call: if (!batch1.isStateChange(batch2.tinted, 1.0, batch2.texture, batch2.smoothing, batch2.blendMode)) needs to be: if (!batch1.isStateChange(batch2.tinted, 1.0, batch2.texture, batch2.smoothing, batch2.blendMode, batch2.numQuads)) what I think could happen now is without this extra numQuads capacity check (incorrectly checking as a numQuads = 1 default), an addQuadBatch could happen, and the numQuads will be clipped at MAX_NUM_QUADS, not generating an error, but the extra quads in the secondary QuadBatch I think would not get copied in and lost in the render?

PrimaryFeather commented 9 years ago

Thanks a lot for making me aware of this, Jeff! Indeed, those parameters were missing. In fact, this would have caused an exception when the maximum batch size had been exceeded. (The "expand" method of "QuadBatch" would have raised an exception.)

jamikado commented 9 years ago

Ok cool thanks for confirming. Though it is a moot point now, I thought the exception wouldn't happen because you don't call expand() in addQuadBatch (versus addQuad) but just manipulate the capacity property directly and that just clips on MAX_NUM_QUADS instead

PrimaryFeather commented 9 years ago

Ah, you're right — I didn't try it out, actually. ;-)