Fix display of nested/vlen compound and variable-length string array datasets

hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java

@@ -73,15 +73,9 @@ public static HDFDisplayConverter getDataDisplayConverter(final DataFormat dataO
 
         dataFormatReference = dataObject;
 
-        Datatype dtype = dataObject.getDatatype();
-
-        // For VLEN(compound), use compound base type so CompoundDataDisplayConverter is created
-        if (dtype.isVLEN() && !dtype.isVarStr() && dtype.getDatatypeBase() != null &&
-            dtype.getDatatypeBase().isCompound()) {
-            dtype = dtype.getDatatypeBase();
-        }
-
-        HDFDisplayConverter converter = getDataDisplayConverter(dtype);
+        // A top-level vlen-of-compound is a single column; dispatch on the dataset's own
+        // datatype so getDataDisplayConverter(VLEN) makes a VlenDataDisplayConverter.
+        HDFDisplayConverter converter = getDataDisplayConverter(dataObject.getDatatype());
 
         return converter;
     }
@@ -237,7 +231,7 @@ private static class CompoundDataDisplayConverter extends HDFDisplayConverter {
             CompoundDataFormat compoundFormat = (CompoundDataFormat)dataFormatReference;
 
             List<Datatype> localSelectedTypes =
-                DataFactoryUtils.filterNonSelectedMembers(compoundFormat, dtype);
+                DataFactoryUtils.filterNonSelectedMembers(compoundFormat, dtype, false);
 
             log.trace("setting up {} base HDFDisplayConverters", localSelectedTypes.size());
 
@@ -331,10 +325,16 @@ public Object canonicalToDisplayValue(Object value)
                         Object curObject = cmpdList.get(i);
                         if (curObject instanceof List)
                             buffer.append(memberTypeConverters[i].canonicalToDisplayValue(curObject));
-                        else {
+                        else if (curObject != null && curObject.getClass().isArray()) {
+                            // Array-of-compound: the member is a column-array indexed by row.
                             Object dataArrayValue = Array.get(curObject, cellRowIdx);
                             buffer.append(memberTypeConverters[i].canonicalToDisplayValue(dataArrayValue));
                         }
+                        else {
+                            // A single compound element (e.g. one element of a vlen-of-compound):
+                            // the member is already a scalar value, not a per-row column.
+                            buffer.append(memberTypeConverters[i].canonicalToDisplayValue(curObject));
+                        }
                     }
                     buffer.append("}");
                 }
