SFML.Net icon indicating copy to clipboard operation
SFML.Net copied to clipboard

VertexArray indexer doesnt returns a reference

Open jcbritobr opened this issue 4 years ago • 6 comments

This indexer in VertexArray does'nt returns a reference to a vertex. Docs are wrong, or the indexer is wrong. A deepy copy leads to poor performance in painting.

//
        // Summary:
        //     Read-write access to vertices by their index. This function doesn't check index,
        //     it must be in range [0, VertexCount - 1]. The behaviour is undefined otherwise.
        //
        // Parameters:
        //   index:
        //     Index of the vertex to get
        //
        // Returns:
        //     Reference to the index-th vertex
        public Vertex this[uint index] { get; set; }

This code will not work because v is a Vertex copy.

        Vertex v = array[(uint)i];
        v.Color = new Color((byte)r.Next(0, 255), (byte)r.Next(0, 255), (byte)r.Next(0, 255));
        v.Position = new Vector2f(r.Next(0, 800), r.Next(0, 600));

jcbritobr avatar Oct 31 '20 13:10 jcbritobr

I'm gonna assume the docs are wrong on this detail as they look copied from the C++ documentation where this indexer does return a reference. I'd also imagine the VertexArray class was originally written before return-by-reference for value types was introduced into C#.

charleyah avatar Nov 01 '20 13:11 charleyah

I'm gonna assume the docs are wrong on this detail as they look copied from the C++ documentation where this indexer does return a reference. I'd also imagine the VertexArray class was originally written before return-by-reference for value types was introduced into C#.

Yep. My sugestion is to maintain indexer as it is, because ref can't work with this operator and create a

public ref Vertex GetVertex(int index) {
    ...
}

This will boost the vertex painting performance a lot.

jcbritobr avatar Nov 01 '20 13:11 jcbritobr

In this case, I don't believe the Vertex copy directly affects draw performance of a VertexArray as this indexer isn't used when drawing, but it obviously contributes to the time it takes to update individual vertexs. Any change for performance boosts is something I'd encorage benchmarking first.

Regarding your suggestion, I'd ask @DemoXinMC for input.

charleyah avatar Nov 01 '20 15:11 charleyah

Yes, I'm refer to the vertex copy time. It seems an implementation that can be tested easily.

jcbritobr avatar Nov 01 '20 15:11 jcbritobr

I discover this using the same code in c#, pascal and rust. Pascal and Rust bindings already read a reference from vertex array. Only c# doesn't. Pascal and Rust are able to update a 1 Mi vertex in this way(pascal = 24fps, rust=42fps c#=16fps). C# got stucked to update a vertex array of this size. If you need I have the three languages implementations for the same algorithms.

jcbritobr avatar Nov 01 '20 15:11 jcbritobr

Since we're doing pInvoke calls, we can't really provide a reference.

Anyone willing to update the documentation?

eXpl0it3r avatar Jun 15 '23 21:06 eXpl0it3r