Skip to content

Commit 989bb7d

Browse files
SoreepeongSoreepeong
committed
More memory safety/debug tagging
1 parent b0a1a1e commit 989bb7d

16 files changed

+937
-698
lines changed

‎Dalamud/ImGuiScene/Helpers/UnknownComBase.cs

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
using System.Diagnostics.CodeAnalysis;
2+
using System.Runtime.CompilerServices;
3+
using System.Runtime.InteropServices;
4+
using System.Threading;
5+
6+
using TerraFX.Interop.Windows;
7+
8+
namespace Dalamud.ImGuiScene.Helpers;
9+
10+
/// <summary>
11+
/// Base for implementing COM. The implementor must have this struct at the beginning of the struct layout.
12+
/// </summary>
13+
/// <remarks>
14+
/// Would use generics as UnknownComBase{T}, but doing so makes the types containing this fail to initialize.
15+
/// </remarks>
16+
[StructLayout(LayoutKind.Sequential)]
17+
[SuppressMessage(
18+
"StyleCop.CSharp.SpacingRules",
19+
"SA1023:Dereference and access of symbols should be spaced correctly",
20+
Justification = "Wtf")]
21+
internal unsafe struct UnknownComBase
22+
{
23+
private static readonly nint[] Vtbl;
24+
25+
private void* pVtbl;
26+
private void* pObject;
27+
private uint* pRefCount;
28+
private delegate*<void*, in Guid, void*> pfnDynamicCast;
29+
private delegate*<void*, void> pfnFinalRelease;
30+
31+
static UnknownComBase()
32+
{
33+
Vtbl = GC.AllocateArray<nint>(3, true);
34+
Vtbl[0] = (nint)(delegate*<UnknownComBase*, Guid*, void**, HRESULT>)&StaticQueryInterface;
35+
Vtbl[1] = (nint)(delegate*<UnknownComBase*, uint>)&StaticAddRef;
36+
Vtbl[2] = (nint)(delegate*<UnknownComBase*, uint>)&StaticRelease;
37+
}
38+
39+
/// <summary>
40+
/// Initializes a new instance of the <see cref="UnknownComBase"/> struct.
41+
/// </summary>
42+
/// <param name="pObject">A pointer to the parent struct.</param>
43+
/// <param name="pRefCount">A pointer to the reference counter.</param>
44+
/// <param name="pfnDynamicCast">Dynamic cast function. Return null to <see cref="E.E_NOINTERFACE"/>. Do not change refcount.</param>
45+
/// <param name="pfnFinalRelease">Final release function.</param>
46+
public UnknownComBase(
47+
void* pObject,
48+
uint* pRefCount,
49+
delegate*<void*, in Guid, void*> pfnDynamicCast,
50+
delegate*<void*, void> pfnFinalRelease = null)
51+
{
52+
this.pVtbl = Unsafe.AsPointer(ref Vtbl[0]);
53+
this.pObject = pObject;
54+
this.pRefCount = pRefCount;
55+
this.pfnDynamicCast = pfnDynamicCast;
56+
this.pfnFinalRelease = pfnFinalRelease;
57+
}
58+
59+
/// <summary>
60+
/// Gets the address of this struct as a pointer to <see cref="IUnknown"/>.
61+
/// </summary>
62+
public IUnknown* AsPunk => (IUnknown*)Unsafe.AsPointer(ref this);
63+
64+
private static HRESULT StaticQueryInterface(UnknownComBase* self, Guid* piid, void** ppvObject)
65+
{
66+
if (ppvObject == null)
67+
return E.E_POINTER;
68+
69+
*ppvObject = null;
70+
if (piid == null)
71+
return E.E_INVALIDARG;
72+
73+
if (*piid == IID.IID_IUnknown)
74+
{
75+
StaticAddRef(self);
76+
*ppvObject = self;
77+
return S.S_OK;
78+
}
79+
80+
if ((*ppvObject = self->pfnDynamicCast(self->pObject, *piid)) is null)
81+
return E.E_NOINTERFACE;
82+
83+
StaticAddRef(self);
84+
return S.S_OK;
85+
}
86+
87+
private static uint StaticAddRef(UnknownComBase* self) => Interlocked.Increment(ref *self->pRefCount);
88+
89+
private static uint StaticRelease(UnknownComBase* self)
90+
{
91+
var r = Interlocked.Decrement(ref *self->pRefCount);
92+
if (r > 0)
93+
return r;
94+
if (self->pfnFinalRelease is not null)
95+
self->pfnFinalRelease(self->pObject);
96+
Marshal.FreeHGlobal((nint)self->pObject);
97+
return 0;
98+
}
99+
}

‎Dalamud/ImGuiScene/IImGuiRenderer.cs

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
using Dalamud.Interface.Internal;
1+
using System.Runtime.CompilerServices;
2+
3+
using Dalamud.Interface.Internal;
24

35
using ImGuiNET;
46

@@ -41,26 +43,20 @@ internal interface IImGuiRenderer : IDisposable
4143