@@ -610,7 +610,14 @@ public Object canonicalToDisplayValue(Object value)
             try {
                 Object obj;
                 Object convertedValue;
-                int arrLen = Array.getLength(value);
+
+                // Column-expanded vlen cells hand us a single scalar; defer to base converter.
+                if (!value.getClass().isArray() && !(value instanceof List)) {
+                    buffer.append(baseTypeConverter.canonicalToDisplayValue(value));
+                    return buffer;
+                }
+
+                int arrLen = (value instanceof List) ? ((List<?>)value).size() : Array.getLength(value);
 
                 log.trace("canonicalToDisplayValue({}): array length={}", value, arrLen);
 
@@ -621,7 +628,7 @@ public Object canonicalToDisplayValue(Object value)
                     if (i > 0)
                         buffer.append(", ");
 
-                    obj = Array.get(value, i);
+                    obj = (value instanceof List) ? ((List<?>)value).get(i) : Array.get(value, i);
 
                     convertedValue = baseTypeConverter.canonicalToDisplayValue(obj);
 

hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java

@@ -47,6 +47,40 @@ public class DataFactoryUtils {
     /** the CMPD_START_IDX_MAP_INDEX value. */
     public static final int CMPD_START_IDX_MAP_INDEX = 1;
 
+    /**
+     * Number of flat leaf names a Datatype contributes to the flat-name list
+     * produced by H5Datatype.extractCompoundInfo. The rules:
+     *   compound        - sum of children's counts (no header entry)
+     *   array(compound) - 1 header + sum of inner compound's children's counts
+     *   array(atomic)   - 1
+     *   vlen(any)       - 1 (header only; vlen recursion is disabled)
+     *   atomic, varstr  - 1
+     */
+    public static int countLeafNames(Datatype t)
+    {
+        if (t == null)
+            return 1;
+        if (t.isCompound()) {
+            int sum                 = 0;
+            List<Datatype> children = t.getCompoundMemberTypes();
+            if (children != null)
+                for (Datatype child : children)
+                    sum += countLeafNames(child);
+            return sum;
+        }
+        if (t.isArray()) {
+            Datatype base = t.getDatatypeBase();
+            if (base != null && base.isCompound()) {
+                int sum = 1;
+                for (Datatype child : base.getCompoundMemberTypes())
+                    sum += countLeafNames(child);
+                return sum;
+            }
+            return 1;
+        }
+        return 1;
+    }
+
     /**
      * Given a CompoundDataFormat, as well as a compound datatype, removes the
      * non-selected datatypes from the List of datatypes inside the compound
@@ -62,34 +96,35 @@ public class DataFactoryUtils {
     public static List<Datatype> filterNonSelectedMembers(CompoundDataFormat dataFormat,
                                                           final Datatype compoundType)
     {
+        return filterNonSelectedMembers(dataFormat, compoundType, true);
+    }
+
+    /**
+     * Variant of {@link #filterNonSelectedMembers(CompoundDataFormat, Datatype)} that
+     * skips the selected-member filter for inner (non-top-level) compounds.
+     *
+     * The dataset's flat selected-member list only enumerates top-level leaves; inner
+     * compound members can't be individually deselected. Filtering an inner compound
+     * against that list would spuriously remove every non-compound member.
+     */
+    public static List<Datatype> filterNonSelectedMembers(CompoundDataFormat dataFormat,
+                                                          final Datatype compoundType, boolean isTopLevel)
+    {
+        List<Datatype> selectedTypes = new ArrayList<>(compoundType.getCompoundMemberTypes());
+        if (!isTopLevel)
+            return selectedTypes;
+
         List<Datatype> allSelectedTypes = Arrays.asList(dataFormat.getSelectedMemberTypes());
         if (allSelectedTypes == null) {
             log.debug("filterNonSelectedMembers(): selected compound member datatype list is null");
             return null;
         }
 
-        /*
-         * Make sure to make a copy of the compound datatype's member list, as we will
-         * make modifications to the list when members aren't selected.
-         */
-        List<Datatype> selectedTypes = new ArrayList<>(compoundType.getCompoundMemberTypes());
-
-        /*
-         * Among the datatypes within this compound type, only keep the ones that are
-         * actually selected in the dataset.
-         */
         Iterator<Datatype> localIt = selectedTypes.iterator();
         while (localIt.hasNext()) {
             Datatype curType = localIt.next();
-
-            /*
-             * Since the passed in allSelectedMembers list is a flattened out datatype
-             * structure, we want to leave the nested compound Datatypes inside our local
-             * list of datatypes.
-             */
             if (curType.isCompound())
                 continue;
-
             if (!allSelectedTypes.contains(curType))
                 localIt.remove();
         }
@@ -189,26 +224,35 @@ else if (base.isArray()) {
                 }
                 log.trace("buildColIdxToStartIdxMap(): arrSize after base={}", arrSize);
             }
+            // A vlen member is always a single column rendered by VlenDataProvider as the
+            // whole sequence (a brace-string, recursing for nested compounds). A top-level
+            // vlen-of-compound is already peeled to its inner compound at the dataset level,
+            // so any vlen reaching here is a member - never expanded into per-element columns.
 
             if (nestedCompoundType != null) {
-                List<Datatype> cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, nestedCompoundType);
+                List<Datatype> cmpdSelectedTypes =
+                    filterNonSelectedMembers(dataFormat, nestedCompoundType, false);
 
                 /*
-                 * For Array/Vlen of Compound types, we repeat the compound members n times,
-                 * where n is the number of array elements of variable-length elements.
-                 * Therefore, we repeat our mapping for these types n times.
+                 * For Array of Compound types, we repeat the compound members n times,
+                 * where n is the number of array elements. For a top-level
+                 * vlen-of-compound, arrSize is 1 (one column per inner member).
                  */
                 for (int j = 0; j < arrSize; j++) {
                     buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex,
                                              curProviderIndex, depth + 1);
                 }
             }
             else if (curType.isCompound()) {
-                List<Datatype> cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType);
+                List<Datatype> cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType, false);
 
                 buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, curProviderIndex,
                                          depth + 1);
             }
