Is there any way to combine the following into one loop?
Okay guys, I'm not a programmer. So I don't know about common idioms. I learned (almost) everything by myself without tutorials. So I have a general advice for everyone: don't learn like the way I did!
Quote:Original post by SneftelQuote:I prefer (and always do) the (i+1)%n stuff, as Antheus suggested, that trick is hard to read, and has 2 variables.Not only is modulus slow, it's only applicable to random-access containers. I'm surprised you find it hard to read -- prev=cur; increment cur is a pretty common idiom -- but I guess I've seen this idiom enough that I couldn't really give a useful opinion on how easy it is to understand the first time. The 2 variables thing is a complete red herring, though. If you're using (i+1)%n all over your loop body, (i+1)%n is a variable, whether you call it that or not.
If you didn't use single-letter variable names in your example and used "cur" and "prev" like you did when explaining it aloud, it probably wouldn't be quite as obtuse.
Hmm? But "prev" and "cur" isn't what they are. That was just a reference to an idiom that has a similar structure to the code I posted.
Author
Huge thank you for all your help guys.
I went for readibility over clever programming after toying with the ideas you posted:
Let me know what you think.
Thanks again.
I went for readibility over clever programming after toying with the ideas you posted:
/// <summary> /// Create a 2D polygon /// </summary> /// <param name="CCW">Are vertices ordered counter clockwise?</param> /// <param name="vertex">n vertices that make up polygon</param> public CD_Polygon(bool CCW, params Vector2[] points) { // A polygon must have at least 3 vertices if (points.Length < 3) { return; } vertices = points; faces = vertices.Length; edge = new Vector2[faces]; normal = new Vector2[faces]; // Drawing parameters vertices_Drawable = new VertexPositionColor[faces]; indices = new short[faces + 1]; for (int i = 0; i < faces; ++i) { int i2 = i + 1 < faces ? i + 1 : 0; edge = Vertices[i2] - Vertices; // Ensure the edges have non-zero length //Debug.Assert(edge.LengthSquared() > Constants.EPSILON * Constants.EPSILON); // Compute normals // Assume +ve Y = "Up" Vector2 n = new Vector2(); if (CCW) { n.X = edge.Y; n.Y = -edge.X; } else { n.X = -edge.Y; n.Y = edge.X; } // Normalise and set normal for current edge n.Normalize(); normal = n; // Calculate polygon centroid centroid += vertices; // Set vertices/indices for drawing the polygon Vector3 position = new Vector3(); position.X = vertices.X; position.Y = vertices.Y; position.Z = 0f; vertices_Drawable = new VertexPositionColor(position, Color.Red); indices = (short)i; } centroid /= faces; type = Collidable2D_Type.POLYGON; }Let me know what you think.
Thanks again.
Why do you reserve one extra element in 'indices'? It doesn't look to me like it gets filled in. Also, don't you have constructors taking arguments for Vector2 and Vector3?
// Some random suggestionspublic CD_Polygon(bool CCW, params Vector2[] points) { // Bad input shouldn't pass silently if (points.Length < 3) { throw new ArgumentException("Must supply at least 3 points"); } vertex_count = points.Length; // 'faces' makes it sound like data for each // face rather than a count of them. And 2D figures don't really have faces anyway. // Using plurals for collection names is a good idea; be consistent. edges = new Vector2[vertex_count]; normals = new Vector2[vertex_count]; // Storing both the original vertices and the drawing information is // redundant; the former can be found from the latter, and if you're actually // doing 3D work, you're going to want the Z components anyway, even if // they're all zero. drawableVertices = new VertexPositionColor[vertex_count]; indices = new short[vertex_count]; // This is clearer than adding to 'centroid', and we didn't zero that out anyway... Vector2 vertex_sum = new Vector2(); for (int i = 0; i < vertex_count; ++i) { Vector2 v1 = points; Vector2 v2 = points; <br> Vector2 v = v2 - v1;<br> edges<span style="font-weight:bold;"> = v;<br><br> <span class="cpp-keyword">int</span> scale = CCW ? -<span class="cpp-number">1</span> : <span class="cpp-number">1</span>;<br> <span class="cpp-comment">// 'normalized' - note descriptive rather than imperative - would return</span><br> <span class="cpp-comment">// a new value.</span><br> normals<span style="font-weight:bold;"> = <span class="cpp-keyword">new</span> Vector2(v.X * scale, v.Y * -scale).normalized();<br><br> vertex_sum += v1;<br><br> drawableVertices<span style="font-weight:bold;"> = <span class="cpp-keyword">new</span> VertexPositionColor(<span class="cpp-keyword">new</span> Vector3(v1.X, v1.Y, 0f), Color.Red);<br> indices<span style="font-weight:bold;"> = (<span class="cpp-keyword">short</span>)i;<br> }<br> centroid = vertex_sum / faces;<br> type = Collidable2D_Type.POLYGON;<br>}<br><br></pre></div><!–ENDSCRIPT–> Quote:Original post by Antheus
modulus tends to be slow.
Really? I had assumed there was a machine instruction for it or something. >_>
Do you have any further information? I might see if I can speed up a few toys of mine to corroborate this.
On-topic, I don't mind the loop Sneftel posted, but I probably would've refactored the loop contents into a subroutine and left the final iteration outside of the loop.
Also, I'd need empirical proof of things like cache misses before refactoring an algorithm around them, largely because the majority of performance anecdotes of old just don't apply these days.
Quote:Original post by FenrisulvurThere is. It takes several clock cycles.Quote:Original post by Antheus
modulus tends to be slow.
Really? I had assumed there was a machine instruction for it or something.
Quote:Also, I'd need empirical proof of things like cache misses before refactoring an algorithm around them, largely because the majority of performance anecdotes of old just don't apply these days.While memory systems have changed, calling cache line misses "anecdotes of old" is backwards. It's only recently that cache misses have become such a huge problem, because processing speed now far outstrips memory latency. If you're curious as to how long your program spends waiting for cache lines to come in, though, check out VTune or CodeAnalyst.
Quote:Original post by Sneftel
It's only recently that cache misses have become such a huge problem, because processing speed now far outstrips memory latency.
Herb Sutter did a great talk on this topic.
Author
Quote:Original post by Zahlman
Why do you reserve one extra element in 'indices'? It doesn't look to me like it gets filled in.
This because if I am drawing the polygon as a triangle strip then I need a final index of 0 to draw the polygon correctly:
public override void Draw() { // Get graphics device and effect GraphicsDevice graphics_Device = Game1.Instance.graphics_Device; Effect effect_Current = Game1.Instance.effect_Manager.effect_Debug; effect_Current.CurrentTechnique = effect_Current.Techniques["Default"]; effect_Current.Parameters["World"].SetValue(World_Transform); effect_Current.Begin(); effect_Current.CurrentTechnique.Passes[0].Begin(); graphics_Device.VertexDeclaration = Game1.Instance.effect_Manager.vd_VertexPositionColor; graphics_Device.DrawUserIndexedPrimitives<VertexPositionColor>(PrimitiveType.LineStrip, vertices_Drawable, 0, vertices_Drawable.Length, indices, 0, vertices_Drawable.Length); effect_Current.CurrentTechnique.Passes[0].End(); effect_Current.End(); }Quote:
Also, don't you have constructors taking arguments for Vector2 and Vector3?
Vector2 n = new Vector2(); n.X = edges.Y * scale; n.Y = edges.X * -scale;is faster than
Vector2 n = new Vector2(edges.Y * scale, edges.X * -scale);Quote:
// Storing both the original vertices and the drawing information is
// redundant; the former can be found from the latter, and if you're actually
// doing 3D work, you're going to want the Z components anyway, even if
// they're all zero.
When dealing with collision tests involving two polygons, is it not quicker to obtain the stored Vector2 vertices than using the X and Y components of the Vector3 vertices?
Quote:Original post by SneftelAnother problem is that on most architectures modulo won't do anything remotely sensible with negative indices. So watch out for expressions like (i-1)%n or the use of i%n on signed variables to guard against buffer overruns.Quote:I prefer (and always do) the (i+1)%n stuff, as Antheus suggested, that trick is hard to read, and has 2 variables.Not only is modulus slow, it's only applicable to random-access containers.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