4244
/// <summary>
4345
/// Sets the texture pipeline. The pipeline must be created from the concrete implementation of this interface.<br />
44-
/// The references of <paramref name="textureHandle"/> and <paramref name="pipelineHandle"/> are copied.
45-
/// You may dispose <paramref name="pipelineHandle"/> after the call.
46+
/// The references of <paramref name="texture"/> and <paramref name="pipeline"/> are copied.
47+
/// You may dispose <paramref name="pipeline"/> after the call.
4648
/// </summary>
47-
/// <param name="textureHandle">The texture handle.</param>
48-
/// <param name="pipelineHandle">The pipeline handle.</param>
49-
void SetTexturePipeline(nint textureHandle, nint pipelineHandle);
49+
/// <param name="texture">The texture handle.</param>
50+
/// <param name="pipeline">The pipeline handle to set, or null to clear.</param>
51+
void SetTexturePipeline(IDalamudTextureWrap texture, ITexturePipelineWrap? pipeline);
5052

5153
/// <summary>
5254
/// Creates a new reference of the pipeline registered for use with the given texture.<br />
5355
/// Dispose after use.
5456
/// </summary>
55-
/// <param name="textureHandle">The texture handle.</param>
56-
/// <returns>The previous pixel shader handle, or 0 if none.</returns>
57-
nint GetTexturePipeline(nint textureHandle);
58-
59-
/// <summary>
60-
/// Disposes a reference to the pipeline.
61-
/// </summary>
62-
/// <param name="pipelineHandle">The pipeline handle.</param>
63-
void ReleaseTexturePipeline(nint pipelineHandle);
57+
/// <param name="texture">The texture handle.</param>
58+
/// <returns>The previous pixel shader handle, or null if none.</returns>
59+
ITexturePipelineWrap? GetTexturePipeline(IDalamudTextureWrap texture);
6460

6561
/// <summary>
6662
/// Adds a user callback handler.
@@ -83,6 +79,13 @@ internal interface IImGuiRenderer : IDisposable
8379
/// <param name="width">The width in pixels.</param>
8480
/// <param name="height">The height in pixels.</param>
8581
/// <param name="format">Format of the texture.</param>
82+
/// <param name="debugName">Name for debugging.</param>
8683
/// <returns>A texture, ready to use in ImGui.</returns>
87-
IDalamudTextureWrap LoadTexture(ReadOnlySpan<byte> data, int pitch, int width, int height, int format);
84+
IDalamudTextureWrap LoadTexture(
85+
ReadOnlySpan<byte> data,
86+
int pitch,
87+
int width,
88+
int height,
89+
int format,
90+
[CallerMemberName] string debugName = "");
8891
}

‎Dalamud/ImGuiScene/IImGuiScene.cs

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -117,28 +117,11 @@ internal interface IImGuiScene : IDisposable
117117
/// <returns>A texture, ready to use in ImGui.</returns>
118118
IDalamudTextureWrap LoadImageFormat(ReadOnlySpan<byte> data, int pitch, int width, int height, int format);
119119

120-
/// <summary>
121-
/// Sets the texture pipeline. The pipeline must be created from the concrete implementation of this interface.<br />
122-
/// The references of <paramref name="textureHandle"/> and <paramref name="pipelineHandle"/> are copied.
123-
/// You may dispose <paramref name="pipelineHandle"/> after the call.
124-
/// </summary>
125-
/// <param name="textureHandle">The texture handle.</param>
126-
/// <param name="pipelineHandle">The pipeline handle.</param>
127-
void SetTexturePipeline(nint textureHandle, nint pipelineHandle);
120+
/// <inheritdoc cref="IImGuiRenderer.SetTexturePipeline"/>
121+
void SetTexturePipeline(IDalamudTextureWrap textureHandle, ITexturePipelineWrap? pipelineHandle);
128122

129-
/// <summary>
130-
/// Creates a new reference of the pipeline registered for use with the given texture.<br />
131-
/// Dispose after use.
132-
/// </summary>
133-
/// <param name="textureHandle">The texture handle.</param>
134-
/// <returns>The previous pixel shader handle, or 0 if none.</returns>
135-
nint GetTexturePipeline(nint textureHandle);
136-
137-
/// <summary>
138-
/// Disposes a reference to the pipeline.
139-
/// </summary>
140-
/// <param name="pipelineHandle">The pipeline handle.</param>
141-
void ReleaseTexturePipeline(nint pipelineHandle);
123+
/// <inheritdoc cref="IImGuiRenderer.GetTexturePipeline"/>
124+
ITexturePipelineWrap? GetTexturePipeline(IDalamudTextureWrap textureHandle);
142125

143126
/// <summary>
144127
/// Determines if <paramref name="cursorHandle"/> is owned by this.

‎Dalamud/ImGuiScene/ITexturePipelineWrap.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
namespace Dalamud.ImGuiScene;
2+
3+
/// <summary>
4+
/// Represents a handle to an immutable texture pipeline.
5+
/// </summary>
6+
public interface ITexturePipelineWrap : ICloneable, IDisposable
7+
{
8+
/// <summary>
9+
/// Gets a value indicating whether this instance of <see cref="ITexturePipelineWrap"/> has been disposed.
10+
/// </summary>
11+
bool IsDisposed { get; }
12+
13+
/// <inheritdoc cref="ICloneable.Clone"/>
14+
new ITexturePipelineWrap Clone();
15+
16+
/// <inheritdoc cref="ICloneable.Clone"/>
17+
object ICloneable.Clone() => this.Clone();
18+
}

0 commit comments

Comments
 (0)