-
Notifications
You must be signed in to change notification settings - Fork 323
IME Events are now completely variable sized. #1120
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.
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
Changes from 3 commits
89e1882
db7cf4b
ca0bc82
161da0a
d271453
cf1af0a
fc5fcfa
a8ea785
d8dfbc4
cad29a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,27 +10,64 @@ namespace UnityEngine.InputSystem.LowLevel | |
/// A specialized event that contains the current IME Composition string, if IME is enabled and active. | ||
/// This event contains the entire current string to date, and once a new composition is submitted will send a blank string event. | ||
/// </summary> | ||
[StructLayout(LayoutKind.Explicit, Size = InputEvent.kBaseEventSize + sizeof(int) + (sizeof(char) * kIMECharBufferSize))] | ||
[StructLayout(LayoutKind.Explicit, Size = InputEvent.kBaseEventSize + sizeof(int) + (sizeof(char)))] | ||
public struct IMECompositionEvent : IInputEventTypeInfo | ||
{ | ||
// These needs to match the native ImeCompositionStringInputEventData settings | ||
internal const int kIMECharBufferSize = 64; | ||
public const int Type = 0x494D4553; | ||
public const int Type = 0x494D4543; | ||
|
||
[FieldOffset(0)] | ||
public InputEvent baseEvent; | ||
internal InputEvent baseEvent; | ||
|
||
[FieldOffset(InputEvent.kBaseEventSize)] | ||
public IMECompositionString compositionString; | ||
internal int length; | ||
|
||
[FieldOffset(InputEvent.kBaseEventSize + sizeof(int))] | ||
internal char bufferStart; | ||
|
||
public FourCC typeStatic => Type; | ||
|
||
public static IMECompositionEvent Create(int deviceId, string compositionString, double time) | ||
internal IMECompositionString GetComposition() | ||
{ | ||
var inputEvent = new IMECompositionEvent(); | ||
inputEvent.baseEvent = new InputEvent(Type, InputEvent.kBaseEventSize + sizeof(int) + (sizeof(char) * kIMECharBufferSize), deviceId, time); | ||
inputEvent.compositionString = new IMECompositionString(compositionString); | ||
return inputEvent; | ||
unsafe | ||
{ | ||
fixed(char* buffer = &bufferStart) | ||
{ | ||
return new IMECompositionString(new IntPtr(buffer), length); | ||
} | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Queues up an IME Composition Event.!-- This event is for unit testing and debug purposes, and is slow, due to the variable way that IME Composition events work. | ||
/// </summary> | ||
/// <param name="deviceId">The Id you'd like to send the composition event at.</param> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Overall, doc comments that don't really provide information and only restate what's already obvious from the code are really aggravating. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Took that segment and did a pass on some of the other doc elements around. Tried to add a bit of something to any of the docs, or remove the code comments entirely for things like GetEnumerator. |
||
/// <param name="str">The IME composition string.</param> | ||
/// <param name="time">The time that the event occurred at.</param> | ||
public static void QueueEvent(int deviceId, string str, double time) | ||
{ | ||
unsafe | ||
{ | ||
int sizeInBytes = (InputEvent.kBaseEventSize + sizeof(int) + sizeof(char)) + (sizeof(char) * str.Length); | ||
IntPtr evtPtr = Marshal.AllocHGlobal(sizeInBytes); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Unity NativeArray (with Allocator.Temp) and Unity Memcpy instead of Marshal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not used NativeArray much, did an attempt, feel I could be missing an easier way to do it, but this passes the tests. |
||
byte* ptr = (byte*)evtPtr.ToPointer(); | ||
InputEvent* evt = (InputEvent*)ptr; | ||
|
||
*evt = new InputEvent(Type, sizeInBytes, deviceId, time); | ||
ptr += InputEvent.kBaseEventSize; | ||
|
||
int* lengthPtr = (int*)ptr; | ||
*lengthPtr = str.Length; | ||
|
||
ptr += sizeof(int); | ||
|
||
fixed(char* p = str) | ||
{ | ||
Buffer.MemoryCopy(p, ptr, str.Length * sizeof(char), str.Length * sizeof(char)); | ||
} | ||
|
||
InputSystem.QueueEvent(new InputEventPtr(evt)); | ||
Marshal.FreeHGlobal(evtPtr); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -42,35 +79,32 @@ public static IMECompositionEvent Create(int deviceId, string compositionString, | |
/// <see cref="ITextInputReceiver.OnIMECompositionChanged"/> method. It can easily be converted to a normal C# string using | ||
/// <see cref="ToString"/>, but is exposed as the raw struct to avoid allocating memory by default. | ||
/// </remarks> | ||
[StructLayout(LayoutKind.Explicit, Size = sizeof(int) + sizeof(char) * LowLevel.IMECompositionEvent.kIMECharBufferSize)] | ||
public unsafe struct IMECompositionString : IEnumerable<char> | ||
{ | ||
internal struct Enumerator : IEnumerator<char> | ||
{ | ||
IMECompositionString m_CompositionString; | ||
IntPtr m_BufferStart; | ||
int m_CharacterCount; | ||
char m_CurrentCharacter; | ||
int m_CurrentIndex; | ||
|
||
public Enumerator(IMECompositionString compositionString) | ||
{ | ||
m_CompositionString = compositionString; | ||
m_BufferStart = compositionString.m_CharBuffer; | ||
m_CharacterCount = compositionString.length; | ||
m_CurrentCharacter = '\0'; | ||
m_CurrentIndex = -1; | ||
} | ||
|
||
public bool MoveNext() | ||
{ | ||
int size = m_CompositionString.Count; | ||
|
||
m_CurrentIndex++; | ||
|
||
if (m_CurrentIndex == size) | ||
if (m_CurrentIndex == m_CharacterCount) | ||
return false; | ||
|
||
fixed(char* ptr = m_CompositionString.buffer) | ||
{ | ||
m_CurrentCharacter = *(ptr + m_CurrentIndex); | ||
} | ||
char* ptr = (char*)m_BufferStart.ToPointer(); | ||
m_CurrentCharacter = *(ptr + m_CurrentIndex); | ||
|
||
return true; | ||
} | ||
|
@@ -89,50 +123,53 @@ public void Dispose() | |
object IEnumerator.Current => Current; | ||
} | ||
|
||
public int Count => size; | ||
int m_Length; | ||
/// <summary> | ||
/// The number of characters in the current IME Composition | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Punctuation. Sentences in API docs should be properly formed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
/// </summary> | ||
/// <value>The Length of the Composition</value> | ||
public int length { get { return m_Length; }} | ||
|
||
IntPtr m_CharBuffer; | ||
|
||
|
||
/// <summary> | ||
/// An Indexer into an individual character in the IME Composition. Will throw an out of range exception if the index is greater than the length of the composition. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
/// </summary> | ||
/// <value>The character at the requested index</value> | ||
public char this[int index] | ||
{ | ||
get | ||
{ | ||
if (index >= Count || index < 0) | ||
if (index >= m_Length || index < 0) | ||
throw new ArgumentOutOfRangeException(nameof(index)); | ||
|
||
fixed(char* ptr = buffer) | ||
{ | ||
return *(ptr + index); | ||
} | ||
char* ptr = (char*)m_CharBuffer.ToPointer(); | ||
return *(ptr + index); | ||
} | ||
} | ||
|
||
[FieldOffset(0)] | ||
int size; | ||
|
||
[FieldOffset(sizeof(int))] | ||
fixed char buffer[IMECompositionEvent.kIMECharBufferSize]; | ||
|
||
public IMECompositionString(string characters) | ||
internal IMECompositionString(IntPtr charBuffer, int length) | ||
{ | ||
if (string.IsNullOrEmpty(characters)) | ||
{ | ||
size = 0; | ||
return; | ||
} | ||
|
||
Debug.Assert(characters.Length < IMECompositionEvent.kIMECharBufferSize); | ||
size = characters.Length; | ||
for (var i = 0; i < size; i++) | ||
buffer[i] = characters[i]; | ||
m_Length = length; | ||
m_CharBuffer = charBuffer; | ||
} | ||
|
||
/// <summary> | ||
/// Returns the IME Composition as a new string | ||
/// </summary> | ||
/// <returns>The IME Composition as a string</returns> | ||
public override string ToString() | ||
{ | ||
fixed(char* ptr = buffer) | ||
{ | ||
return new string(ptr, 0, size); | ||
} | ||
char* ptr = (char*)m_CharBuffer.ToPointer(); | ||
return new string(ptr, 0, m_Length); | ||
} | ||
|
||
/// <summary> | ||
/// Gets an Enumerator that enumerates over the individual IME Composition characters | ||
/// </summary> | ||
/// <returns>The IME Composition Enuemrator</returns> | ||
public IEnumerator<char> GetEnumerator() | ||
{ | ||
return new Enumerator(this); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it slow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed, I didn't like the AllocHGlobal and copying 1 character at a time. Had no good justification it was slow.