DavidKimYongRak / box2d

Automatically exported from code.google.com/p/box2d
0 stars 0 forks source link

Merge edge & chain shape #324

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Chain & Edge shapes both exist even though Chain shape covers the use cases for 
Edge (albeit uses functions from Edge). I feel that this odd "extension" of 
Edge Shape is unnecessary, and that Chain Shape is the only one that really 
needs to exist. This could cut down on time to prepare contacts, etc as right 
now function calls are passed off to a fake edge that is temporarily created to 
handle Chain Shape contacts.

The merge would be pretty simple; remove Edge Shape, add a "SetAsEdge" function 
in ChainShape which makes it create a single edge (same as using SetAsChain 
with two vertices).

Original issue reported on code.google.com by Jonno.5000 on 27 Dec 2013 at 9:04

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I wrote up a patch which does what I said above. Patch notes:
- Any edge-related files are now gone. EP collider has moved to ChainPolygon 
collision files.
- b2ChainShape::GetChildEdge is now gone, instead opting for in-place 
pointers/references to vertices required for prev, v1, v2, and next
- b2Shape::Type adjusted, e_edge is gone
- Added b2ChainShape::CreateEdge which accepts the same arguments and has the 
same usage as b2EdgeShape::Set
- Added b2ChainShape::Clear, a utility function which will reset the chain 
shape back to its initial state.
- Removed assert from b2ChainShape::CreateLoop and b2ChainShape::CreateChain 
which ensured that the chain was empty. Instead, if the chain is not empty, it 
will call Clear to reset it prior to allowing re-use of the chain. This allows 
the chain to be re-set without requiring re-initialization.
- Adjusted all demos to use chain shapes instead of edges

Original comment by Jonno.5000 on 31 Dec 2013 at 12:59

Attachments:

GoogleCodeExporter commented 9 years ago
+1 for the b2ChainShape::Clear() method. I think that the Clear() method should 
be merged in (there's no other way of resetting the object)

Cheers

Original comment by Dimitris...@gmail.com on 28 Jan 2014 at 9:55

GoogleCodeExporter commented 9 years ago
The chain shape is an efficient way to store many edge shapes that are 
connected together. The 3D analogy is Mesh versus Triangle. Now in 3D you 
probably wouldn't let a fixture be a single triangle. So I don't want to 
eliminate b2EdgeShape. Instead I would probably just not allow a fixture to be 
made from one.

Original comment by erinca...@gmail.com on 4 Apr 2014 at 5:36

GoogleCodeExporter commented 9 years ago
I decided not to remove and change b2EdgeShape or b2ChainShape. This is mainly 
a stylistic request and doesn't affect usability in an important way.

I did add b2ChainShape::Clear().

Completed: At revision: 307  
Feature request 324. Implemented b2ChainShape::Clear.

Original comment by erinca...@gmail.com on 5 Apr 2014 at 8:45