Keyboard optimization #1133
Merged
Keyboard optimization #1133
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lkit-Unity into Keyboard_Optimization # Conflicts: # Assets/HoloToolkit-Examples/Input/Scenes/KeyboardTest.unity
The animation of the caret causes a redraw of the whole keyboard if it is not in a separate child canvas
This prevents the batching from breaking for the highlighted elemented
The background somehow broke batching on all its children. Also the caret doesn't seem to update the canvas.
Also rename private variable to match convention
| private void Update() | ||
| { | ||
| #if UNITY_EDITOR |
There was a problem hiding this comment.
We should probably use if (Application.isEditor) instead.
There was a problem hiding this comment.
Won't we have a lot of unnecessary calls to methods that won't be there otherwise?
There was a problem hiding this comment.
What if we tag the method as [Conditional("UNITY_EDITOR")]
There was a problem hiding this comment.
The method gets called whether it's in the editor or not cause it's a unity API
StephenHodgson
approved these changes
Oct 17, 2017
|
|
||
| if (m_rectTransform == null) | ||
| if (rectTransform == null) | ||
| { | ||
| Debug.LogError("This component must be attached to a GameObject with a RectTransform component!"); |
There was a problem hiding this comment.
Do we really need this debug log if the component is required?
The component is required so it can't be null.
|
Over to you @StephenHodgson |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #853
Fixes #945
Since we have the UI raycast changes now in the master it's time to add the keyboard optimizations since I can be sure the raycasting will not cause issues.
I explained most of the optimizations already in #853 and I ended up with some additional minor changes:
All of these changes are on the Keyboard prefab and there are no code changes involved (other than the minor refactoring I did on UICollection).
This should be tested on the HoloLens as it was pretty much unusable before due to the lag from the keyboard. Unfortunately, I won't have access to one in the next few weeks. It can also be tested in the KeyboardTest-scene.
For the sprite atlas, I also had to turn on the Sprite Packer in the new mode. This will also be required for people that want to use the keyboard. Without this setting, the performance of the keyboard will be slightly worse.
Maybe it would make sense to add this setting to the configuration steps?