+            else if (curType.isVLEN() && !curType.isVarStr()) {
+                for (int j = 0; j < arrSize; j++)
+                    outMap.put(curMapIndex[0]++, curProviderIndex[0]);
+            }
             else
                 outMap.put(curMapIndex[0]++, curProviderIndex[0]);
 
@@ -297,9 +341,12 @@ else if (base.isArray()) {
                 }
                 log.trace("buildRelColIdxToStartIdxMap(): arrSize after base={}", arrSize);
             }
+            // Single column per vlen member (see buildColIdxToProviderMap). No per-element
+            // expansion; top-level vlen-of-compound is already peeled at the dataset level.
 
             if (nestedCompoundType != null) {
-                List<Datatype> cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, nestedCompoundType);
+                List<Datatype> cmpdSelectedTypes =
+                    filterNonSelectedMembers(dataFormat, nestedCompoundType, false);
 
                 /*
                  * For Array/Vlen of Compound types, we repeat the compound members n times,
@@ -318,11 +365,21 @@ else if (curType.isCompound()) {
                 if (depth == 0)
                     curStartIdx[0] = curMapIndex[0];
 
-                List<Datatype> cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType);
+                List<Datatype> cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType, false);
 
                 buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, curStartIdx,
                                             depth + 1);
             }
+            else if (curType.isVLEN() && !curType.isVarStr()) {
+                for (int j = 0; j < arrSize; j++) {
+                    if (depth == 0) {
+                        outMap.put(curMapIndex[0], curMapIndex[0]);
+                        curMapIndex[0]++;
+                    }
+                    else
+                        outMap.put(curMapIndex[0]++, curStartIdx[0]);
+                }
+            }
             else {
                 if (depth == 0) {
                     outMap.put(curMapIndex[0], curMapIndex[0]);
@@ -333,4 +390,42 @@ else if (curType.isCompound()) {
             }
         }
     }
+
+    /**
+     * Return true when the datatype tree contains a construct whose table-view
+     * write path is not symmetric with the (recently expanded) display path,
+     * so a single-cell edit cannot be reliably mapped back to storage.
+     */
+    public static boolean isUnsafeForWrite(Datatype dtype) { return isUnsafe(dtype, false); }
+
+    private static boolean isUnsafe(Datatype dtype, boolean insideCompound)
+    {
+        if (dtype == null)
+            return false;
+
+        if (dtype.isVLEN() && !dtype.isVarStr())
+            return true;
+
+        if (dtype.isArray()) {
+            Datatype base = dtype.getDatatypeBase();
+            if (base != null && (base.isCompound() || base.isArray() || (base.isVLEN() && !base.isVarStr())))
+                return true;
+            return insideCompound;
+        }
+
+        if (dtype.isCompound()) {
+            if (insideCompound)
+                return true;
+            List<Datatype> members = dtype.getCompoundMemberTypes();
+            if (members != null) {
+                for (Datatype m : members) {
+                    if (isUnsafe(m, true))
+                        return true;
+                }
+            }
+            return false;
+        }
+
+        return false;
+    }
 }