From fa821ec603fcb4da1d8cb2dcdadbdaa25baebfba Mon Sep 17 00:00:00 2001 From: ousttrue Date: Wed, 22 Dec 2021 15:24:32 +0900 Subject: [PATCH 1/3] add SafeMarshalCopy --- .../Runtime/Extensions/ArrayExtensions.cs | 28 +------ Assets/UniGLTF/Runtime/UniGLTF/IO/ArrayPin.cs | 22 +----- .../UniGLTF/Runtime/UniGLTF/IO/BytesReader.cs | 7 +- Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs | 4 +- .../Runtime/UniGLTF/IO/SafeMarshalCopy.cs | 77 +++++++++++++++++++ .../UniGLTF/IO/SafeMarshalCopy.cs.meta | 11 +++ .../4x4_gray_import_as_linear.png.meta | 4 +- .../4x4_gray_import_as_normal_map.png.meta | 4 +- .../UniGLTF/4x4_gray_import_as_srgb.png.meta | 10 +-- Assets/VRM10/Tests/MigrationTests.cs | 10 +-- 10 files changed, 110 insertions(+), 67 deletions(-) create mode 100644 Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs create mode 100644 Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs.meta diff --git a/Assets/UniGLTF/Runtime/Extensions/ArrayExtensions.cs b/Assets/UniGLTF/Runtime/Extensions/ArrayExtensions.cs index 53216fab5..42d15c547 100644 --- a/Assets/UniGLTF/Runtime/Extensions/ArrayExtensions.cs +++ b/Assets/UniGLTF/Runtime/Extensions/ArrayExtensions.cs @@ -89,15 +89,6 @@ namespace UniGLTF public static class ArrayExtensions { - public static int MarshalCopyTo(this ArraySegment src, T[] dst) where T : struct - { - var size = dst.Length * Marshal.SizeOf(typeof(T)); - using (var pin = Pin.Create(dst)) - { - Marshal.Copy(src.Array, src.Offset, pin.Ptr, size); - } - return size; - } public static T[] SelectInplace(this T[] src, Func pred) { @@ -107,21 +98,6 @@ namespace UniGLTF } return src; } - - public static void Copy(ArraySegment src, ArraySegment dst) - where TFrom : struct - where TTo : struct - { - var bytes = new byte[src.Count * Marshal.SizeOf(typeof(TFrom))]; - using (var pin = Pin.Create(src)) - { - Marshal.Copy(pin.Ptr, bytes, 0, bytes.Length); - }; - using (var pin = Pin.Create(dst)) - { - Marshal.Copy(bytes, 0, pin.Ptr, bytes.Length); - }; - } } public static class ListExtensions @@ -134,7 +110,7 @@ namespace UniGLTF } public static class ArraySegmentExtensions - { + { public static ArraySegment Slice(this ArraySegment self, int start, int length) { if (start + length > self.Count) @@ -156,6 +132,6 @@ namespace UniGLTF } return self.Slice(start, self.Count - start); } - + } } diff --git a/Assets/UniGLTF/Runtime/UniGLTF/IO/ArrayPin.cs b/Assets/UniGLTF/Runtime/UniGLTF/IO/ArrayPin.cs index 2a1c85a54..f488751ea 100644 --- a/Assets/UniGLTF/Runtime/UniGLTF/IO/ArrayPin.cs +++ b/Assets/UniGLTF/Runtime/UniGLTF/IO/ArrayPin.cs @@ -89,30 +89,14 @@ namespace UniGLTF { public static int FromBytes(this ArraySegment src, T[] dst) where T : struct { - var dstSize = dst.Length * Marshal.SizeOf(typeof(T)); - if (src.Count > dstSize) - { - throw new ArgumentOutOfRangeException(); - } - using (var pin = ArrayPin.Create(dst)) - { - Marshal.Copy(src.Array, src.Offset, pin.Ptr, src.Count); - } + SafeMarshalCopy.CopyBytesToArray(src, dst); return src.Count; } public static int ToBytes(this T[] src, ArraySegment dst) where T : struct { - var srcSize = src.Length * Marshal.SizeOf(typeof(T)); - if (srcSize > dst.Count) - { - throw new ArgumentOutOfRangeException(); - } - using (var pin = ArrayPin.Create(src)) - { - Marshal.Copy(pin.Ptr, dst.Array, dst.Offset, srcSize); - } - return srcSize; + SafeMarshalCopy.CopyArrayToToBytes(src, dst); + return dst.Count; } } } diff --git a/Assets/UniGLTF/Runtime/UniGLTF/IO/BytesReader.cs b/Assets/UniGLTF/Runtime/UniGLTF/IO/BytesReader.cs index d3e8e58a1..589649c05 100644 --- a/Assets/UniGLTF/Runtime/UniGLTF/IO/BytesReader.cs +++ b/Assets/UniGLTF/Runtime/UniGLTF/IO/BytesReader.cs @@ -10,7 +10,7 @@ namespace UniGLTF Byte[] m_bytes; int m_pos; - public BytesReader(Byte[] bytes, int pos=0) + public BytesReader(Byte[] bytes, int pos = 0) { m_bytes = bytes; m_pos = pos; @@ -63,8 +63,9 @@ namespace UniGLTF public void ReadToArray(T[] dst) where T : struct { - var size = new ArraySegment(m_bytes, m_pos, m_bytes.Length - m_pos).MarshalCopyTo(dst); - m_pos += size; + var bytes = new ArraySegment(m_bytes, m_pos, m_bytes.Length - m_pos); + SafeMarshalCopy.CopyBytesToArray(bytes, dst); + m_pos += bytes.Count; } public T ReadStruct() where T : struct diff --git a/Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs b/Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs index 59c88990b..f37401fb5 100644 --- a/Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs +++ b/Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs @@ -157,7 +157,7 @@ namespace UniGLTF var segment = GetBytesFromBuffer(view.buffer); var attrib = new T[count]; var bytes = new ArraySegment(segment.Array, segment.Offset + view.byteOffset + byteOffset, count * view.byteStride); - bytes.MarshalCopyTo(attrib); + SafeMarshalCopy.CopyBytesToArray(bytes, attrib); return attrib; } @@ -289,7 +289,7 @@ namespace UniGLTF var view = GLTF.bufferViews[vertexAccessor.bufferView]; var segment = GetBytesFromBuffer(view.buffer); var bytes = new ArraySegment(segment.Array, segment.Offset + view.byteOffset + vertexAccessor.byteOffset, vertexAccessor.count * view.byteStride); - bytes.MarshalCopyTo(attrib); + SafeMarshalCopy.CopyBytesToArray(bytes, attrib); result = attrib; } else diff --git a/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs b/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs new file mode 100644 index 000000000..c3674efb8 --- /dev/null +++ b/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs @@ -0,0 +1,77 @@ +using System; +using System.Runtime.InteropServices; + +namespace UniGLTF +{ + /// + /// Marshal.Copy + /// * ptr to bytes + /// * bytes to ptr + /// の両方向がある。 + /// ptr になったら範囲は分からん。 + /// ptr にする前に範囲チェックするのを明確にするのがこの Utility の意図である。 + /// + /// Marshal.Copy を使わずにこの関数を使うべし + /// + /// + public static class SafeMarshalCopy + { + /// + /// bytes から T[] へのコピー + /// + public static void CopyBytesToArray(ArraySegment src, T[] dst) where T : struct + { + if (src.Array == null || dst == null) + { + throw new System.ArgumentNullException(); + } + if (src.Offset < 0) + { + throw new System.AccessViolationException("CopyBytesToArray: ArraySegment: negative offset"); + } + if ((src.Offset + src.Count) > src.Array.Length) + { + throw new System.AccessViolationException("CopyBytesToArray: ArraySegment: exceed"); + } + var dstByteSize = dst.Length * Marshal.SizeOf(typeof(T)); + if (src.Count > dstByteSize) + { + throw new System.AccessViolationException("CopyBytesToArray: src > dst"); + } + + using (var pin = Pin.Create(dst)) + { + Marshal.Copy(src.Array, src.Offset, pin.Ptr, src.Count); + } + } + + /// + /// T[] から bytes へのコピー + /// + public static void CopyArrayToToBytes(T[] src, ArraySegment dst) where T : struct + { + if (dst.Array == null || src == null) + { + throw new System.ArgumentNullException(); + } + if (dst.Offset < 0) + { + throw new System.AccessViolationException("CopyArrayToToBytes: ArraySegment: negative offset"); + } + if (dst.Offset + dst.Count > dst.Array.Length) + { + throw new System.AccessViolationException("CopyArrayToToBytes: ArraySegment: exceed"); + } + var srcByteSize = src.Length * Marshal.SizeOf(typeof(T)); + if (srcByteSize > dst.Count) + { + throw new System.AccessViolationException("CopyArrayToToBytes: src > dst"); + } + + using (var pin = Pin.Create(src)) + { + Marshal.Copy(pin.Ptr, dst.Array, dst.Offset, srcByteSize); + } + } + } +} diff --git a/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs.meta b/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs.meta new file mode 100644 index 000000000..1fc2117da --- /dev/null +++ b/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 234ebccfeacecfe4da0351a092c985af +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_linear.png.meta b/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_linear.png.meta index d40385c4f..8968f3a02 100644 --- a/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_linear.png.meta +++ b/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_linear.png.meta @@ -32,8 +32,8 @@ TextureImporter: textureSettings: serializedVersion: 2 filterMode: 0 - aniso: -1 - mipBias: -100 + aniso: 1 + mipBias: 0 wrapU: 0 wrapV: 0 wrapW: 0 diff --git a/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_normal_map.png.meta b/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_normal_map.png.meta index 746c100ae..01d1f4a11 100644 --- a/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_normal_map.png.meta +++ b/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_normal_map.png.meta @@ -32,8 +32,8 @@ TextureImporter: textureSettings: serializedVersion: 2 filterMode: 0 - aniso: -1 - mipBias: -100 + aniso: 1 + mipBias: 0 wrapU: 0 wrapV: 0 wrapW: 0 diff --git a/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_srgb.png.meta b/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_srgb.png.meta index 2f875a10b..ab00cb466 100644 --- a/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_srgb.png.meta +++ b/Assets/UniGLTF/Tests/UniGLTF/4x4_gray_import_as_srgb.png.meta @@ -32,11 +32,11 @@ TextureImporter: textureSettings: serializedVersion: 2 filterMode: 0 - aniso: -1 - mipBias: -100 - wrapU: -1 - wrapV: -1 - wrapW: -1 + aniso: 1 + mipBias: 0 + wrapU: 0 + wrapV: 0 + wrapW: 0 nPOTScale: 1 lightmap: 0 compressionQuality: 50 diff --git a/Assets/VRM10/Tests/MigrationTests.cs b/Assets/VRM10/Tests/MigrationTests.cs index cc0c44e85..21ea537c2 100644 --- a/Assets/VRM10/Tests/MigrationTests.cs +++ b/Assets/VRM10/Tests/MigrationTests.cs @@ -121,10 +121,7 @@ namespace UniVRM10 u.m33 = 15; Assert.AreEqual(new UnityEngine.Vector4(0, 1, 2, 3), u.GetRow(0)); var bytes = new Byte[64]; - using (var pin = Pin.Create(new[] { u })) - { - Marshal.Copy(pin.Ptr, bytes, 0, 64); - } + SafeMarshalCopy.CopyArrayToToBytes(new[] { u }, new ArraySegment(bytes)); Assert.AreEqual(1.0f, BitConverter.ToSingle(bytes, 16)); } @@ -149,10 +146,7 @@ namespace UniVRM10 u.M43 = 14; u.M44 = 15; var bytes = new Byte[64]; - using (var pin = Pin.Create(new[] { u })) - { - Marshal.Copy(pin.Ptr, bytes, 0, 64); - } + SafeMarshalCopy.CopyArrayToToBytes(new[] { u }, new ArraySegment(bytes)); Assert.AreEqual(1.0f, BitConverter.ToSingle(bytes, 4)); } From 4d8cfb95d89c144521cf1e1061ab2ed3c24054d0 Mon Sep 17 00:00:00 2001 From: ousttrue Date: Wed, 22 Dec 2021 15:47:18 +0900 Subject: [PATCH 2/3] ArraySegment.Count --- Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs b/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs index c3674efb8..1e33d8324 100644 --- a/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs +++ b/Assets/UniGLTF/Runtime/UniGLTF/IO/SafeMarshalCopy.cs @@ -21,7 +21,7 @@ namespace UniGLTF /// public static void CopyBytesToArray(ArraySegment src, T[] dst) where T : struct { - if (src.Array == null || dst == null) + if (src.Array == null || dst == null || src.Count == 0) { throw new System.ArgumentNullException(); } @@ -50,7 +50,7 @@ namespace UniGLTF /// public static void CopyArrayToToBytes(T[] src, ArraySegment dst) where T : struct { - if (dst.Array == null || src == null) + if (dst.Array == null || src == null || dst.Count == 0) { throw new System.ArgumentNullException(); } From 8fbcb5c6a719a3ac3e9a8118f7fb5e8a781f3de8 Mon Sep 17 00:00:00 2001 From: ousttrue Date: Wed, 22 Dec 2021 16:01:44 +0900 Subject: [PATCH 3/3] =?UTF-8?q?bufferView.stride=20=E3=81=8C=E7=84=A1?= =?UTF-8?q?=E3=81=84=E3=81=A8=E3=81=8D=E3=81=AE=E5=87=A6=E7=BD=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/KhronosGroup/glTF/blob/main/specification/2.0/schema/bufferView.schema.json#L24 --- Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs b/Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs index f37401fb5..614dc3ad2 100644 --- a/Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs +++ b/Assets/UniGLTF/Runtime/UniGLTF/IO/GltfData.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Runtime.InteropServices; namespace UniGLTF { @@ -156,7 +157,7 @@ namespace UniGLTF { var segment = GetBytesFromBuffer(view.buffer); var attrib = new T[count]; - var bytes = new ArraySegment(segment.Array, segment.Offset + view.byteOffset + byteOffset, count * view.byteStride); + var bytes = new ArraySegment(segment.Array, segment.Offset + view.byteOffset + byteOffset, count * Marshal.SizeOf()); SafeMarshalCopy.CopyBytesToArray(bytes, attrib); return attrib; } @@ -288,7 +289,7 @@ namespace UniGLTF var attrib = new float[vertexAccessor.count * vertexAccessor.TypeCount]; var view = GLTF.bufferViews[vertexAccessor.bufferView]; var segment = GetBytesFromBuffer(view.buffer); - var bytes = new ArraySegment(segment.Array, segment.Offset + view.byteOffset + vertexAccessor.byteOffset, vertexAccessor.count * view.byteStride); + var bytes = new ArraySegment(segment.Array, segment.Offset + view.byteOffset + vertexAccessor.byteOffset, vertexAccessor.count * 4 * vertexAccessor.TypeCount); SafeMarshalCopy.CopyBytesToArray(bytes, attrib); result = attrib; }