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.

Sign up for GitHub

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

Use Span for SFML.Net API #263

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Use Span for SFML.Net API #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions SFML.Module.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

<ItemGroup>
<PackageReference Include="CSFML" Version="[2.6.1, 2.7)" />
<PackageReference Include="System.Memory" Version="4.5.5" />
</ItemGroup>

</Project>
2 changes: 1 addition & 1 deletion examples/opengl/OpenGL.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ static void Main()
{
GL.GenTextures(1, out texture);
GL.BindTexture(TextureTarget.Texture2D, texture);
GL.TexImage2D(TextureTarget.Texture2D, 0, PixelInternalFormat.Rgba, (int)image.Size.X, (int)image.Size.Y, 0, PixelFormat.Rgba, PixelType.UnsignedByte, image.Pixels);
GL.TexImage2D(TextureTarget.Texture2D, 0, PixelInternalFormat.Rgba, (int)image.Size.X, (int)image.Size.Y, 0, PixelFormat.Rgba, PixelType.UnsignedByte, image.Pixels.ToArray());
GL.TexParameter(TextureTarget.Texture2D, TextureParameterName.TextureMagFilter, (int)TextureMagFilter.Linear);
GL.TexParameter(TextureTarget.Texture2D, TextureParameterName.TextureMinFilter, (int)TextureMinFilter.Linear);
}
Expand Down
15 changes: 7 additions & 8 deletions src/SFML.Audio/Music.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,17 @@ public Music(Stream stream) :
/// <param name="bytes">Byte array containing the file contents</param>
/// <exception cref="LoadingFailedException" />
////////////////////////////////////////////////////////////
public Music(byte[] bytes) :
public Music(Span<byte> bytes) :
base(IntPtr.Zero)
{
GCHandle pin = GCHandle.Alloc(bytes, GCHandleType.Pinned);
try
unsafe
{
CPointer = sfMusic_createFromMemory(pin.AddrOfPinnedObject(), (UIntPtr)bytes.Length);
}
finally
{
pin.Free();
fixed (byte* ptr = bytes)
{
CPointer = sfMusic_createFromMemory((IntPtr)ptr, (UIntPtr)bytes.Length);
}
}

if (CPointer == IntPtr.Zero)
{
throw new LoadingFailedException("music");
Expand Down
29 changes: 16 additions & 13 deletions src/SFML.Audio/SoundBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,17 @@ public SoundBuffer(Stream stream) :
/// <param name="bytes">Byte array containing the file contents</param>
/// <exception cref="LoadingFailedException" />
////////////////////////////////////////////////////////////
public SoundBuffer(byte[] bytes) :
public SoundBuffer(Span<byte> bytes) :
base(IntPtr.Zero)
{
GCHandle pin = GCHandle.Alloc(bytes, GCHandleType.Pinned);
try
{
CPointer = sfSoundBuffer_createFromMemory(pin.AddrOfPinnedObject(), (UIntPtr)bytes.Length);
}
finally
unsafe
{
pin.Free();
fixed (byte* ptr = bytes)
{
CPointer = sfSoundBuffer_createFromMemory((IntPtr)ptr, (UIntPtr)bytes.Length);
}
}

if (CPointer == IntPtr.Zero)
{
throw new LoadingFailedException("sound buffer");
Expand All @@ -96,7 +95,7 @@ public SoundBuffer(byte[] bytes) :
/// <param name="sampleRate">Sample rate</param>
/// <exception cref="LoadingFailedException" />
////////////////////////////////////////////////////////////
public SoundBuffer(short[] samples, uint channelCount, uint sampleRate) :
public SoundBuffer(Span<short> samples, uint channelCount, uint sampleRate) :
base(IntPtr.Zero)
{
unsafe
Expand Down Expand Up @@ -181,13 +180,17 @@ public Time Duration
/// (sf::Int16).
/// </summary>
////////////////////////////////////////////////////////////
public short[] Samples
public Span<short> Samples
{
get
{
short[] SamplesArray = new short[sfSoundBuffer_getSampleCount(CPointer)];
Marshal.Copy(sfSoundBuffer_getSamples(CPointer), SamplesArray, 0, SamplesArray.Length);
return SamplesArray;
ulong count = sfSoundBuffer_getSampleCount(CPointer);
IntPtr ptr = sfSoundBuffer_getSamples(CPointer);

unsafe
{
return new Span<short>((void*)ptr, (int)count);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we .NET doesn't own the memory for samples, should we be handling this this way? Our Span could be invalidated if CSFML moves things IIRC

Copy link
Contributor

@charleyah charleyah Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be safe for the lifetime of the SoundBuffer object instance for as long as all public members on SoundBuffer class don't produce iterator invalidation on the underlying std::vector containing the samples on sf::SoundBuffer (m_samples).

}
}
}

Expand Down
10 changes: 8 additions & 2 deletions src/SFML.Audio/SoundBufferRecorder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;

namespace SFML.Audio
Expand Down Expand Up @@ -57,9 +58,14 @@ protected override bool OnStart()
/// <param name="samples">Array of samples to process</param>
/// <returns>False to stop recording audio data, true to continue</returns>
////////////////////////////////////////////////////////////
protected override bool OnProcessSamples(short[] samples)
protected override bool OnProcessSamples(Span<short> samples)
{
mySamplesArray.AddRange(samples);
if (mySamplesArray.Capacity < mySamplesArray.Count + samples.Length)
mySamplesArray.Capacity = mySamplesArray.Count + samples.Length;

for (int i = 0; i < samples.Length; i++)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there not a way that we can use AddRange() with Span? Also, Audio is likely going to benefit from a significant rework, but that's beyond the scope of this draft.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no such overload for AddRange() if I remember correctly.

mySamplesArray.Add(samples[i]);

return true;
}

Expand Down
10 changes: 5 additions & 5 deletions src/SFML.Audio/SoundRecorder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ protected virtual bool OnStart()
/// <param name="samples">Array of samples to process</param>
/// <returns>False to stop recording audio data, true to continue</returns>
////////////////////////////////////////////////////////////
protected abstract bool OnProcessSamples(short[] samples);
protected abstract bool OnProcessSamples(Span<short> samples);

////////////////////////////////////////////////////////////
/// <summary>
Expand Down Expand Up @@ -286,10 +286,10 @@ private bool StartRecording(IntPtr userData)
////////////////////////////////////////////////////////////
private bool ProcessSamples(IntPtr samples, UIntPtr nbSamples, IntPtr userData)
{
short[] samplesArray = new short[(int)nbSamples];
Marshal.Copy(samples, samplesArray, 0, samplesArray.Length);

return OnProcessSamples(samplesArray);
unsafe
{
return OnProcessSamples(new Span<short>((void*)samples, (int)nbSamples));
}
}

////////////////////////////////////////////////////////////
Expand Down
15 changes: 7 additions & 8 deletions src/SFML.Graphics/Font.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,17 @@ public Font(Stream stream) : base(IntPtr.Zero)
/// <param name="bytes">Byte array containing the file contents</param>
/// <exception cref="LoadingFailedException" />
////////////////////////////////////////////////////////////
public Font(byte[] bytes) :
public Font(Span<byte> bytes) :
base(IntPtr.Zero)
{
GCHandle pin = GCHandle.Alloc(bytes, GCHandleType.Pinned);
try
unsafe
{
CPointer = sfFont_createFromMemory(pin.AddrOfPinnedObject(), (UIntPtr)bytes.Length);
}
finally
{
pin.Free();
fixed (byte* ptr = bytes)
{
CPointer = sfFont_createFromMemory((IntPtr)ptr, (UIntPtr)bytes.Length);
}
}

if (CPointer == IntPtr.Zero)
{
throw new LoadingFailedException("font");
Expand Down
34 changes: 18 additions & 16 deletions src/SFML.Graphics/Image.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,17 @@ public Image(Stream stream) :
/// <param name="bytes">Byte array containing the file contents</param>
/// <exception cref="LoadingFailedException" />
////////////////////////////////////////////////////////////
public Image(byte[] bytes) :
public Image(Span<byte> bytes) :
base(IntPtr.Zero)
{
GCHandle pin = GCHandle.Alloc(bytes, GCHandleType.Pinned);
try
{
CPointer = sfImage_createFromMemory(pin.AddrOfPinnedObject(), (UIntPtr)bytes.Length);
}
finally
unsafe
{
pin.Free();
fixed (byte* ptr = bytes)
{
CPointer = sfImage_createFromMemory((IntPtr)ptr, (UIntPtr)bytes.Length);
}
}

if (CPointer == IntPtr.Zero)
{
throw new LoadingFailedException("image");
Expand Down Expand Up @@ -148,14 +147,14 @@ public Image(Color[,] pixels) :
/// <param name="pixels">array containing the pixels</param>
/// <exception cref="LoadingFailedException" />
////////////////////////////////////////////////////////////
public Image(uint width, uint height, byte[] pixels) :
public Image(uint width, uint height, Span<byte> pixels) :
base(IntPtr.Zero)
{
unsafe
{
fixed (byte* PixelsPtr = pixels)
fixed (byte* ptr = pixels)
{
CPointer = sfImage_createFromPixels(width, height, PixelsPtr);
CPointer = sfImage_createFromPixels(width, height, ptr);
}
}

Expand Down Expand Up @@ -310,14 +309,17 @@ public void SetPixel(uint x, uint y, Color color)
/// </summary>
/// <returns>Array of pixels</returns>
////////////////////////////////////////////////////////////
public byte[] Pixels
public Span<byte> Pixels
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it reasonable to add a set method for Pixels?

{
get
{
Vector2u size = Size;
byte[] PixelsPtr = new byte[size.X * size.Y * 4];
Marshal.Copy(sfImage_getPixelsPtr(CPointer), PixelsPtr, 0, PixelsPtr.Length);
return PixelsPtr;
unsafe
{
Vector2u size = Size;
IntPtr ptr = sfImage_getPixelsPtr(CPointer);
uint length = size.X * size.Y * 4;
return new Span<byte>((void*)ptr, (int)length);
}
}
}

Expand Down
28 changes: 3 additions & 25 deletions src/SFML.Graphics/RenderTarget.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using SFML.System;

namespace SFML.Graphics
Expand Down Expand Up @@ -179,7 +180,7 @@ public interface RenderTarget
/// <param name="vertices">Array of vertices to draw</param>
/// <param name="type">Type of primitives to draw</param>
////////////////////////////////////////////////////////////
void Draw(Vertex[] vertices, PrimitiveType type);
void Draw(Span<Vertex> vertices, PrimitiveType type);

////////////////////////////////////////////////////////////
/// <summary>
Expand All @@ -189,30 +190,7 @@ public interface RenderTarget
/// <param name="type">Type of primitives to draw</param>
/// <param name="states">Render states to use for drawing</param>
////////////////////////////////////////////////////////////
void Draw(Vertex[] vertices, PrimitiveType type, RenderStates states);

////////////////////////////////////////////////////////////
/// <summary>
/// Draw primitives defined by a sub-array of vertices, with default render states
/// </summary>
/// <param name="vertices">Array of vertices to draw</param>
/// <param name="start">Index of the first vertex to draw in the array</param>
/// <param name="count">Number of vertices to draw</param>
/// <param name="type">Type of primitives to draw</param>
////////////////////////////////////////////////////////////
void Draw(Vertex[] vertices, uint start, uint count, PrimitiveType type);

////////////////////////////////////////////////////////////
/// <summary>
/// Draw primitives defined by a sub-array of vertices
/// </summary>
/// <param name="vertices">Pointer to the vertices</param>
/// <param name="start">Index of the first vertex to use in the array</param>
/// <param name="count">Number of vertices to draw</param>
/// <param name="type">Type of primitives to draw</param>
/// <param name="states">Render states to use for drawing</param>
////////////////////////////////////////////////////////////
void Draw(Vertex[] vertices, uint start, uint count, PrimitiveType type, RenderStates states);
void Draw(Span<Vertex> vertices, PrimitiveType type, RenderStates states);

////////////////////////////////////////////////////////////
/// <summary>
Expand Down
35 changes: 3 additions & 32 deletions src/SFML.Graphics/RenderTexture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ public void Draw(Drawable drawable, RenderStates states)
/// <param name="vertices">Pointer to the vertices</param>
/// <param name="type">Type of primitives to draw</param>
////////////////////////////////////////////////////////////
public void Draw(Vertex[] vertices, PrimitiveType type)
public void Draw(Span<Vertex> vertices, PrimitiveType type)
{
Draw(vertices, type, RenderStates.Default);
}
Expand All @@ -373,44 +373,15 @@ public void Draw(Vertex[] vertices, PrimitiveType type)
/// <param name="type">Type of primitives to draw</param>
/// <param name="states">Render states to use for drawing</param>
////////////////////////////////////////////////////////////
public void Draw(Vertex[] vertices, PrimitiveType type, RenderStates states)
{
Draw(vertices, 0, (uint)vertices.Length, type, states);
}

////////////////////////////////////////////////////////////
/// <summary>
/// Draw primitives defined by a sub-array of vertices, with default render states
/// </summary>
/// <param name="vertices">Array of vertices to draw</param>
/// <param name="start">Index of the first vertex to draw in the array</param>
/// <param name="count">Number of vertices to draw</param>
/// <param name="type">Type of primitives to draw</param>
////////////////////////////////////////////////////////////
public void Draw(Vertex[] vertices, uint start, uint count, PrimitiveType type)
{
Draw(vertices, start, count, type, RenderStates.Default);
}

////////////////////////////////////////////////////////////
/// <summary>
/// Draw primitives defined by a sub-array of vertices
/// </summary>
/// <param name="vertices">Pointer to the vertices</param>
/// <param name="start">Index of the first vertex to use in the array</param>
/// <param name="count">Number of vertices to draw</param>
/// <param name="type">Type of primitives to draw</param>
/// <param name="states">Render states to use for drawing</param>
////////////////////////////////////////////////////////////
public void Draw(Vertex[] vertices, uint start, uint count, PrimitiveType type, RenderStates states)
public void Draw(Span<Vertex> vertices, PrimitiveType type, RenderStates states)
{
RenderStates.MarshalData marshaledStates = states.Marshal();

unsafe
{
fixed (Vertex* vertexPtr = vertices)
{
sfRenderTexture_drawPrimitives(CPointer, vertexPtr + start, (UIntPtr)count, type, ref marshaledStates);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to FORCE using slices? This removes the ability to specify offsets and lengths during Draw()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I see it, the second overload is just a span-like version of the first one, so users could just do vertices.Slice(0, 3) if they want to do offsets and lengths.

sfRenderTexture_drawPrimitives(CPointer, vertexPtr, (UIntPtr)vertices.Length, type, ref marshaledStates);
}
}
}
Expand Down
Loading