review feedback #1

Author: pmavridis

com.unity.render-pipelines.high-definition/Editor/Compositor/CompositionLayerUI.Drawers.cs

@@ -38,51 +38,52 @@ public static void DrawItemInList(Rect rect, SerializedCompositionLayer serializ
 
             // Compute the desired indentation 
             {
-                rect.x = isCameraStack ? rect.x + CompositorStyle.k_ListItemStackPading + 2 : rect.x + 2;
-                rect.width = isCameraStack ? rect.width - CompositorStyle.k_ListItemStackPading : rect.width;
+                const float listBorder = 2.0f;
+                rect.x = isCameraStack ? rect.x + CompositorStyle.k_ListItemStackPading + listBorder : rect.x + listBorder;
+                rect.width = isCameraStack ? rect.width - CompositorStyle.k_ListItemStackPading - listBorder : rect.width - listBorder;
                 rect.y += CompositorStyle.k_ListItemPading;
                 rect.height = EditorGUIUtility.singleLineHeight;
             }
 
             if (thumbnail)
             {
                 Rect newRect = rect;
-                newRect.width = 20;
+                newRect.width = EditorGUIUtility.singleLineHeight;
                 EditorGUI.PropertyField(newRect, serialized.Show, GUIContent.none);
-                rect.x += 20;
+                rect.x += CompositorStyle.k_CheckboxSpacing;
                 Rect previewRect = rect;
                 previewRect.width = CompositorStyle.k_ThumbnailSize * aspectRatio;
                 previewRect.height = CompositorStyle.k_ThumbnailSize;
                 EditorGUI.DrawPreviewTexture(previewRect, thumbnail);
-                previewRect.x += previewRect.width + 5;
-                rect.x += previewRect.width + 12;
-                rect.width -= previewRect.width + 12;
+                previewRect.x += previewRect.width + CompositorStyle.k_ThumbnailDivider;
+                rect.x += previewRect.width + CompositorStyle.k_ThumbnailSpacing;
+                rect.width -= previewRect.width + CompositorStyle.k_ThumbnailSpacing;
 
                 if (isAlphaEnbaled
                     && (thumbnail.format == RenderTextureFormat.ARGBHalf
                     || thumbnail.format == RenderTextureFormat.ARGBFloat
                     || thumbnail.format == RenderTextureFormat.ARGB64))
                 {
                     EditorGUI.DrawTextureAlpha(previewRect, thumbnail);
-                    rect.x += previewRect.width + 12;
-                    rect.width -= previewRect.width + 12;
+                    rect.x += previewRect.width + CompositorStyle.k_ThumbnailSpacing;
+                    rect.width -= previewRect.width + CompositorStyle.k_ThumbnailSpacing;
                 }
 
-                rect.y += 6;
+                rect.y += CompositorStyle.k_LabelVerticalOffset;
                 EditorGUI.LabelField(rect, serialized.LayerName.stringValue);
             }
             else
             {
                 Rect newRect = rect;
-                newRect.width = 20;
+                newRect.width = EditorGUIUtility.singleLineHeight;
                 EditorGUI.PropertyField(newRect, serialized.Show, GUIContent.none);
-                newRect.x += 20;
+                newRect.x += CompositorStyle.k_CheckboxSpacing;
                 if (isCameraStack)
                 {
                     Rect iconRect = newRect;
                     iconRect.width = CompositorStyle.k_IconSize;
                     iconRect.height = CompositorStyle.k_IconSize;
-                    iconRect.y -= 5;
+                    iconRect.y -= CompositorStyle.k_IconVerticalOffset;
                     switch (serialized.InputLayerType.enumValueIndex)
                     {
                         case (int)CompositorLayer.LayerType.Camera:
@@ -99,12 +100,11 @@ public static void DrawItemInList(Rect rect, SerializedCompositionLayer serializ
                             Debug.Log("Unknown layer type: Please add code here to draw this type of layer.");
                             break;
                     }
-                    newRect.x += CompositorStyle.k_IconSize + 5;
+                    newRect.x += CompositorStyle.k_IconSize + CompositorStyle.k_IconSpacing;
                 }
 
-                newRect.width = rect.width - 60 - 20;
+                newRect.width = rect.width - newRect.x;
                 EditorGUI.LabelField(newRect, serialized.LayerName.stringValue);
-                rect.y += rect.height;
             }
         }
 

com.unity.render-pipelines.high-definition/Editor/Compositor/CompositionManagerEditor.Styles.cs

@@ -9,6 +9,15 @@ namespace UnityEditor.Rendering.HighDefinition.Compositor
     static internal class CompositorStyle
     {
         internal static readonly int k_ThumbnailSize = 32;
+        internal static readonly int k_ThumbnailDivider = 5;   // the horizontal space in between the thumbnails
+        internal static readonly int k_ThumbnailSpacing = 10;  // the horizontal space after the thumbnail
+        internal static readonly int k_IconSpacing = 5;        // the horizontal space after an icon
+        internal static readonly int k_CheckboxSpacing = 20;   // the horizontal space for a checkbox
+
+        internal static readonly int k_IconVerticalOffset = 5;  // used to center the icons vertically
+        internal static readonly int k_LabelVerticalOffset = 6; // used to center the labels vertically
+        
+
         internal static readonly int k_IconSize = 28;
         internal static readonly int k_ListItemPading = 4;
         internal static readonly int k_ListItemStackPading = 20;

com.unity.render-pipelines.high-definition/Editor/Compositor/CompositionManagerEditor.cs

@@ -181,7 +181,6 @@ public override void OnInspectorGUI()
 
                 m_layerList.drawHeaderCallback = (Rect rect) =>
                 {
-                //EditorGUI.LabelField(rect, "Render Schedule", EditorStyles.largeLabel);
                 };
 
                 m_layerList.drawElementCallback = (Rect rect, int index, bool isActive, bool isFocused) =>

com.unity.render-pipelines.high-definition/Editor/Compositor/CompositionUtils.cs

@@ -79,7 +79,7 @@ static public void SetDefaultLayers(CompositionManager compositor)
             {
                 if (compositor.layers[i].outputTarget == CompositorLayer.OutputTarget.CompositorLayer)
                 {
-                    if ( (i+i < compositor.numLayers - 1) && (compositor.layers[i+1].outputTarget == CompositorLayer.OutputTarget.CameraStack))
+                    if ((i+i < compositor.numLayers - 1) && (compositor.layers[i+1].outputTarget == CompositorLayer.OutputTarget.CameraStack))
                     {
                         continue;
                     }
@@ -102,7 +102,7 @@ static public void LoadOrCreateCompositionProfileAsset(CompositionManager compos
 
                 newProfile = ScriptableObject.CreateInstance<CompositionProfile>();
 
-                // path = AssetDatabase.GenerateUniqueAssetPath(path);
+                //Note: no need to GenerateUniqueAssetPath(path), since we know that LoadAssetAtPath failed at this path
                 AssetDatabase.CreateAsset(newProfile, path);
                 AssetDatabase.SaveAssets();
                 AssetDatabase.Refresh();

com.unity.render-pipelines.high-definition/Runtime/Compositor/AdditionalCompositorData.cs

@@ -30,7 +30,7 @@ public void Reset()
             m_clearAlpha = true;
             m_imageFitMode = BackgroundFitMode.Stretch;
 
-            if (m_layerFilters !=null )
+            if (m_layerFilters !=null)
             {
                 m_layerFilters.Clear();
             }

com.unity.render-pipelines.high-definition/Runtime/Compositor/AlphaInjection.cs

@@ -5,7 +5,7 @@
 
 namespace UnityEngine.Rendering.HighDefinition.Compositor
 {
-    [Serializable, VolumeComponentMenu("Post-processing/Custom/AlphaIjection")]
+    [Serializable, VolumeComponentMenu("Post-processing/Custom/AlphaInjection")]
     internal sealed class AlphaInjection : CustomPostProcessVolumeComponent, IPostProcessComponent
     {
         internal class ShaderIDs