From feff3cf6d2b5565af0fb1e3ae14515f49466bf4e Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Wed, 20 May 2026 10:01:23 -0500 Subject: [PATCH 01/11] Fix display for nested/vlen cmpds --- .../DataDisplayConverterFactory.java | 13 +- .../hdf/view/TableView/DataFactoryUtils.java | 202 +++++++++++++++--- .../view/TableView/DataProviderFactory.java | 63 +++++- .../view/TableView/DataValidatorFactory.java | 2 +- .../TableView/DefaultCompoundDSTableView.java | 140 ++++++++++-- .../main/java/hdf/object/h5/H5CompoundDS.java | 11 + 6 files changed, 367 insertions(+), 64 deletions(-) diff --git a/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java index 50651015a..62a027583 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java @@ -237,7 +237,7 @@ private static class CompoundDataDisplayConverter extends HDFDisplayConverter { CompoundDataFormat compoundFormat = (CompoundDataFormat)dataFormatReference; List localSelectedTypes = - DataFactoryUtils.filterNonSelectedMembers(compoundFormat, dtype); + DataFactoryUtils.filterNonSelectedMembers(compoundFormat, dtype, false); log.trace("setting up {} base HDFDisplayConverters", localSelectedTypes.size()); @@ -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); diff --git a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java index 197b6154d..7797ef05a 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java @@ -13,11 +13,14 @@ package hdf.view.TableView; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import hdf.object.CompoundDataFormat; import hdf.object.Datatype; @@ -47,6 +50,96 @@ 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 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; + } + + /** + * Column-expansion width for a vlen member, looked up by Datatype identity. + * Returns 1 for non-vlen types, varstr, or types not present in the map. + */ + public static int getVlenExpansion(Map vlenMaxLens, Datatype dtype) + { + if (dtype == null || !dtype.isVLEN() || dtype.isVarStr() || vlenMaxLens == null) + return 1; + Integer n = vlenMaxLens.get(dtype); + return (n == null || n.intValue() < 1) ? 1 : n.intValue(); + } + + /** + * Walk just-read compound data and return the max vlen length per vlen member. + * Identity-keyed so structurally identical Datatypes at different positions can + * carry different widths. + */ + public static IdentityHashMap + computeVlenMaxLens(List selectedTypes, Object dataValue) + { + IdentityHashMap out = new IdentityHashMap<>(); + if (!(dataValue instanceof List) || selectedTypes == null) + return out; + + List cols = (List)dataValue; + for (int i = 0; i < selectedTypes.size() && i < cols.size(); i++) { + Datatype t = selectedTypes.get(i); + if (t == null || !t.isVLEN() || t.isVarStr()) + continue; + + Object col = cols.get(i); + int max = 0; + if (col instanceof ArrayList[]) { + for (ArrayList row : (ArrayList[])col) { + if (row != null && row.size() > max) + max = row.size(); + } + } + else if (col != null && col.getClass().isArray()) { + int len = Array.getLength(col); + for (int j = 0; j < len; j++) { + Object row = Array.get(col, j); + if (row instanceof ArrayList al && al.size() > max) + max = al.size(); + else if (row != null && row.getClass().isArray()) { + int sz = Array.getLength(row); + if (sz > max) + max = sz; + } + } + } + out.put(t, Integer.valueOf(max)); + } + return out; + } + /** * Given a CompoundDataFormat, as well as a compound datatype, removes the * non-selected datatypes from the List of datatypes inside the compound @@ -62,34 +155,36 @@ public class DataFactoryUtils { public static List 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 filterNonSelectedMembers(CompoundDataFormat dataFormat, + final Datatype compoundType, + boolean isTopLevel) + { + List selectedTypes = new ArrayList<>(compoundType.getCompoundMemberTypes()); + if (!isTopLevel) + return selectedTypes; + List 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 selectedTypes = new ArrayList<>(compoundType.getCompoundMemberTypes()); - - /* - * Among the datatypes within this compound type, only keep the ones that are - * actually selected in the dataset. - */ Iterator 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(); } @@ -114,14 +209,23 @@ public static HashMap[] buildIndexMaps(CompoundDataFormat data List localSelectedTypes) throws Exception { + Map vlenMaxLens; + try { + vlenMaxLens = computeVlenMaxLens(localSelectedTypes, dataFormat.getData()); + } + catch (Exception ex) { + log.debug("buildIndexMaps(): vlen length scan failed, defaulting to 1 per vlen: ", ex); + vlenMaxLens = new IdentityHashMap<>(); + } + HashMap[] maps = new HashMap[2]; maps[COL_TO_BASE_CLASS_MAP_INDEX] = new HashMap<>(); maps[CMPD_START_IDX_MAP_INDEX] = new HashMap<>(); buildColIdxToProviderMap(maps[COL_TO_BASE_CLASS_MAP_INDEX], dataFormat, localSelectedTypes, - new int[] {0}, new int[] {0}, 0); + vlenMaxLens, new int[] {0}, new int[] {0}, 0); buildRelColIdxToStartIdxMap(maps[CMPD_START_IDX_MAP_INDEX], dataFormat, localSelectedTypes, - new int[] {0}, new int[] {0}, 0); + vlenMaxLens, new int[] {0}, new int[] {0}, 0); return maps; } @@ -153,7 +257,8 @@ public static HashMap[] buildIndexMaps(CompoundDataFormat data */ private static void buildColIdxToProviderMap(HashMap outMap, CompoundDataFormat dataFormat, - List localSelectedTypes, int[] curMapIndex, + List localSelectedTypes, + Map vlenMaxLens, int[] curMapIndex, int[] curProviderIndex, int depth) throws Exception { for (int i = 0; i < localSelectedTypes.size(); i++) { @@ -189,9 +294,18 @@ else if (base.isArray()) { } log.trace("buildColIdxToStartIdxMap(): arrSize after base={}", arrSize); } + else if (curType.isVLEN() && !curType.isVarStr()) { + // Only a top-level vlen-of-compound expands into its inner compound's + // leaves; inner vlens contribute a single column. + arrSize = getVlenExpansion(vlenMaxLens, curType); + Datatype base = curType.getDatatypeBase(); + if (depth == 0 && base != null && base.isCompound()) + nestedCompoundType = base; + } if (nestedCompoundType != null) { - List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, nestedCompoundType); + List cmpdSelectedTypes = + filterNonSelectedMembers(dataFormat, nestedCompoundType, false); /* * For Array/Vlen of Compound types, we repeat the compound members n times, @@ -199,15 +313,19 @@ else if (base.isArray()) { * Therefore, we repeat our mapping for these types n times. */ for (int j = 0; j < arrSize; j++) { - buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, - curProviderIndex, depth + 1); + buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, + curMapIndex, curProviderIndex, depth + 1); } } else if (curType.isCompound()) { - List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType); + List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType, false); - buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, curProviderIndex, - depth + 1); + buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, 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]); @@ -261,7 +379,8 @@ else if (curType.isCompound()) { */ private static void buildRelColIdxToStartIdxMap(HashMap outMap, CompoundDataFormat dataFormat, - List localSelectedTypes, int[] curMapIndex, + List localSelectedTypes, + Map vlenMaxLens, int[] curMapIndex, int[] curStartIdx, int depth) throws Exception { for (int i = 0; i < localSelectedTypes.size(); i++) { @@ -297,9 +416,16 @@ else if (base.isArray()) { } log.trace("buildRelColIdxToStartIdxMap(): arrSize after base={}", arrSize); } + else if (curType.isVLEN() && !curType.isVarStr()) { + arrSize = getVlenExpansion(vlenMaxLens, curType); + Datatype base = curType.getDatatypeBase(); + if (depth == 0 && base != null && base.isCompound()) + nestedCompoundType = base; + } if (nestedCompoundType != null) { - List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, nestedCompoundType); + List cmpdSelectedTypes = + filterNonSelectedMembers(dataFormat, nestedCompoundType, false); /* * For Array/Vlen of Compound types, we repeat the compound members n times, @@ -310,18 +436,28 @@ else if (base.isArray()) { if (depth == 0) curStartIdx[0] = curMapIndex[0]; - buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, - curStartIdx, depth + 1); + buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, + curMapIndex, curStartIdx, depth + 1); } } else if (curType.isCompound()) { if (depth == 0) curStartIdx[0] = curMapIndex[0]; - List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType); + List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType, false); - buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, curStartIdx, - depth + 1); + buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, 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) { diff --git a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java index 2d515da3f..b58147255 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java @@ -329,8 +329,11 @@ public Object getDataValue(Object obj, int index) try { if (obj instanceof ArrayList) theValue = ((ArrayList)obj).get(index); - else + else if (obj != null && obj.getClass().isArray()) theValue = Array.get(obj, index); + else + // Scalar input: row dimension already resolved by the caller. + theValue = obj; } catch (Exception ex) { log.debug("getDataValue({}): failure: ", index, ex); @@ -620,7 +623,7 @@ private static class CompoundDataProvider extends HDFDataProvider { selectedMemberOrders = compoundFormat.getSelectedMemberOrders(); List localSelectedTypes = - DataFactoryUtils.filterNonSelectedMembers(compoundFormat, dtype); + DataFactoryUtils.filterNonSelectedMembers(compoundFormat, dtype, false); log.trace("setting up {} base HDFDataProviders", localSelectedTypes.size()); @@ -794,6 +797,21 @@ else if (base instanceof ComplexDataProvider) { "CompoundDataProvider.getDataValue: theValue={}, rowIdx={}, adjustedColIndex={}", theValue, rowIdx, adjustedColIndex); } + else if (base instanceof VlenDataProvider) { + // Walk back to the first column mapped to this vlen provider so the + // offset within its column block is the cell's element index. + int vlenStartIdx = columnIndex; + while (vlenStartIdx > 0) { + Integer prevProviderIdx = baseProviderIndexMap.get(vlenStartIdx - 1); + if (prevProviderIdx == null || baseTypeProviders[prevProviderIdx] != base) + break; + vlenStartIdx--; + } + theValue = + ((VlenDataProvider)base).getElementValue(colValue, rowIdx, columnIndex - vlenStartIdx); + if (theValue == null) + theValue = ""; + } else { log.trace( "CompoundDataProvider.getDataValue: Non-container: Calling base.getDataValue(colValue, {})", @@ -1357,6 +1375,8 @@ private static class VlenDataProvider extends HDFDataProvider { private static final Logger log = LoggerFactory.getLogger(VlenDataProvider.class); private final HDFDataProvider baseTypeDataProvider; + private final Datatype baseType; + private final int numInnerLeaves; private final StringBuilder buffer; @@ -1367,10 +1387,10 @@ private static class VlenDataProvider extends HDFDataProvider { { super(dtype, dataBuf, dataTransposed); - Datatype baseType = dtype.getDatatypeBase(); - baseTypeClass = baseType.getDatatypeClass(); - + baseType = dtype.getDatatypeBase(); + baseTypeClass = baseType.getDatatypeClass(); baseTypeDataProvider = getDataProvider(baseType, dataBuf, dataTransposed); + numInnerLeaves = DataFactoryUtils.countLeafNames(baseType); buffer = new StringBuilder(); } @@ -1560,6 +1580,39 @@ private Object[] retrieveArrayOfAtomicElements(Object objBuf, int rowStartIdx) return tempArray; } + /** + * Return one cell value from the row's vlen, given the cell's offset within + * the vlen's column block. The block is laid out element-major: + * {@code [elem0_leaf0, elem0_leaf1, ..., elem1_leaf0, ...]}. Returns null + * when the offset falls past the row's actual vlen length. + */ + Object getElementValue(Object objBuf, int rowIdx, int elementIndex) + { + try { + ArrayList vlElements = ((ArrayList[])objBuf)[rowIdx]; + + if (baseTypeDataProvider instanceof CompoundDataProvider && numInnerLeaves > 0) { + int vlenElemIdx = elementIndex / numInnerLeaves; + int leafIdx = elementIndex % numInnerLeaves; + if (vlenElemIdx < 0 || vlenElemIdx >= vlElements.size()) + return null; + return baseTypeDataProvider.getDataValue(vlElements.get(vlenElemIdx), leafIdx, 0); + } + + if (elementIndex < 0 || elementIndex >= vlElements.size()) + return null; + + Object elem = vlElements.get(elementIndex); + if (elem instanceof byte[]) + return baseTypeDataProvider.getDataValue(elem, 0); + return elem; + } + catch (Exception ex) { + log.debug("getElementValue(rowIdx={}, elementIndex={}): failure: ", rowIdx, elementIndex, ex); + return DataFactoryUtils.errStr; + } + } + @Override public Object getDataValue(Object obj, int index) { diff --git a/hdfview/src/main/java/hdf/view/TableView/DataValidatorFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataValidatorFactory.java index 21ba4ff92..0046b4afd 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataValidatorFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataValidatorFactory.java @@ -244,7 +244,7 @@ private static class CompoundDataValidator extends HDFDataValidator { CompoundDataFormat compoundFormat = (CompoundDataFormat)dataFormatReference; List localSelectedTypes = - DataFactoryUtils.filterNonSelectedMembers(compoundFormat, dtype); + DataFactoryUtils.filterNonSelectedMembers(compoundFormat, dtype, false); log.trace("setting up {} base HDFDataValidators", localSelectedTypes.size()); diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java index cdfe7bf35..d454d4c16 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java @@ -22,6 +22,7 @@ import java.util.Iterator; import java.util.List; import java.util.ListIterator; +import java.util.Map; import java.util.StringTokenizer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -831,6 +832,8 @@ private class CompoundDSColumnHeaderDataProvider implements IDataProvider { // actually label the columns. private final ArrayList columnNames; + private final Map vlenMaxLens; + private final int ncols; private final int groupSize; @@ -849,6 +852,8 @@ private class CompoundDSColumnHeaderDataProvider implements IDataProvider { List selectedTypes = DataFactoryUtils.filterNonSelectedMembers(dataFormat, cmpdType); final List datasetMemberNames = Arrays.asList(dataFormat.getSelectedMemberNames()); + vlenMaxLens = DataFactoryUtils.computeVlenMaxLens(selectedTypes, dataValue); + columnNames = new ArrayList<>(dataFormat.getSelectedMemberCount()); recursiveColumnHeaderSetup(columnNames, dataFormat, cmpdType, datasetMemberNames, selectedTypes); @@ -911,7 +916,8 @@ else if (nestedCompoundType.isArray()) { * architectural issue. */ if (memberTypes.isEmpty()) { - memberTypes = DataFactoryUtils.filterNonSelectedMembers(dataFormat, nestedCompoundType); + memberTypes = + DataFactoryUtils.filterNonSelectedMembers(dataFormat, nestedCompoundType, false); } /* @@ -945,25 +951,56 @@ else if (nestedCompoundType.isArray()) { memberTypes); } else if (curDtype.isVLEN() && !curDtype.isVarStr()) { - /* - * For VLEN of COMPOUND, peel off the VLEN wrapper and recurse with the - * compound base type. Each cell displays the variable-length values as a - * brace-enclosed list, so no column multiplication is needed. - */ + // VLEN of X: expand to max_vlen columns with [i] suffix, mirroring the ARRAY branch. + int vlenLen = DataFactoryUtils.getVlenExpansion(vlenMaxLens, curDtype); Datatype baseType = curDtype.getDatatypeBase(); + if (baseType != null && baseType.isCompound()) { - if (memberTypes.isEmpty()) { - memberTypes = DataFactoryUtils.filterNonSelectedMembers(dataFormat, baseType); + if (memberTypes.isEmpty()) + memberTypes = DataFactoryUtils.filterNonSelectedMembers(dataFormat, baseType, false); + + StringBuilder sBuilder = new StringBuilder(); + ArrayList nestedMemberNames = new ArrayList<>(vlenLen * memberNames.size()); + for (int i = 0; i < vlenLen; i++) { + for (int j = 0; j < memberNames.size(); j++) { + sBuilder.setLength(0); + sBuilder.append(memberNames.get(j).replaceAll(CompoundDS.SEPARATOR, "->")); + sBuilder.append("[").append(i).append("]"); + nestedMemberNames.add(sBuilder.toString()); + } + } + + recursiveColumnHeaderSetup(outColNames, dataFormat, baseType, nestedMemberNames, + memberTypes); + } + else { + StringBuilder sBuilder = new StringBuilder(); + for (int j = 0; j < memberNames.size(); j++) { + for (int i = 0; i < vlenLen; i++) { + sBuilder.setLength(0); + sBuilder.append(memberNames.get(j).replaceAll(CompoundDS.SEPARATOR, "->")); + sBuilder.append("[").append(i).append("]"); + outColNames.add(sBuilder.toString()); + } } - recursiveColumnHeaderSetup(outColNames, dataFormat, baseType, memberNames, memberTypes); } } else if (curDtype.isCompound()) { + /* + * memberNames is the flat list of leaf names; memberTypes is the list of + * top-level member types. Advance through memberTypes one slot at a time, + * consuming countLeafNames(topType) flat names per slot. + */ ListIterator localIt = memberNames.listIterator(); + int topIdx = 0; + int remainingLeavesInTop = DataFactoryUtils.countLeafNames(memberTypes.get(0)); while (localIt.hasNext()) { - int curIdx = localIt.nextIndex(); + if (remainingLeavesInTop <= 0 && topIdx + 1 < memberTypes.size()) { + topIdx++; + remainingLeavesInTop = DataFactoryUtils.countLeafNames(memberTypes.get(topIdx)); + } String curName = localIt.next(); - Datatype curType = memberTypes.get(curIdx % memberTypes.size()); + Datatype curType = memberTypes.get(topIdx); Datatype nestedArrayOfCompoundType = null; boolean nestedArrayOfCompound = false; @@ -989,29 +1026,86 @@ else if (curDtype.isCompound()) { * members n times, where n is the number of array or vlen elements. */ if (nestedArrayOfCompound) { - List selTypes = - DataFactoryUtils.filterNonSelectedMembers(dataFormat, nestedArrayOfCompoundType); - List selMemberNames = new ArrayList<>(selTypes.size()); - - int arrCmpdLen = calcArrayOfCompoundLen(selTypes); - for (int i = 0; i < arrCmpdLen; i++) { - selMemberNames.add(localIt.next()); + List selTypes = DataFactoryUtils.filterNonSelectedMembers( + dataFormat, nestedArrayOfCompoundType, false); + + List selMemberNames; + int namesConsumed; + if (curType.isVLEN()) { + // Vlen wraps the flat name list at the header level only; + // synthesize the inner-compound leaf names from the declared + // member-name list, using the header as prefix. + String baseName = curName.replaceAll(CompoundDS.SEPARATOR, "->"); + selMemberNames = + buildInnerCompoundLeafNames(nestedArrayOfCompoundType, baseName); + namesConsumed = 1; + } + else { + // Array-of-compound: flat list has header + each inner leaf name. + selMemberNames = new ArrayList<>(selTypes.size()); + int arrCmpdLen = calcArrayOfCompoundLen(selTypes); + selMemberNames.add(curName); + for (int i = 1; i < arrCmpdLen; i++) + selMemberNames.add(localIt.next()); + namesConsumed = arrCmpdLen; } recursiveColumnHeaderSetup(outColNames, dataFormat, curType, selMemberNames, selTypes); + remainingLeavesInTop -= namesConsumed; + } + else if (curType.isVLEN() && !curType.isVarStr()) { + int vlenLen = DataFactoryUtils.getVlenExpansion(vlenMaxLens, curType); + String baseName = curName.replaceAll(CompoundDS.SEPARATOR, "->"); + for (int i = 0; i < vlenLen; i++) + outColNames.add(baseName + "[" + i + "]"); + remainingLeavesInTop--; } else { // Copy the dataset member name reference, so changes to the column name // don't affect the dataset's internal member names. curName = new String(curName.replaceAll(CompoundDS.SEPARATOR, "->")); - outColNames.add(curName); + remainingLeavesInTop--; } } } } + /** + * Produce "prefix->leaf" names for each leaf of {@code innerCompound}. + * Used by the vlen-of-compound header path, whose flat name list contains + * only the wrapper header rather than per-leaf entries. + */ + private List buildInnerCompoundLeafNames(Datatype innerCompound, String prefix) + { + List out = new ArrayList<>(); + appendInnerCompoundLeafNames(innerCompound, prefix, out); + return out; + } + + private void appendInnerCompoundLeafNames(Datatype t, String prefix, List out) + { + if (t == null) + return; + if (t.isCompound()) { + List names = t.getCompoundMemberNames(); + List children = t.getCompoundMemberTypes(); + if (names == null || children == null) + return; + for (int i = 0; i < names.size(); i++) { + String childName = prefix + "->" + names.get(i); + Datatype childT = children.get(i); + if (childT != null && childT.isCompound()) + appendInnerCompoundLeafNames(childT, childName, out); + else + out.add(childName); + } + return; + } + out.add(prefix); + } + private int calcArrayOfCompoundLen(List datatypes) { int count = 0; @@ -1157,10 +1251,12 @@ else if (groupTitleStartPosition > 0) { } else if (allColumnNames[i].matches(".*\\[[0-9]*\\]")) { /* - * Top-level ARRAY of COMPOUND types. + * Top-level array/vlen of an atomic base. Each element is a single + * column, so group them all under the member name rather than a + * generic per-element "ARRAY[i]" group. */ - columnHeaderBuilder.append("ARRAY"); - processArrayOfCompound(columnHeaderBuilder, allColumnNames[i]); + String baseName = allColumnNames[i].replaceAll("\\[[0-9]*\\]$", ""); + columnHeaderBuilder.append(baseName); columnGroupHeaderLayer.addColumnsIndexesToGroup(columnHeaderBuilder.toString(), colindex); diff --git a/object/src/main/java/hdf/object/h5/H5CompoundDS.java b/object/src/main/java/hdf/object/h5/H5CompoundDS.java index d8be2f4b5..8bebca9e1 100644 --- a/object/src/main/java/hdf/object/h5/H5CompoundDS.java +++ b/object/src/main/java/hdf/object/h5/H5CompoundDS.java @@ -1262,6 +1262,17 @@ else if (memberType.isVLEN() || H5.H5DreadVL(dsetID, compTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, (Object[])memberData); + + // H5DreadVL was called with a single-field compound transfer type, so + // each row comes back wrapped in a one-element record. Unwrap to expose + // the payload directly. + if (memberType.isVLEN()) { + Object[] rows = (Object[])memberData; + for (int r = 0; r < rows.length; r++) { + if (rows[r] instanceof java.util.ArrayList rec && rec.size() == 1) + rows[r] = rec.get(0); + } + } } else { log.trace( From b3093fec0812bc2356c87a69dd76530a0eaa56e6 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 20 May 2026 18:16:52 +0000 Subject: [PATCH 02/11] Committing clang-format changes --- .../java/hdf/view/TableView/DataFactoryUtils.java | 11 +++++------ .../java/hdf/view/TableView/DataProviderFactory.java | 4 ++-- .../view/TableView/DefaultCompoundDSTableView.java | 5 ++--- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java index 7797ef05a..c4072c3a0 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java @@ -101,8 +101,8 @@ public static int getVlenExpansion(Map vlenMaxLens, Datatype * Identity-keyed so structurally identical Datatypes at different positions can * carry different widths. */ - public static IdentityHashMap - computeVlenMaxLens(List selectedTypes, Object dataValue) + public static IdentityHashMap computeVlenMaxLens(List selectedTypes, + Object dataValue) { IdentityHashMap out = new IdentityHashMap<>(); if (!(dataValue instanceof List) || selectedTypes == null) @@ -167,8 +167,7 @@ public static List filterNonSelectedMembers(CompoundDataFormat dataFor * against that list would spuriously remove every non-compound member. */ public static List filterNonSelectedMembers(CompoundDataFormat dataFormat, - final Datatype compoundType, - boolean isTopLevel) + final Datatype compoundType, boolean isTopLevel) { List selectedTypes = new ArrayList<>(compoundType.getCompoundMemberTypes()); if (!isTopLevel) @@ -313,8 +312,8 @@ else if (curType.isVLEN() && !curType.isVarStr()) { * Therefore, we repeat our mapping for these types n times. */ for (int j = 0; j < arrSize; j++) { - buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, - curMapIndex, curProviderIndex, depth + 1); + buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, curMapIndex, + curProviderIndex, depth + 1); } } else if (curType.isCompound()) { diff --git a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java index b58147255..b67adcfc3 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java @@ -807,8 +807,8 @@ else if (base instanceof VlenDataProvider) { break; vlenStartIdx--; } - theValue = - ((VlenDataProvider)base).getElementValue(colValue, rowIdx, columnIndex - vlenStartIdx); + theValue = ((VlenDataProvider)base) + .getElementValue(colValue, rowIdx, columnIndex - vlenStartIdx); if (theValue == null) theValue = ""; } diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java index d454d4c16..5358fc8b0 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java @@ -1036,9 +1036,8 @@ else if (curDtype.isCompound()) { // synthesize the inner-compound leaf names from the declared // member-name list, using the header as prefix. String baseName = curName.replaceAll(CompoundDS.SEPARATOR, "->"); - selMemberNames = - buildInnerCompoundLeafNames(nestedArrayOfCompoundType, baseName); - namesConsumed = 1; + selMemberNames = buildInnerCompoundLeafNames(nestedArrayOfCompoundType, baseName); + namesConsumed = 1; } else { // Array-of-compound: flat list has header + each inner leaf name. From 699c5a6b4ac27e822bed1768ac35ece3ef9fccad Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Wed, 27 May 2026 10:54:56 -0500 Subject: [PATCH 03/11] Guard against unsafe writes --- .../hdf/view/TableView/DataFactoryUtils.java | 42 +++++++++++ .../view/TableView/DefaultBaseTableView.java | 74 +++++++++++++++++++ .../TableView/DefaultCompoundDSTableView.java | 43 ++++++----- 3 files changed, 141 insertions(+), 18 deletions(-) diff --git a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java index c4072c3a0..3690523bb 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java @@ -468,4 +468,46 @@ else if (curType.isVLEN() && !curType.isVarStr()) { } } } + + /** + * 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 members = dtype.getCompoundMemberTypes(); + if (members != null) { + for (Datatype m : members) { + if (isUnsafe(m, true)) + return true; + } + } + return false; + } + + return false; + } } diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java index 71e52eb8f..9846e05a0 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java @@ -105,6 +105,7 @@ import org.eclipse.nebula.widgets.nattable.style.DisplayMode; import org.eclipse.nebula.widgets.nattable.style.HorizontalAlignmentEnum; import org.eclipse.nebula.widgets.nattable.style.Style; +import org.eclipse.nebula.widgets.nattable.ui.action.IKeyAction; import org.eclipse.nebula.widgets.nattable.ui.action.IMouseAction; import org.eclipse.nebula.widgets.nattable.ui.binding.UiBindingRegistry; import org.eclipse.nebula.widgets.nattable.ui.matcher.CellEditorMouseEventMatcher; @@ -119,6 +120,7 @@ import org.eclipse.swt.custom.ScrolledComposite; import org.eclipse.swt.events.DisposeEvent; import org.eclipse.swt.events.DisposeListener; +import org.eclipse.swt.events.KeyEvent; import org.eclipse.swt.events.MouseEvent; import org.eclipse.swt.events.SelectionAdapter; import org.eclipse.swt.events.SelectionEvent; @@ -222,6 +224,14 @@ protected enum ViewType { /** status if file is read only. */ protected boolean isReadOnly = false; + /** + * True when the dataset's datatype contains a construct whose write path + * is not yet symmetric with the display path (see + * {@link DataFactoryUtils#isUnsafeForWrite}). Editing is force-disabled + * and a notice is shown when the user attempts to edit. + */ + protected boolean unsafeForWrite = false; + /** status if the enums are to display converted. */ protected boolean isEnumConverted = false; @@ -412,6 +422,13 @@ public void widgetDisposed(DisposeEvent e) } } + if (!isReadOnly && DataFactoryUtils.isUnsafeForWrite(dataObject.getDatatype())) { + unsafeForWrite = true; + isReadOnly = true; + log.info("dataObject({}) editing disabled: unsafe datatype for table-view write path", + dataObject); + } + log.trace("dataObject({}) isReadOnly={}", dataObject, isReadOnly); long[] dims = dataObject.getDims(); @@ -1278,6 +1295,33 @@ public void updateValueInFile() log.debug("updateValueInFile(): EXIT - value changed flag cleared"); } + /** + * Show the editing-disabled notice. Called when the user attempts to edit + * a cell in a dataset whose datatype has no symmetric write path (see + * {@link DataFactoryUtils#isUnsafeForWrite}). Throttled to one dialog per + * second so a held-down key or rapid clicks don't stack popups. + */ + private long lastUnsafeWriteNoticeMs = 0; + protected final void showUnsafeWriteNotice() + { + if (!unsafeForWrite || shell == null || shell.isDisposed()) + return; + long now = System.currentTimeMillis(); + if (now - lastUnsafeWriteNoticeMs < 1000) + return; + lastUnsafeWriteNoticeMs = now; + + Tools.showInformation( + shell, "Editing disabled", + "HDFView does not support editing datasets that contain any of the following:\n" + + " - variable-length sequences (non-string)\n" + + " - arrays of compound, array, or variable-length sequence\n" + + " - arrays nested inside compound\n" + + " - compounds nested inside compound\n\n" + + "Use a separate tool (h5py, the HDF5 C API, h5edit, etc.) to modify these " + + "datatypes."); + } + @Override public HObject getDataObject() { @@ -2546,6 +2590,36 @@ public void configureUiBindings(UiBindingRegistry uiBindingRegistry) new MouseEditAction()); } }); + + // When the datatype's write path is not yet implemented, intercept + // edit attempts and show a dialog explaining what's blocked. + if (unsafeForWrite) { + this.addConfiguration(new AbstractUiBindingConfiguration() { + @Override + public void configureUiBindings(UiBindingRegistry uiBindingRegistry) + { + uiBindingRegistry.registerFirstDoubleClickBinding( + new MouseEventMatcher(SWT.NONE, GridRegion.BODY, + MouseEventMatcher.LEFT_BUTTON), + new IMouseAction() { + @Override + public void run(NatTable table, MouseEvent event) + { + showUnsafeWriteNotice(); + } + }); + uiBindingRegistry.registerFirstKeyBinding(new LetterOrDigitKeyEventMatcher(), + new IKeyAction() { + @Override + public void run(NatTable table, + KeyEvent event) + { + showUnsafeWriteNotice(); + } + }); + } + }); + } } } } diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java index 5358fc8b0..0c6c2e2a3 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java @@ -27,24 +27,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import hdf.object.CompoundDS; -import hdf.object.CompoundDataFormat; -import hdf.object.DataFormat; -import hdf.object.Datatype; -import hdf.object.FileFormat; -import hdf.object.HObject; -import hdf.object.ScalarDS; -import hdf.object.h5.H5Datatype; -import hdf.view.DataView.DataViewManager; -import hdf.view.HDFView; -import hdf.view.Tools; -import hdf.view.ViewProperties; - -import hdf.hdf5lib.HDF5Constants; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.eclipse.nebula.widgets.nattable.NatTable; import org.eclipse.nebula.widgets.nattable.config.DefaultNatTableStyleConfiguration; import org.eclipse.nebula.widgets.nattable.config.EditableRule; @@ -71,6 +53,22 @@ import org.eclipse.swt.SWT; import org.eclipse.swt.custom.ScrolledComposite; import org.eclipse.swt.widgets.Composite; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import hdf.hdf5lib.HDF5Constants; +import hdf.object.CompoundDS; +import hdf.object.CompoundDataFormat; +import hdf.object.DataFormat; +import hdf.object.Datatype; +import hdf.object.FileFormat; +import hdf.object.HObject; +import hdf.object.ScalarDS; +import hdf.object.h5.H5Datatype; +import hdf.view.DataView.DataViewManager; +import hdf.view.HDFView; +import hdf.view.Tools; +import hdf.view.ViewProperties; /** * A class to construct a CompoundDS TableView. @@ -680,6 +678,15 @@ private class CompoundDSCellSelectionListener implements ILayerListener { public void handleLayerEvent(ILayerEvent e) { if (e instanceof CellSelectionEvent) { + // For datatypes where the display column count has been + // expanded (array-of-compound, vlen-of-compound, etc.) the + // listener below hits a null ptr because baseIndexMap was built from the + // unexpanded member count. Editing is already disabled for + // these via unsafeForWrite, and the listener's side effects + // (ref preview, etc.) don't apply, so just return. + if (unsafeForWrite) + return; + CellSelectionEvent event = (CellSelectionEvent)e; boolean valIsRegRef = false; boolean valIsObjRef = false; From 32e46030481622133f012e3236cfb8b243affe3d Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 16:03:09 +0000 Subject: [PATCH 04/11] Committing clang-format changes --- .../hdf/view/TableView/DataFactoryUtils.java | 8 ++--- .../view/TableView/DefaultBaseTableView.java | 35 +++++++++---------- .../TableView/DefaultCompoundDSTableView.java | 34 +++++++++--------- 3 files changed, 37 insertions(+), 40 deletions(-) diff --git a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java index 3690523bb..4fd3cc8e8 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java @@ -474,10 +474,7 @@ else if (curType.isVLEN() && !curType.isVarStr()) { * 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); - } + public static boolean isUnsafeForWrite(Datatype dtype) { return isUnsafe(dtype, false); } private static boolean isUnsafe(Datatype dtype, boolean insideCompound) { @@ -489,8 +486,7 @@ private static boolean isUnsafe(Datatype dtype, boolean insideCompound) if (dtype.isArray()) { Datatype base = dtype.getDatatypeBase(); - if (base != null && (base.isCompound() || base.isArray() - || (base.isVLEN() && !base.isVarStr()))) + if (base != null && (base.isCompound() || base.isArray() || (base.isVLEN() && !base.isVarStr()))) return true; return insideCompound; } diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java index 9846e05a0..e0501bf6a 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java @@ -1311,15 +1311,15 @@ protected final void showUnsafeWriteNotice() return; lastUnsafeWriteNoticeMs = now; - Tools.showInformation( - shell, "Editing disabled", - "HDFView does not support editing datasets that contain any of the following:\n" - + " - variable-length sequences (non-string)\n" - + " - arrays of compound, array, or variable-length sequence\n" - + " - arrays nested inside compound\n" - + " - compounds nested inside compound\n\n" - + "Use a separate tool (h5py, the HDF5 C API, h5edit, etc.) to modify these " - + "datatypes."); + Tools.showInformation(shell, "Editing disabled", + "HDFView does not support editing datasets that contain any of the following:\n" + + " - variable-length sequences (non-string)\n" + + " - arrays of compound, array, or variable-length sequence\n" + + " - arrays nested inside compound\n" + + " - compounds nested inside compound\n\n" + + + "Use a separate tool (h5py, the HDF5 C API, h5edit, etc.) to modify these " + + "datatypes."); } @Override @@ -2608,15 +2608,14 @@ public void run(NatTable table, MouseEvent event) showUnsafeWriteNotice(); } }); - uiBindingRegistry.registerFirstKeyBinding(new LetterOrDigitKeyEventMatcher(), - new IKeyAction() { - @Override - public void run(NatTable table, - KeyEvent event) - { - showUnsafeWriteNotice(); - } - }); + uiBindingRegistry.registerFirstKeyBinding( + new LetterOrDigitKeyEventMatcher(), new IKeyAction() { + @Override + public void run(NatTable table, KeyEvent event) + { + showUnsafeWriteNotice(); + } + }); } }); } diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java index 0c6c2e2a3..754b4226d 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java @@ -27,6 +27,24 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import hdf.object.CompoundDS; +import hdf.object.CompoundDataFormat; +import hdf.object.DataFormat; +import hdf.object.Datatype; +import hdf.object.FileFormat; +import hdf.object.HObject; +import hdf.object.ScalarDS; +import hdf.object.h5.H5Datatype; +import hdf.view.DataView.DataViewManager; +import hdf.view.HDFView; +import hdf.view.Tools; +import hdf.view.ViewProperties; + +import hdf.hdf5lib.HDF5Constants; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import org.eclipse.nebula.widgets.nattable.NatTable; import org.eclipse.nebula.widgets.nattable.config.DefaultNatTableStyleConfiguration; import org.eclipse.nebula.widgets.nattable.config.EditableRule; @@ -53,22 +71,6 @@ import org.eclipse.swt.SWT; import org.eclipse.swt.custom.ScrolledComposite; import org.eclipse.swt.widgets.Composite; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import hdf.hdf5lib.HDF5Constants; -import hdf.object.CompoundDS; -import hdf.object.CompoundDataFormat; -import hdf.object.DataFormat; -import hdf.object.Datatype; -import hdf.object.FileFormat; -import hdf.object.HObject; -import hdf.object.ScalarDS; -import hdf.object.h5.H5Datatype; -import hdf.view.DataView.DataViewManager; -import hdf.view.HDFView; -import hdf.view.Tools; -import hdf.view.ViewProperties; /** * A class to construct a CompoundDS TableView. From 0848e32e942db928265ab1a5157fd0241dae5dcb Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Fri, 29 May 2026 11:17:20 -0500 Subject: [PATCH 05/11] Fix array of varstr read --- object/src/main/java/hdf/object/h5/H5ScalarDS.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/object/src/main/java/hdf/object/h5/H5ScalarDS.java b/object/src/main/java/hdf/object/h5/H5ScalarDS.java index f36a0b508..c6deb407b 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarDS.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarDS.java @@ -974,7 +974,9 @@ else if ((originalBuf == null) || dsDatatype.isEnum() || dsDatatype.isText() || tid = dsDatatype.createNative(); log.trace("scalarDatasetCommonIO(): native type created tid={}", tid); - if (dsDatatype.isVarStr()) { + if (dsDatatype.isVarStr() || + (dsDatatype.isArray() && dsDatatype.getDatatypeBase() != null && + dsDatatype.getDatatypeBase().isVarStr())) { log.trace( "scalarDatasetCommonIO(): H5Dread_VLStrings did={} tid={} spaceIDs[0]={} spaceIDs[1]={}", did, tid, @@ -1125,7 +1127,9 @@ else if (dsDatatype.isFloat() && dsDatatype.getDatatypeSize() == 16) { try { tid = dsDatatype.createNative(); - if (dsDatatype.isVarStr()) { + if (dsDatatype.isVarStr() || + (dsDatatype.isArray() && dsDatatype.getDatatypeBase() != null && + dsDatatype.getDatatypeBase().isVarStr())) { log.trace( "scalarDatasetCommonIO(): H5Dwrite_VLStrings did={} tid={} spaceIDs[0]={} spaceIDs[1]={}", did, tid, From 211146e897c7e8db276ee5fcf290aa55b1ebcabe Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Thu, 4 Jun 2026 13:33:47 -0500 Subject: [PATCH 06/11] Remove support for vlen pre-allocation --- object/src/main/java/hdf/object/h5/H5CompoundDS.java | 4 ++-- object/src/main/java/hdf/object/h5/H5Datatype.java | 7 +++---- object/src/main/java/hdf/object/h5/H5ScalarAttr.java | 4 ++-- object/src/main/java/hdf/object/h5/H5ScalarDS.java | 4 ++-- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/object/src/main/java/hdf/object/h5/H5CompoundDS.java b/object/src/main/java/hdf/object/h5/H5CompoundDS.java index 8bebca9e1..f2be8e1a2 100644 --- a/object/src/main/java/hdf/object/h5/H5CompoundDS.java +++ b/object/src/main/java/hdf/object/h5/H5CompoundDS.java @@ -1214,10 +1214,10 @@ private Object readSingleCompoundMember(long dsetID, long[] spaceIDs, int nSelPo (spaceIDs[0] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[0], (spaceIDs[1] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[1]); + // Slots are left null: H5DreadVL installs a freshly + // allocated ArrayList into each slot. @SuppressWarnings("rawtypes") ArrayList[] vlBuf = new ArrayList[nSelPoints]; - for (int j = 0; j < nSelPoints; j++) - vlBuf[j] = new ArrayList<>(); H5.H5DreadVL(dsetID, compTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, vlBuf); diff --git a/object/src/main/java/hdf/object/h5/H5Datatype.java b/object/src/main/java/hdf/object/h5/H5Datatype.java index 306ffba13..bb6f665ef 100644 --- a/object/src/main/java/hdf/object/h5/H5Datatype.java +++ b/object/src/main/java/hdf/object/h5/H5Datatype.java @@ -2166,11 +2166,10 @@ else if ((typeClass == CLASS_STRING) || (typeClass == CLASS_REFERENCE)) { else if (dtype.isVLEN()) { log.trace("allocateArray(): isVLEN"); + // Slots are left null: the JNI read routines (H5DreadVL/H5AreadVL) + // install a freshly allocated ArrayList into each slot. Callers must + // not pre-allocate the per-element lists. data = new ArrayList[numPoints]; - for (int j = 0; j < numPoints; j++) - ((ArrayList[])data)[j] = new ArrayList(); - // if (baseType != null) - // ((ArrayList<>)data).add(H5Datatype.allocateArray(baseType, numPoints)); } else if (typeClass == CLASS_ARRAY) { log.trace("allocateArray(): class CLASS_ARRAY"); diff --git a/object/src/main/java/hdf/object/h5/H5ScalarAttr.java b/object/src/main/java/hdf/object/h5/H5ScalarAttr.java index 791d2c86b..96dcad4fd 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarAttr.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarAttr.java @@ -1210,9 +1210,9 @@ else if (dsDatatype.isCompound()) { else if (dsDatatype.isVLEN()) { log.trace("attributeCommonIO():read ioType:VLEN-REF H5Aread isArray()={}", dsDatatype.isArray()); + // Slots are left null: H5AreadVL installs a freshly + // allocated ArrayList into each slot. theData = new ArrayList[(int)lsize]; - for (int j = 0; j < lsize; j++) - ((ArrayList[])theData)[j] = new ArrayList(); try { H5.H5AreadVL(attrID, tid, (Object[])theData); diff --git a/object/src/main/java/hdf/object/h5/H5ScalarDS.java b/object/src/main/java/hdf/object/h5/H5ScalarDS.java index c6deb407b..4af594577 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarDS.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarDS.java @@ -940,9 +940,9 @@ private Object scalarDatasetCommonIO(H5File.IO_TYPE ioType, Object writeBuf) thr } } else if (dsDatatype.isVLEN()) { + // Slots are left null: H5DreadVL installs a freshly + // allocated ArrayList into each slot. theData = new ArrayList[(int)totalSelectedSpacePoints]; - for (int j = 0; j < (int)totalSelectedSpacePoints; j++) - ((ArrayList[])theData)[j] = new ArrayList(); } else if ((originalBuf == null) || dsDatatype.isEnum() || dsDatatype.isText() || dsDatatype.isRefObj() || From 64ccc599f31a7057998c10a8291eadafbadd0026 Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Thu, 18 Jun 2026 14:37:18 -0500 Subject: [PATCH 07/11] Display nested types in single cell --- .../DataDisplayConverterFactory.java | 20 +-- .../hdf/view/TableView/DataFactoryUtils.java | 114 +++--------------- .../view/TableView/DataProviderFactory.java | 54 +++------ .../TableView/DefaultCompoundDSTableView.java | 70 +++-------- .../main/java/hdf/object/h5/H5CompoundDS.java | 41 +++++-- .../main/java/hdf/object/h5/H5Datatype.java | 12 +- 6 files changed, 108 insertions(+), 203 deletions(-) diff --git a/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java index 62a027583..a338e085b 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java +++ b/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; } @@ -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("}"); } diff --git a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java index 4fd3cc8e8..7997c620d 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java @@ -13,14 +13,11 @@ package hdf.view.TableView; -import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; -import java.util.IdentityHashMap; import java.util.Iterator; import java.util.List; -import java.util.Map; import hdf.object.CompoundDataFormat; import hdf.object.Datatype; @@ -84,62 +81,6 @@ public static int countLeafNames(Datatype t) return 1; } - /** - * Column-expansion width for a vlen member, looked up by Datatype identity. - * Returns 1 for non-vlen types, varstr, or types not present in the map. - */ - public static int getVlenExpansion(Map vlenMaxLens, Datatype dtype) - { - if (dtype == null || !dtype.isVLEN() || dtype.isVarStr() || vlenMaxLens == null) - return 1; - Integer n = vlenMaxLens.get(dtype); - return (n == null || n.intValue() < 1) ? 1 : n.intValue(); - } - - /** - * Walk just-read compound data and return the max vlen length per vlen member. - * Identity-keyed so structurally identical Datatypes at different positions can - * carry different widths. - */ - public static IdentityHashMap computeVlenMaxLens(List selectedTypes, - Object dataValue) - { - IdentityHashMap out = new IdentityHashMap<>(); - if (!(dataValue instanceof List) || selectedTypes == null) - return out; - - List cols = (List)dataValue; - for (int i = 0; i < selectedTypes.size() && i < cols.size(); i++) { - Datatype t = selectedTypes.get(i); - if (t == null || !t.isVLEN() || t.isVarStr()) - continue; - - Object col = cols.get(i); - int max = 0; - if (col instanceof ArrayList[]) { - for (ArrayList row : (ArrayList[])col) { - if (row != null && row.size() > max) - max = row.size(); - } - } - else if (col != null && col.getClass().isArray()) { - int len = Array.getLength(col); - for (int j = 0; j < len; j++) { - Object row = Array.get(col, j); - if (row instanceof ArrayList al && al.size() > max) - max = al.size(); - else if (row != null && row.getClass().isArray()) { - int sz = Array.getLength(row); - if (sz > max) - max = sz; - } - } - } - out.put(t, Integer.valueOf(max)); - } - return out; - } - /** * Given a CompoundDataFormat, as well as a compound datatype, removes the * non-selected datatypes from the List of datatypes inside the compound @@ -208,23 +149,14 @@ public static HashMap[] buildIndexMaps(CompoundDataFormat data List localSelectedTypes) throws Exception { - Map vlenMaxLens; - try { - vlenMaxLens = computeVlenMaxLens(localSelectedTypes, dataFormat.getData()); - } - catch (Exception ex) { - log.debug("buildIndexMaps(): vlen length scan failed, defaulting to 1 per vlen: ", ex); - vlenMaxLens = new IdentityHashMap<>(); - } - HashMap[] maps = new HashMap[2]; maps[COL_TO_BASE_CLASS_MAP_INDEX] = new HashMap<>(); maps[CMPD_START_IDX_MAP_INDEX] = new HashMap<>(); buildColIdxToProviderMap(maps[COL_TO_BASE_CLASS_MAP_INDEX], dataFormat, localSelectedTypes, - vlenMaxLens, new int[] {0}, new int[] {0}, 0); + new int[] {0}, new int[] {0}, 0); buildRelColIdxToStartIdxMap(maps[CMPD_START_IDX_MAP_INDEX], dataFormat, localSelectedTypes, - vlenMaxLens, new int[] {0}, new int[] {0}, 0); + new int[] {0}, new int[] {0}, 0); return maps; } @@ -256,8 +188,7 @@ public static HashMap[] buildIndexMaps(CompoundDataFormat data */ private static void buildColIdxToProviderMap(HashMap outMap, CompoundDataFormat dataFormat, - List localSelectedTypes, - Map vlenMaxLens, int[] curMapIndex, + List localSelectedTypes, int[] curMapIndex, int[] curProviderIndex, int depth) throws Exception { for (int i = 0; i < localSelectedTypes.size(); i++) { @@ -293,33 +224,29 @@ else if (base.isArray()) { } log.trace("buildColIdxToStartIdxMap(): arrSize after base={}", arrSize); } - else if (curType.isVLEN() && !curType.isVarStr()) { - // Only a top-level vlen-of-compound expands into its inner compound's - // leaves; inner vlens contribute a single column. - arrSize = getVlenExpansion(vlenMaxLens, curType); - Datatype base = curType.getDatatypeBase(); - if (depth == 0 && base != null && base.isCompound()) - nestedCompoundType = base; - } + // 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 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, vlenMaxLens, curMapIndex, + buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, curProviderIndex, depth + 1); } } else if (curType.isCompound()) { List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType, false); - buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, curMapIndex, + buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, curProviderIndex, depth + 1); } else if (curType.isVLEN() && !curType.isVarStr()) { @@ -378,8 +305,7 @@ else if (curType.isVLEN() && !curType.isVarStr()) { */ private static void buildRelColIdxToStartIdxMap(HashMap outMap, CompoundDataFormat dataFormat, - List localSelectedTypes, - Map vlenMaxLens, int[] curMapIndex, + List localSelectedTypes, int[] curMapIndex, int[] curStartIdx, int depth) throws Exception { for (int i = 0; i < localSelectedTypes.size(); i++) { @@ -415,12 +341,8 @@ else if (base.isArray()) { } log.trace("buildRelColIdxToStartIdxMap(): arrSize after base={}", arrSize); } - else if (curType.isVLEN() && !curType.isVarStr()) { - arrSize = getVlenExpansion(vlenMaxLens, curType); - Datatype base = curType.getDatatypeBase(); - if (depth == 0 && base != null && base.isCompound()) - nestedCompoundType = base; - } + // 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 cmpdSelectedTypes = @@ -435,8 +357,8 @@ else if (curType.isVLEN() && !curType.isVarStr()) { if (depth == 0) curStartIdx[0] = curMapIndex[0]; - buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, - curMapIndex, curStartIdx, depth + 1); + buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, + curStartIdx, depth + 1); } } else if (curType.isCompound()) { @@ -445,7 +367,7 @@ else if (curType.isCompound()) { List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType, false); - buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, vlenMaxLens, curMapIndex, + buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, curStartIdx, depth + 1); } else if (curType.isVLEN() && !curType.isVarStr()) { diff --git a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java index b67adcfc3..be69d7a25 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java @@ -74,17 +74,11 @@ public static HDFDataProvider getDataProvider(final DataFormat dataObject, final dataFormatReference = dataObject; - Datatype dtype = dataObject.getDatatype(); - - // For VLEN(compound), use the compound base type so CompoundDataProvider is created. - // The VLEN aspect is handled in the read path (H5DreadVL), and each member's data - // is a String[] of brace-enclosed values. - if (dtype.isVLEN() && !dtype.isVarStr() && dtype.getDatatypeBase() != null && - dtype.getDatatypeBase().isCompound()) { - dtype = dtype.getDatatypeBase(); - } - - HDFDataProvider dataProvider = getDataProvider(dtype, dataBuf, dataTransposed); + // A top-level vlen-of-compound is a single column showing the whole sequence, so + // dispatch on the dataset's own datatype: getDataProvider(VLEN) makes a + // VlenDataProvider (read path supplies one bare ArrayList[] of nested-List elements). + HDFDataProvider dataProvider = + getDataProvider(dataObject.getDatatype(), dataBuf, dataTransposed); return dataProvider; } @@ -798,19 +792,11 @@ else if (base instanceof ComplexDataProvider) { theValue, rowIdx, adjustedColIndex); } else if (base instanceof VlenDataProvider) { - // Walk back to the first column mapped to this vlen provider so the - // offset within its column block is the cell's element index. - int vlenStartIdx = columnIndex; - while (vlenStartIdx > 0) { - Integer prevProviderIdx = baseProviderIndexMap.get(vlenStartIdx - 1); - if (prevProviderIdx == null || baseTypeProviders[prevProviderIdx] != base) - break; - vlenStartIdx--; - } - theValue = ((VlenDataProvider)base) - .getElementValue(colValue, rowIdx, columnIndex - vlenStartIdx); - if (theValue == null) - theValue = ""; + // A vlen member is a single column showing the whole sequence. Delegate + // to the provider's full-sequence rendering (an array of per-element + // values that the VlenDataDisplayConverter joins into a brace-string), + // rather than fetching one element by column offset. + theValue = base.getDataValue(colValue, fieldIdx, rowIdx); } else { log.trace( @@ -1496,20 +1482,16 @@ else if (baseTypeDataProvider instanceof ComplexDataProvider) { private Object[] retrieveArrayOfCompoundElements(Object objBuf, int columnIndex, int rowIndex) { - long vlSize = Array.getLength(objBuf); - log.trace("retrieveArrayOfCompoundElements(): vlSize={}", vlSize); - long adjustedRowIdx = - (rowIndex * vlSize * colCount) + - (columnIndex / ((CompoundDataProvider)baseTypeDataProvider).baseProviderIndexMap.size()); - long adjustedColIdx = - columnIndex % ((CompoundDataProvider)baseTypeDataProvider).baseProviderIndexMap.size(); - /* - * Since we flatten array of compound types, we only need to return a single - * value. + * A vlen-of-compound member is one column showing the whole sequence. The read + * path hands each row a list of compound elements already parsed into nested + * Lists (e.g. [[10, [11, 12]], [20, [21, 22]]]). Return the row's elements as an + * array; VlenDataDisplayConverter wraps them in [...] and the inner + * CompoundDataDisplayConverter renders each element as {...} (recursing for + * nested compounds). */ - return new Object[] { - baseTypeDataProvider.getDataValue(objBuf, (int)adjustedColIdx, (int)adjustedRowIdx)}; + ArrayList vlElements = ((ArrayList[])objBuf)[rowIndex]; + return vlElements.toArray(); } private Object[] retrieveArrayOfArrayElements(Object objBuf, int columnIndex, int startRowIndex) diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java index 754b4226d..b32433ff6 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java @@ -22,7 +22,6 @@ import java.util.Iterator; import java.util.List; import java.util.ListIterator; -import java.util.Map; import java.util.StringTokenizer; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -841,8 +840,6 @@ private class CompoundDSColumnHeaderDataProvider implements IDataProvider { // actually label the columns. private final ArrayList columnNames; - private final Map vlenMaxLens; - private final int ncols; private final int groupSize; @@ -852,17 +849,15 @@ private class CompoundDSColumnHeaderDataProvider implements IDataProvider { Datatype cmpdType = dataObject.getDatatype(); - // Resolve VLEN(compound) to the compound base type for display purposes - if (cmpdType.isVLEN() && !cmpdType.isVarStr() && cmpdType.getDatatypeBase() != null && - cmpdType.getDatatypeBase().isCompound()) { - cmpdType = cmpdType.getDatatypeBase(); - } - - List selectedTypes = DataFactoryUtils.filterNonSelectedMembers(dataFormat, cmpdType); + // A top-level vlen-of-compound is a single column (the whole sequence). It is not + // a compound, so it has no members to filter - use it directly as the sole type. + List selectedTypes; + if (cmpdType.isVLEN() && !cmpdType.isVarStr()) + selectedTypes = new ArrayList<>(java.util.Collections.singletonList(cmpdType)); + else + selectedTypes = DataFactoryUtils.filterNonSelectedMembers(dataFormat, cmpdType); final List datasetMemberNames = Arrays.asList(dataFormat.getSelectedMemberNames()); - vlenMaxLens = DataFactoryUtils.computeVlenMaxLens(selectedTypes, dataValue); - columnNames = new ArrayList<>(dataFormat.getSelectedMemberCount()); recursiveColumnHeaderSetup(columnNames, dataFormat, cmpdType, datasetMemberNames, selectedTypes); @@ -960,39 +955,11 @@ else if (nestedCompoundType.isArray()) { memberTypes); } else if (curDtype.isVLEN() && !curDtype.isVarStr()) { - // VLEN of X: expand to max_vlen columns with [i] suffix, mirroring the ARRAY branch. - int vlenLen = DataFactoryUtils.getVlenExpansion(vlenMaxLens, curDtype); - Datatype baseType = curDtype.getDatatypeBase(); - - if (baseType != null && baseType.isCompound()) { - if (memberTypes.isEmpty()) - memberTypes = DataFactoryUtils.filterNonSelectedMembers(dataFormat, baseType, false); - - StringBuilder sBuilder = new StringBuilder(); - ArrayList nestedMemberNames = new ArrayList<>(vlenLen * memberNames.size()); - for (int i = 0; i < vlenLen; i++) { - for (int j = 0; j < memberNames.size(); j++) { - sBuilder.setLength(0); - sBuilder.append(memberNames.get(j).replaceAll(CompoundDS.SEPARATOR, "->")); - sBuilder.append("[").append(i).append("]"); - nestedMemberNames.add(sBuilder.toString()); - } - } - - recursiveColumnHeaderSetup(outColNames, dataFormat, baseType, nestedMemberNames, - memberTypes); - } - else { - StringBuilder sBuilder = new StringBuilder(); - for (int j = 0; j < memberNames.size(); j++) { - for (int i = 0; i < vlenLen; i++) { - sBuilder.setLength(0); - sBuilder.append(memberNames.get(j).replaceAll(CompoundDS.SEPARATOR, "->")); - sBuilder.append("[").append(i).append("]"); - outColNames.add(sBuilder.toString()); - } - } - } + // A top-level vlen (including vlen-of-compound) is a single column showing + // the whole sequence. Never recurse into a compound base - that would create + // one column per inner member. + for (int j = 0; j < memberNames.size(); j++) + outColNames.add(memberNames.get(j).replaceAll(CompoundDS.SEPARATOR, "->")); } else if (curDtype.isCompound()) { /* @@ -1014,10 +981,11 @@ else if (curDtype.isCompound()) { boolean nestedArrayOfCompound = false; /* - * Recursively detect any nested array/vlen of compound types and deal with them - * by creating multiple copies of the member names. + * Recursively detect any nested ARRAY of compound types and deal with them + * by creating multiple copies of the member names. A vlen is NOT expanded - + * it is a single column (handled below), so it is excluded here. */ - if (curType.isArray() || curType.isVLEN()) { + if (curType.isArray()) { Datatype base = curType.getDatatypeBase(); while (base != null) { if (base.isCompound()) { @@ -1063,10 +1031,10 @@ else if (curDtype.isCompound()) { remainingLeavesInTop -= namesConsumed; } else if (curType.isVLEN() && !curType.isVarStr()) { - int vlenLen = DataFactoryUtils.getVlenExpansion(vlenMaxLens, curType); + // A vlen member is a single column showing the whole sequence as a + // brace-string (VlenDataProvider recurses for nested compounds). String baseName = curName.replaceAll(CompoundDS.SEPARATOR, "->"); - for (int i = 0; i < vlenLen; i++) - outColNames.add(baseName + "[" + i + "]"); + outColNames.add(baseName); remainingLeavesInTop--; } else { diff --git a/object/src/main/java/hdf/object/h5/H5CompoundDS.java b/object/src/main/java/hdf/object/h5/H5CompoundDS.java index f2be8e1a2..e9c349297 100644 --- a/object/src/main/java/hdf/object/h5/H5CompoundDS.java +++ b/object/src/main/java/hdf/object/h5/H5CompoundDS.java @@ -944,14 +944,41 @@ private Object compoundTypeIO(H5File.IO_TYPE ioType, long did, long[] spaceIDs, } else if (cmpdType.isVLEN() && !cmpdType.isVarStr()) { /* - * For VLEN of COMPOUND, peel off the VLEN wrapper and recurse with the compound - * base type. The actual VLEN reading is handled in readSingleCompoundMember(), - * which detects the VLEN wrapper via dsDatatype.isVLEN() and uses H5DreadVL. + * Top-level VLEN-of-compound is displayed as a SINGLE column showing the whole + * sequence (e.g. [{1, 2}, {3, 4}]), consistent with how a vlen member of a + * compound is shown. Read the whole vlen in one H5DreadVL call; the JNI parses + * each compound element into nested Lists (e.g. [[1, 2], [3, 4]]), exactly the + * shape VlenDataProvider/VlenDataDisplayConverter render. The dataset enumerates + * a single member (see H5Datatype.extractCompoundInfo), so we return one column. */ - Datatype baseType = cmpdType.getDatatypeBase(); - if (baseType != null && baseType.isCompound()) { - theData = compoundTypeIO(ioType, did, spaceIDs, nSelPoints, (H5Datatype)baseType, writeBuf, - globalMemberIndex); + if (ioType == H5File.IO_TYPE.READ) { + long wholeTid = -1; + try { + wholeTid = H5.H5Dget_type(did); + @SuppressWarnings("rawtypes") + ArrayList[] vlBuf = new ArrayList[nSelPoints]; + H5.H5DreadVL(did, wholeTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, vlBuf); + globalMemberIndex[0]++; + theData = vlBuf; + } + catch (HDF5DataFiltersException exfltr) { + log.debug("compoundTypeIO(): top-level VLEN read failure: ", exfltr); + throw new HDF5Exception("Filter not available exception: " + exfltr.getMessage()); + } + catch (Exception ex) { + log.debug("compoundTypeIO(): top-level VLEN read failure: ", ex); + throw new HDF5Exception("failed to read VLEN-of-compound dataset: " + ex.getMessage()); + } + finally { + if (wholeTid >= 0) { + try { + H5.H5Tclose(wholeTid); + } + catch (Exception ex) { + log.debug("compoundTypeIO(): H5Tclose(wholeTid {}) failure: ", wholeTid, ex); + } + } + } } } else if (cmpdType.isCompound()) { diff --git a/object/src/main/java/hdf/object/h5/H5Datatype.java b/object/src/main/java/hdf/object/h5/H5Datatype.java index bb6f665ef..ddd37689b 100644 --- a/object/src/main/java/hdf/object/h5/H5Datatype.java +++ b/object/src/main/java/hdf/object/h5/H5Datatype.java @@ -2974,9 +2974,15 @@ public static void extractCompoundInfo(final H5Datatype dtype, String name, List H5Datatype.extractCompoundInfo((H5Datatype)dtype.getDatatypeBase(), name, names, flatListTypes); } else if (dtype.isVLEN() && !dtype.isVarStr()) { - log.trace( - "extractCompoundInfo(): variable-length type - extracting compound info from base datatype"); - H5Datatype.extractCompoundInfo((H5Datatype)dtype.getDatatypeBase(), name, names, flatListTypes); + /* + * A vlen (including vlen-of-compound) is a single leaf: it is displayed as one + * column showing the whole sequence as a string. Do NOT recurse into the base + * compound - that would enumerate the inner members as separate columns. + */ + log.trace("extractCompoundInfo(): variable-length type - adding as a single leaf"); + if (names != null) + names.add(name); + flatListTypes.add(dtype); } else if (dtype.isCompound()) { List compoundMemberNames = dtype.getCompoundMemberNames(); From 3a3f0bb665fe6e6852253b8abb4ea1e402f2aa8c Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Tue, 15 Sep 2026 10:49:55 -0500 Subject: [PATCH 08/11] Read array and vlen data the way the JNI expects The JNI in HDF5 wants one slot per point, with each slot holding a list of that point's elements. That model came in with https://github.com/HDFGroup/hdf5/pull/2156. The slots are left for the read routines to populate (see https://github.com/HDFGroup/hdf5/pull/6413). Arrays containing variable-length data are now allocated as documented, and arrays of fixed-length data have the flat layout the plain read path fills. Detection of variable-length data now recurses the way the JNI's own does. I removed the special case that routed arrays of vlen strings through the VLStrings entry points, since those choose between two different return shapes at runtime and the ordinary path is correct once handed the right buffer. Providers now say whether a cell can be written before an editor opens, since an exception thrown while committing an edit escapes without the table reporting it, and leaves the editor stuck open. New tests cover the datatypes this work is about, several of which had none before. They build their files at test time under the build directory. One test is disabled for now. A vlen of fixed-length strings reads each element to the first null byte rather than for its declared width, so elements run into each other and past the end of the buffer. That seems to be in HDF5's JNI rather than here. --- .../DataDisplayConverterFactory.java | 15 +- .../hdf/view/TableView/DataFactoryUtils.java | 92 +-- .../view/TableView/DataProviderFactory.java | 215 +++++-- .../view/TableView/DefaultBaseTableView.java | 73 --- .../TableView/DefaultCompoundDSTableView.java | 92 +-- .../TableView/DefaultScalarDSTableView.java | 3 + .../view/TableView/DataFactoryUtilsTest.java | 214 +++++++ .../main/java/hdf/object/h5/H5CompoundDS.java | 96 +-- .../main/java/hdf/object/h5/H5Datatype.java | 76 ++- .../main/java/hdf/object/h5/H5ScalarAttr.java | 3 +- .../main/java/hdf/object/h5/H5ScalarDS.java | 17 +- .../java/object/TestNestedDatatypeShapes.java | 552 ++++++++++++++++++ .../java/object/TestVlenArrayDatatypes.java | 291 +++++++++ 13 files changed, 1405 insertions(+), 334 deletions(-) create mode 100644 hdfview/src/test/java/hdf/view/TableView/DataFactoryUtilsTest.java create mode 100644 object/src/test/java/object/TestNestedDatatypeShapes.java create mode 100644 object/src/test/java/object/TestVlenArrayDatatypes.java diff --git a/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java index a338e085b..f3139941c 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataDisplayConverterFactory.java @@ -73,8 +73,6 @@ public static HDFDisplayConverter getDataDisplayConverter(final DataFormat dataO dataFormatReference = dataObject; - // 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; @@ -326,13 +324,12 @@ public Object canonicalToDisplayValue(Object value) if (curObject instanceof List) buffer.append(memberTypeConverters[i].canonicalToDisplayValue(curObject)); else if (curObject != null && curObject.getClass().isArray()) { - // Array-of-compound: the member is a column-array indexed by row. + // Array-of-compound: the member is 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. + // A single compound element: the member is already the value. buffer.append(memberTypeConverters[i].canonicalToDisplayValue(curObject)); } } @@ -469,7 +466,9 @@ public Object canonicalToDisplayValue(Object value) try { Object obj; Object convertedValue; - int arrLen = Array.getLength(value); + + // An array's elements may arrive as a List, nested arrays included. + int arrLen = (value instanceof List) ? ((List)value).size() : Array.getLength(value); log.trace("canonicalToDisplayValue({}): array length={}", value, arrLen); @@ -480,7 +479,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); @@ -611,7 +610,7 @@ public Object canonicalToDisplayValue(Object value) Object obj; Object convertedValue; - // Column-expanded vlen cells hand us a single scalar; defer to base converter. + // A scalar cell defers to the base converter. if (!value.getClass().isArray() && !(value instanceof List)) { buffer.append(baseTypeConverter.canonicalToDisplayValue(value)); return buffer; diff --git a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java index 7997c620d..b57e50aec 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataFactoryUtils.java @@ -48,13 +48,9 @@ public class DataFactoryUtils { 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 + * Number of flat leaf names a Datatype contributes to the list produced by + * H5Datatype.extractCompoundInfo: a compound sums its children, an + * array-of-compound adds a header entry, anything else counts once. */ public static int countLeafNames(Datatype t) { @@ -100,12 +96,8 @@ public static List filterNonSelectedMembers(CompoundDataFormat dataFor } /** - * 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. + * As above, but an inner compound keeps every member: the dataset's selected-member + * list enumerates only top-level leaves, so filtering against it would drop them all. */ public static List filterNonSelectedMembers(CompoundDataFormat dataFormat, final Datatype compoundType, boolean isTopLevel) @@ -224,19 +216,14 @@ 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 cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, nestedCompoundType, false); /* - * 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 Array of Compound types, repeat the compound members once per + * array element. */ for (int j = 0; j < arrSize; j++) { buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, @@ -246,12 +233,12 @@ else if (base.isArray()) { else if (curType.isCompound()) { List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType, false); - buildColIdxToProviderMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, - curProviderIndex, depth + 1); + 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]); + // A vlen is one column: it holds the whole sequence. + outMap.put(curMapIndex[0]++, curProviderIndex[0]); } else outMap.put(curMapIndex[0]++, curProviderIndex[0]); @@ -341,8 +328,6 @@ 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 cmpdSelectedTypes = @@ -367,18 +352,17 @@ else if (curType.isCompound()) { List cmpdSelectedTypes = filterNonSelectedMembers(dataFormat, curType, false); - buildRelColIdxToStartIdxMap(outMap, dataFormat, cmpdSelectedTypes, curMapIndex, - curStartIdx, depth + 1); + 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]); + // A vlen is one column: it holds the whole sequence. + if (depth == 0) { + outMap.put(curMapIndex[0], curMapIndex[0]); + curMapIndex[0]++; } + else + outMap.put(curMapIndex[0]++, curStartIdx[0]); } else { if (depth == 0) { @@ -390,42 +374,4 @@ else if (curType.isVLEN() && !curType.isVarStr()) { } } } - - /** - * 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 members = dtype.getCompoundMemberTypes(); - if (members != null) { - for (Datatype m : members) { - if (isUnsafe(m, true)) - return true; - } - } - return false; - } - - return false; - } } diff --git a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java index be69d7a25..fbce3b6cc 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java @@ -74,11 +74,7 @@ public static HDFDataProvider getDataProvider(final DataFormat dataObject, final dataFormatReference = dataObject; - // A top-level vlen-of-compound is a single column showing the whole sequence, so - // dispatch on the dataset's own datatype: getDataProvider(VLEN) makes a - // VlenDataProvider (read path supplies one bare ArrayList[] of nested-List elements). - HDFDataProvider dataProvider = - getDataProvider(dataObject.getDatatype(), dataBuf, dataTransposed); + HDFDataProvider dataProvider = getDataProvider(dataObject.getDatatype(), dataBuf, dataTransposed); return dataProvider; } @@ -326,7 +322,7 @@ public Object getDataValue(Object obj, int index) else if (obj != null && obj.getClass().isArray()) theValue = Array.get(obj, index); else - // Scalar input: row dimension already resolved by the caller. + // The caller already resolved the row dimension. theValue = obj; } catch (Exception ex) { @@ -339,6 +335,20 @@ else if (obj != null && obj.getClass().isArray()) return theValue; } + /** + * Whether this cell can be written back. A provider that cannot map a displayed + * cell to storage returns false, so the table declines to open an editor rather + * than rejecting the value after the user has typed it. + * + * @param columnIndex + * the column + * @param rowIndex + * the row + * + * @return true when the cell can be edited + */ + public boolean isCellEditable(int columnIndex, int rowIndex) { return true; } + /** * update the data value of a compound type. * @@ -792,10 +802,7 @@ else if (base instanceof ComplexDataProvider) { theValue, rowIdx, adjustedColIndex); } else if (base instanceof VlenDataProvider) { - // A vlen member is a single column showing the whole sequence. Delegate - // to the provider's full-sequence rendering (an array of per-element - // values that the VlenDataDisplayConverter joins into a brace-string), - // rather than fetching one element by column offset. + // A vlen member is one column holding the whole sequence. theValue = base.getDataValue(colValue, fieldIdx, rowIdx); } else { @@ -862,6 +869,19 @@ public Object getDataValue(Object obj, int index) "getDataValue(Object, int) should not be called for CompoundDataProviders"); } + @Override + public boolean isCellEditable(int columnIndex, int rowIndex) + { + try { + int fieldIdx = columnIndex % getColumnCount(); + return baseTypeProviders[baseProviderIndexMap.get(fieldIdx)].isCellEditable(fieldIdx, + rowIndex); + } + catch (Exception ex) { + return true; + } + } + @Override public void setDataValue(int columnIndex, int rowIndex, Object newValue) { @@ -959,6 +979,9 @@ public void setDataValue(int columnIndex, int rowIndex, Object newValue) isValueChanged = true; log.trace("CompoundDataProvider.setDataValue: SUCCESS"); } + catch (UnsupportedOperationException uoe) { + throw uoe; + } catch (Exception ex) { log.debug("CompoundDataProvider.setDataValue({}, {})=({}): cell value update failure: ", rowIndex, columnIndex, newValue, ex); @@ -1026,6 +1049,9 @@ public void setDataValue(int columnIndex, int rowIndex, Object bufObject, Object isValueChanged = true; log.trace("=== COMPOUND setDataValue(bufObject) SUCCESS ==="); } + catch (UnsupportedOperationException uoe) { + throw uoe; + } catch (Exception ex) { log.trace("setDataValue({}, {}, {})=({}): cell value update failure: ", rowIndex, columnIndex, bufObject, newValue, ex); @@ -1063,6 +1089,9 @@ private static class ArrayDataProvider extends HDFDataProvider { private final int nCols; + /** Whether the array's base type is a variable-length string. */ + private final boolean isVarStrBase; + ArrayDataProvider(final Datatype dtype, final Object dataBuf, final boolean dataTransposed) throws Exception { @@ -1088,6 +1117,35 @@ else if (baseType.isBitField() || baseType.isOpaque()) nCols = (int)arraySize * ((CompoundDataProvider)baseTypeDataProvider).nCols; else nCols = super.getColumnCount(); + + isVarStrBase = baseType.isVarStr(); + } + + /** + * Return this array's elements for one selected point, or null when the buffer is + * a flat run of base-type values rather than a List per point. + */ + private Object[] retrieveObjectModelElements(Object objBuf, int pointIndex) + { + if (!(objBuf instanceof Object[] slots)) + return null; + if (pointIndex < 0 || pointIndex >= slots.length) + return null; + if (!(slots[pointIndex] instanceof List elements)) + return null; + + Object[] values = new Object[elements.size()]; + for (int i = 0; i < values.length; i++) { + Object element = elements.get(i); + + // Opaque, reference and bitfield elements arrive as raw bytes. + if (element instanceof byte[]) + values[i] = baseTypeDataProvider.getDataValue(element, 0); + else + values[i] = element; + } + + return values; } @Override @@ -1097,6 +1155,10 @@ public Object getDataValue(int columnIndex, int rowIndex) try { int bufIndex = physicalLocationToBufIndex(rowIndex, columnIndex); + Object[] objModelElements = retrieveObjectModelElements(dataBuf, bufIndex); + if (objModelElements != null) + return theValue = objModelElements; + bufIndex *= arraySize; if (baseTypeDataProvider instanceof CompoundDataProvider) { @@ -1146,6 +1208,10 @@ public Object getDataValue(Object obj, int columnIndex, int rowIndex) { log.trace("getDataValue(obj={} rowIndex={}, columnIndex={}): start", obj, rowIndex, columnIndex); try { + Object[] objModelElements = retrieveObjectModelElements(obj, rowIndex); + if (objModelElements != null) + return theValue = objModelElements; + long index = rowIndex * arraySize; if (baseTypeDataProvider instanceof CompoundDataProvider) { @@ -1239,16 +1305,32 @@ public Object getDataValue(Object obj, int index) "getDataValue(Object, int) should not be called for ArrayDataProviders"); } + @Override + public boolean isCellEditable(int columnIndex, int rowIndex) + { + int bufIndex = physicalLocationToBufIndex(rowIndex, columnIndex); + if (dataBuf instanceof Object[] slots && bufIndex >= 0 && bufIndex < slots.length && + slots[bufIndex] instanceof List) + return isVarStrBase; + return true; + } + @Override public void setDataValue(int columnIndex, int rowIndex, Object newValue) { try { int bufIndex = physicalLocationToBufIndex(rowIndex, columnIndex); + if (updateObjectModelElements(dataBuf, newValue, bufIndex)) + return; + bufIndex *= arraySize; updateArrayElements(dataBuf, newValue, columnIndex, bufIndex); } + catch (UnsupportedOperationException uoe) { + throw uoe; + } catch (Exception ex) { log.debug("setDataValue({}, {}, {}): cell value update failure: ", rowIndex, columnIndex, newValue, ex); @@ -1260,10 +1342,16 @@ public void setDataValue(int columnIndex, int rowIndex, Object newValue) public void setDataValue(int columnIndex, int rowIndex, Object bufObject, Object newValue) { try { + if (updateObjectModelElements(bufObject, newValue, rowIndex)) + return; + long bufIndex = rowIndex * arraySize; updateArrayElements(bufObject, newValue, columnIndex, (int)bufIndex); } + catch (UnsupportedOperationException uoe) { + throw uoe; + } catch (Exception ex) { log.debug("setDataValue({}, {}, {}, {}): cell value update failure: ", rowIndex, columnIndex, bufObject, newValue, ex); @@ -1278,6 +1366,42 @@ public void setDataValue(int index, Object bufObject, Object newValue) "setDataValue(int, Object, Object) should not be called for ArrayDataProviders"); } + /** + * Write one cell back into the List held in this point's slot, returning false if + * the buffer is not in that layout. + */ + private boolean updateObjectModelElements(Object curBuf, Object newValue, int pointIndex) + { + if (!(curBuf instanceof Object[] slots) || pointIndex < 0 || pointIndex >= slots.length) + return false; + if (!(slots[pointIndex] instanceof List elements)) + return false; + + // The bracketed cell text is only unambiguous when the elements are scalars. + if (!isVarStrBase) + throw new UnsupportedOperationException( + "editing an array of variable-length data is only supported for strings"); + + StringTokenizer st = new StringTokenizer((String)newValue, ",[]"); + if (st.countTokens() < arraySize) { + log.trace("updateObjectModelElements(): number of data points ({}) < array size {}", + st.countTokens(), arraySize); + return true; + } + + @SuppressWarnings("unchecked") + List target = (List)elements; + int count = (int)Math.min(arraySize, target.size()); + for (int i = 0; i < count; i++) { + String token = st.nextToken().trim(); + if (!token.equals(target.get(i))) { + target.set(i, token); + isValueChanged = true; + } + } + return true; + } + private void updateArrayElements(Object curBuf, Object newValue, int columnIndex, int bufStartIndex) { StringTokenizer st = new StringTokenizer((String)newValue, ",[]"); @@ -1361,8 +1485,6 @@ private static class VlenDataProvider extends HDFDataProvider { private static final Logger log = LoggerFactory.getLogger(VlenDataProvider.class); private final HDFDataProvider baseTypeDataProvider; - private final Datatype baseType; - private final int numInnerLeaves; private final StringBuilder buffer; @@ -1373,10 +1495,9 @@ private static class VlenDataProvider extends HDFDataProvider { { super(dtype, dataBuf, dataTransposed); - baseType = dtype.getDatatypeBase(); + Datatype baseType = dtype.getDatatypeBase(); baseTypeClass = baseType.getDatatypeClass(); baseTypeDataProvider = getDataProvider(baseType, dataBuf, dataTransposed); - numInnerLeaves = DataFactoryUtils.countLeafNames(baseType); buffer = new StringBuilder(); } @@ -1562,39 +1683,6 @@ private Object[] retrieveArrayOfAtomicElements(Object objBuf, int rowStartIdx) return tempArray; } - /** - * Return one cell value from the row's vlen, given the cell's offset within - * the vlen's column block. The block is laid out element-major: - * {@code [elem0_leaf0, elem0_leaf1, ..., elem1_leaf0, ...]}. Returns null - * when the offset falls past the row's actual vlen length. - */ - Object getElementValue(Object objBuf, int rowIdx, int elementIndex) - { - try { - ArrayList vlElements = ((ArrayList[])objBuf)[rowIdx]; - - if (baseTypeDataProvider instanceof CompoundDataProvider && numInnerLeaves > 0) { - int vlenElemIdx = elementIndex / numInnerLeaves; - int leafIdx = elementIndex % numInnerLeaves; - if (vlenElemIdx < 0 || vlenElemIdx >= vlElements.size()) - return null; - return baseTypeDataProvider.getDataValue(vlElements.get(vlenElemIdx), leafIdx, 0); - } - - if (elementIndex < 0 || elementIndex >= vlElements.size()) - return null; - - Object elem = vlElements.get(elementIndex); - if (elem instanceof byte[]) - return baseTypeDataProvider.getDataValue(elem, 0); - return elem; - } - catch (Exception ex) { - log.debug("getElementValue(rowIdx={}, elementIndex={}): failure: ", rowIdx, elementIndex, ex); - return DataFactoryUtils.errStr; - } - } - @Override public Object getDataValue(Object obj, int index) { @@ -1602,9 +1690,21 @@ public Object getDataValue(Object obj, int index) "getDataValue(Object, int) should not be called for VlenDataProviders"); } + @Override + public boolean isCellEditable(int columnIndex, int rowIndex) + { + return !(baseTypeDataProvider instanceof CompoundDataProvider); + } + @Override public void setDataValue(int columnIndex, int rowIndex, Object newValue) { + // A vlen-of-compound cell is the whole sequence, which no per-member write + // can be derived from. + if (baseTypeDataProvider instanceof CompoundDataProvider) + throw new UnsupportedOperationException( + "editing a variable-length sequence of compound values is not supported"); + try { int bufIndex = physicalLocationToBufIndex(rowIndex, columnIndex); @@ -1624,6 +1724,12 @@ public void setDataValue(int columnIndex, int rowIndex, Object newValue) @Override public void setDataValue(int columnIndex, int rowIndex, Object bufObject, Object newValue) { + // A vlen-of-compound cell is the whole sequence, which no per-member write + // can be derived from. + if (baseTypeDataProvider instanceof CompoundDataProvider) + throw new UnsupportedOperationException( + "editing a variable-length sequence of compound values is not supported"); + try { long vlSize = Array.getLength(bufObject); log.trace("setDataValue(): vlSize={} for [c{}, r{}]", vlSize, columnIndex, rowIndex); @@ -1664,19 +1770,8 @@ else if (baseTypeDataProvider instanceof VlenDataProvider) private void updateArrayOfCompoundElements(Object newValue, Object curBuf, int columnIndex, int rowIndex) { - long vlSize = Array.getLength(curBuf); - log.trace("updateArrayOfCompoundElements(): vlSize={}", vlSize); - long adjustedRowIdx = - (rowIndex * vlSize * colCount) + - (columnIndex / ((CompoundDataProvider)baseTypeDataProvider).baseProviderIndexMap.size()); - long adjustedColIdx = - columnIndex % ((CompoundDataProvider)baseTypeDataProvider).baseProviderIndexMap.size(); - - /* - * Since we flatten array of compound types, we only need to update a single value. - */ - baseTypeDataProvider.setDataValue((int)adjustedColIdx, (int)adjustedRowIdx, curBuf, newValue); - isValueChanged = isValueChanged || baseTypeDataProvider.getIsValueChanged(); + throw new UnsupportedOperationException( + "editing a variable-length sequence of compound values is not supported"); } private void updateArrayOfArrayElements(Object newValue, Object curBuf, int columnIndex, int rowIndex) diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java index e0501bf6a..71e52eb8f 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultBaseTableView.java @@ -105,7 +105,6 @@ import org.eclipse.nebula.widgets.nattable.style.DisplayMode; import org.eclipse.nebula.widgets.nattable.style.HorizontalAlignmentEnum; import org.eclipse.nebula.widgets.nattable.style.Style; -import org.eclipse.nebula.widgets.nattable.ui.action.IKeyAction; import org.eclipse.nebula.widgets.nattable.ui.action.IMouseAction; import org.eclipse.nebula.widgets.nattable.ui.binding.UiBindingRegistry; import org.eclipse.nebula.widgets.nattable.ui.matcher.CellEditorMouseEventMatcher; @@ -120,7 +119,6 @@ import org.eclipse.swt.custom.ScrolledComposite; import org.eclipse.swt.events.DisposeEvent; import org.eclipse.swt.events.DisposeListener; -import org.eclipse.swt.events.KeyEvent; import org.eclipse.swt.events.MouseEvent; import org.eclipse.swt.events.SelectionAdapter; import org.eclipse.swt.events.SelectionEvent; @@ -224,14 +222,6 @@ protected enum ViewType { /** status if file is read only. */ protected boolean isReadOnly = false; - /** - * True when the dataset's datatype contains a construct whose write path - * is not yet symmetric with the display path (see - * {@link DataFactoryUtils#isUnsafeForWrite}). Editing is force-disabled - * and a notice is shown when the user attempts to edit. - */ - protected boolean unsafeForWrite = false; - /** status if the enums are to display converted. */ protected boolean isEnumConverted = false; @@ -422,13 +412,6 @@ public void widgetDisposed(DisposeEvent e) } } - if (!isReadOnly && DataFactoryUtils.isUnsafeForWrite(dataObject.getDatatype())) { - unsafeForWrite = true; - isReadOnly = true; - log.info("dataObject({}) editing disabled: unsafe datatype for table-view write path", - dataObject); - } - log.trace("dataObject({}) isReadOnly={}", dataObject, isReadOnly); long[] dims = dataObject.getDims(); @@ -1295,33 +1278,6 @@ public void updateValueInFile() log.debug("updateValueInFile(): EXIT - value changed flag cleared"); } - /** - * Show the editing-disabled notice. Called when the user attempts to edit - * a cell in a dataset whose datatype has no symmetric write path (see - * {@link DataFactoryUtils#isUnsafeForWrite}). Throttled to one dialog per - * second so a held-down key or rapid clicks don't stack popups. - */ - private long lastUnsafeWriteNoticeMs = 0; - protected final void showUnsafeWriteNotice() - { - if (!unsafeForWrite || shell == null || shell.isDisposed()) - return; - long now = System.currentTimeMillis(); - if (now - lastUnsafeWriteNoticeMs < 1000) - return; - lastUnsafeWriteNoticeMs = now; - - Tools.showInformation(shell, "Editing disabled", - "HDFView does not support editing datasets that contain any of the following:\n" - + " - variable-length sequences (non-string)\n" - + " - arrays of compound, array, or variable-length sequence\n" - + " - arrays nested inside compound\n" - + " - compounds nested inside compound\n\n" - + - "Use a separate tool (h5py, the HDF5 C API, h5edit, etc.) to modify these " - + "datatypes."); - } - @Override public HObject getDataObject() { @@ -2590,35 +2546,6 @@ public void configureUiBindings(UiBindingRegistry uiBindingRegistry) new MouseEditAction()); } }); - - // When the datatype's write path is not yet implemented, intercept - // edit attempts and show a dialog explaining what's blocked. - if (unsafeForWrite) { - this.addConfiguration(new AbstractUiBindingConfiguration() { - @Override - public void configureUiBindings(UiBindingRegistry uiBindingRegistry) - { - uiBindingRegistry.registerFirstDoubleClickBinding( - new MouseEventMatcher(SWT.NONE, GridRegion.BODY, - MouseEventMatcher.LEFT_BUTTON), - new IMouseAction() { - @Override - public void run(NatTable table, MouseEvent event) - { - showUnsafeWriteNotice(); - } - }); - uiBindingRegistry.registerFirstKeyBinding( - new LetterOrDigitKeyEventMatcher(), new IKeyAction() { - @Override - public void run(NatTable table, KeyEvent event) - { - showUnsafeWriteNotice(); - } - }); - } - }); - } } } } diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java index b32433ff6..dee9c9d8c 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java @@ -325,6 +325,9 @@ public boolean isEditable(int columnIndex, int rowIndex) * limitation. */ // return !(isReadOnly || isDisplayTypeChar || showAsBin || showAsHex); + if (!dataProvider.isCellEditable(columnIndex, rowIndex)) + return false; + return !isReadOnly; } }; @@ -679,15 +682,6 @@ private class CompoundDSCellSelectionListener implements ILayerListener { public void handleLayerEvent(ILayerEvent e) { if (e instanceof CellSelectionEvent) { - // For datatypes where the display column count has been - // expanded (array-of-compound, vlen-of-compound, etc.) the - // listener below hits a null ptr because baseIndexMap was built from the - // unexpanded member count. Editing is already disabled for - // these via unsafeForWrite, and the listener's side effects - // (ref preview, etc.) don't apply, so just return. - if (unsafeForWrite) - return; - CellSelectionEvent event = (CellSelectionEvent)e; boolean valIsRegRef = false; boolean valIsObjRef = false; @@ -715,6 +709,10 @@ public void handleLayerEvent(ILayerEvent e) catch (Exception ex) { log.debug("CompoundDSCellSelectionListener: buildIndexMaps", ex); } + if (maps == null) { + log.debug("CompoundDSCellSelectionListener: index maps unavailable"); + return; + } baseIndexMap = maps[DataFactoryUtils.COL_TO_BASE_CLASS_MAP_INDEX]; relCmpdStartIndexMap = maps[DataFactoryUtils.CMPD_START_IDX_MAP_INDEX]; @@ -754,17 +752,38 @@ public void handleLayerEvent(ILayerEvent e) log.trace("CompoundDSCellSelectionListener: CellSelected fieldIndex={}:{}", rowIdx, fieldIndex); - int bIndex = baseIndexMap.get(fieldIndex - 1); - Object colValue = ((List)dataValue).get(bIndex); - if (colValue == null) - log.debug("CompoundDSCellSelectionListener: CellSelected colValue is null for Idx={}", - bIndex); + /* + * The display may cover more columns than the compound has members, so a + * column can fall outside the map. Reference detection is best-effort; + * the label and value field below are updated either way. + */ + Integer bIndexObj = baseIndexMap.get(fieldIndex - 1); + if (bIndexObj == null) { + log.debug("CompoundDSCellSelectionListener: no member mapping for column {}", + fieldIndex - 1); + } + else if (bIndexObj < 0 || bIndexObj >= selectedMemberTypes.length) { + log.debug("CompoundDSCellSelectionListener: member index {} out of range (max {})", + bIndexObj, selectedMemberTypes.length); + } + else { + int bIndex = bIndexObj; + + if (dataValue instanceof List) { + Object colValue = ((List)dataValue).get(bIndex); + if (colValue == null) + log.debug( + "CompoundDSCellSelectionListener: CellSelected colValue is null for Idx={}", + bIndex); + } - Datatype selectedType = selectedMemberTypes[bIndex]; + Datatype selectedType = selectedMemberTypes[bIndex]; - if (selectedType.isRef()) { - valIsRegRef = (selectedType.getDatatypeSize() == HDF5Constants.H5R_DSET_REG_REF_BUF_SIZE); - valIsObjRef = (selectedType.getDatatypeSize() == HDF5Constants.H5R_OBJ_REF_BUF_SIZE); + if (selectedType.isRef()) { + valIsRegRef = + (selectedType.getDatatypeSize() == HDF5Constants.H5R_DSET_REG_REF_BUF_SIZE); + valIsObjRef = (selectedType.getDatatypeSize() == HDF5Constants.H5R_OBJ_REF_BUF_SIZE); + } } int rowStart = ((RowHeaderDataProvider)rowHeaderDataProvider).start; @@ -849,8 +868,7 @@ private class CompoundDSColumnHeaderDataProvider implements IDataProvider { Datatype cmpdType = dataObject.getDatatype(); - // A top-level vlen-of-compound is a single column (the whole sequence). It is not - // a compound, so it has no members to filter - use it directly as the sole type. + // A top-level vlen is one column and has no members to filter. List selectedTypes; if (cmpdType.isVLEN() && !cmpdType.isVarStr()) selectedTypes = new ArrayList<>(java.util.Collections.singletonList(cmpdType)); @@ -955,17 +973,14 @@ else if (nestedCompoundType.isArray()) { memberTypes); } else if (curDtype.isVLEN() && !curDtype.isVarStr()) { - // A top-level vlen (including vlen-of-compound) is a single column showing - // the whole sequence. Never recurse into a compound base - that would create - // one column per inner member. + // A top-level vlen is one column holding the whole sequence. for (int j = 0; j < memberNames.size(); j++) outColNames.add(memberNames.get(j).replaceAll(CompoundDS.SEPARATOR, "->")); } else if (curDtype.isCompound()) { /* - * memberNames is the flat list of leaf names; memberTypes is the list of - * top-level member types. Advance through memberTypes one slot at a time, - * consuming countLeafNames(topType) flat names per slot. + * memberNames is flat leaf names, memberTypes top-level member types; + * each type consumes countLeafNames() of the names. */ ListIterator localIt = memberNames.listIterator(); int topIdx = 0; @@ -981,9 +996,8 @@ else if (curDtype.isCompound()) { boolean nestedArrayOfCompound = false; /* - * Recursively detect any nested ARRAY of compound types and deal with them - * by creating multiple copies of the member names. A vlen is NOT expanded - - * it is a single column (handled below), so it is excluded here. + * A nested ARRAY of compound repeats the member names once per + * element. A vlen is one column and is handled below. */ if (curType.isArray()) { Datatype base = curType.getDatatypeBase(); @@ -1009,9 +1023,8 @@ else if (curDtype.isCompound()) { List selMemberNames; int namesConsumed; if (curType.isVLEN()) { - // Vlen wraps the flat name list at the header level only; - // synthesize the inner-compound leaf names from the declared - // member-name list, using the header as prefix. + // A vlen contributes only a header name, so synthesize the + // inner leaf names from it. String baseName = curName.replaceAll(CompoundDS.SEPARATOR, "->"); selMemberNames = buildInnerCompoundLeafNames(nestedArrayOfCompoundType, baseName); namesConsumed = 1; @@ -1031,8 +1044,7 @@ else if (curDtype.isCompound()) { remainingLeavesInTop -= namesConsumed; } else if (curType.isVLEN() && !curType.isVarStr()) { - // A vlen member is a single column showing the whole sequence as a - // brace-string (VlenDataProvider recurses for nested compounds). + // A vlen member is one column holding the whole sequence. String baseName = curName.replaceAll(CompoundDS.SEPARATOR, "->"); outColNames.add(baseName); remainingLeavesInTop--; @@ -1048,11 +1060,7 @@ else if (curType.isVLEN() && !curType.isVarStr()) { } } - /** - * Produce "prefix->leaf" names for each leaf of {@code innerCompound}. - * Used by the vlen-of-compound header path, whose flat name list contains - * only the wrapper header rather than per-leaf entries. - */ + /** Produce "prefix->leaf" names for each leaf of {@code innerCompound}. */ private List buildInnerCompoundLeafNames(Datatype innerCompound, String prefix) { List out = new ArrayList<>(); @@ -1226,11 +1234,7 @@ else if (groupTitleStartPosition > 0) { colindex); } else if (allColumnNames[i].matches(".*\\[[0-9]*\\]")) { - /* - * Top-level array/vlen of an atomic base. Each element is a single - * column, so group them all under the member name rather than a - * generic per-element "ARRAY[i]" group. - */ + // Group every element under the member name. String baseName = allColumnNames[i].replaceAll("\\[[0-9]*\\]$", ""); columnHeaderBuilder.append(baseName); diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultScalarDSTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultScalarDSTableView.java index b96f9bae5..d79ee9341 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultScalarDSTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultScalarDSTableView.java @@ -850,6 +850,9 @@ public boolean isEditable(int columnIndex, int rowIndex) * (0b00000000-0b11111111). Related: DefaultCompoundDSTableView.java line 320 has same * limitation. */ + if (!dataProvider.isCellEditable(columnIndex, rowIndex)) + return false; + return !(isReadOnly || isDisplayTypeChar || showAsBin || showAsHex); } }; diff --git a/hdfview/src/test/java/hdf/view/TableView/DataFactoryUtilsTest.java b/hdfview/src/test/java/hdf/view/TableView/DataFactoryUtilsTest.java new file mode 100644 index 000000000..f89910992 --- /dev/null +++ b/hdfview/src/test/java/hdf/view/TableView/DataFactoryUtilsTest.java @@ -0,0 +1,214 @@ +/***************************************************************************** + * Copyright by The HDF Group. * + * All rights reserved. * + * * + * This file is part of the HDF Java Products distribution. * + * The full copyright notice, including terms governing use, modification, * + * and redistribution, is contained in the COPYING file, which can be found * + * at the root of the source code distribution tree, * + * or in https://www.hdfgroup.org/licenses. * + * If you do not have access to either file, you may request a copy from * + * help@hdfgroup.org. * + ****************************************************************************/ + +package hdf.view.TableView; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.file.Path; +import java.util.List; + +import hdf.object.Datatype; +import hdf.object.FileFormat; +import hdf.object.h5.H5CompoundDS; +import hdf.object.h5.H5File; + +import hdf.hdf5lib.H5; +import hdf.hdf5lib.HDF5Constants; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Tests for DataFactoryUtils leaf counting and member filtering against a compound + * of {int, vlen-of-int, vlen-of-compound}: an inner compound reached by recursion + * keeps all of its members, and a vlen counts as one leaf whatever it wraps. + */ +public class DataFactoryUtilsTest { + + @TempDir + static Path tempDir; + + private static final String DATASET_NAME = "compound_of_vlen_compound"; + + private static H5File testFile; + private static H5CompoundDS testDataset; + + @BeforeAll + static void createFile() throws Exception + { + String path = tempDir.resolve("data_factory_utils_test.h5").toString(); + + long innerTid = -1, vlenIntTid = -1, vlenInnerTid = -1, outerTid = -1; + long fid = -1, spaceId = -1, did = -1; + try { + innerTid = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, 8); + H5.H5Tinsert(innerTid, "p", 0, HDF5Constants.H5T_NATIVE_INT); + H5.H5Tinsert(innerTid, "q", 4, HDF5Constants.H5T_NATIVE_INT); + + vlenIntTid = H5.H5Tvlen_create(HDF5Constants.H5T_NATIVE_INT); + vlenInnerTid = H5.H5Tvlen_create(innerTid); + + // Lay fields out back-to-back using actual sizes rather than assumed + // struct-alignment offsets (hvl_t size is platform-dependent). + long idOffset = 0; + long tagsOffset = idOffset + H5.H5Tget_size(HDF5Constants.H5T_NATIVE_INT); + long nestedOffset = tagsOffset + H5.H5Tget_size(vlenIntTid); + long outerSize = nestedOffset + H5.H5Tget_size(vlenInnerTid); + + outerTid = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, outerSize); + H5.H5Tinsert(outerTid, "id", idOffset, HDF5Constants.H5T_NATIVE_INT); + H5.H5Tinsert(outerTid, "tags", tagsOffset, vlenIntTid); + H5.H5Tinsert(outerTid, "nested", nestedOffset, vlenInnerTid); + + fid = H5.H5Fcreate(path, HDF5Constants.H5F_ACC_TRUNC, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT); + spaceId = H5.H5Screate_simple(1, new long[] {0}, null); + did = H5.H5Dcreate(fid, DATASET_NAME, outerTid, spaceId, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT, HDF5Constants.H5P_DEFAULT); + } + finally { + if (did >= 0) + H5.H5Dclose(did); + if (spaceId >= 0) + H5.H5Sclose(spaceId); + if (fid >= 0) + H5.H5Fclose(fid); + if (outerTid >= 0) + H5.H5Tclose(outerTid); + if (vlenInnerTid >= 0) + H5.H5Tclose(vlenInnerTid); + if (vlenIntTid >= 0) + H5.H5Tclose(vlenIntTid); + if (innerTid >= 0) + H5.H5Tclose(innerTid); + } + + testFile = (H5File)(new H5File()).createInstance(path, FileFormat.READ); + testFile.open(); + testDataset = (H5CompoundDS)testFile.get("/" + DATASET_NAME); + testDataset.init(); + } + + @AfterAll + static void closeFile() throws Exception + { + if (testFile != null) + testFile.close(); + } + + private static Datatype memberType(String name) + { + List names = testDataset.getDatatype().getCompoundMemberNames(); + List types = testDataset.getDatatype().getCompoundMemberTypes(); + return types.get(names.indexOf(name)); + } + + @Test + void countLeafNames_atomicMemberIsOne() + { + assertEquals(1, DataFactoryUtils.countLeafNames(memberType("id"))); + } + + @Test + void countLeafNames_vlenOfAtomicIsOne() + { + assertEquals(1, DataFactoryUtils.countLeafNames(memberType("tags"))); + } + + @Test + void countLeafNames_vlenOfCompoundIsOne() + { + // A vlen member is always a single column, regardless of what it wraps - + // it must NOT be flattened into its base compound's leaf count. + Datatype nested = memberType("nested"); + assertTrue(nested.isVLEN()); + assertTrue(nested.getDatatypeBase().isCompound()); + + assertEquals(1, DataFactoryUtils.countLeafNames(nested)); + } + + @Test + void countLeafNames_plainCompoundSumsItsChildren() + { + Datatype innerCompound = memberType("nested").getDatatypeBase(); + assertEquals(2, DataFactoryUtils.countLeafNames(innerCompound)); // p, q + } + + @Test + void countLeafNames_topLevelCompoundCountsTopLevelMembersOnly() + { + // id(1) + tags:VLEN(1) + nested:VLEN(1) = 3 - NOT the fully-flattened leaf + // count (which would incorrectly be 1 + 1 + 2 = 4). + assertEquals(3, DataFactoryUtils.countLeafNames(testDataset.getDatatype())); + } + + @Test + void countLeafNames_agreesWithExtractCompoundInfoFlatNameList() + { + // countLeafNames exists to mirror the flat leaf-name list that + // H5Datatype.extractCompoundInfo produces; recursiveColumnHeaderSetup walks that + // list using these counts to decide which top-level member each name belongs to. + // If the two ever disagree, column headers silently misalign - so assert the + // relationship itself rather than a hardcoded number. + assertEquals(testDataset.getSelectedMemberNames().length, + DataFactoryUtils.countLeafNames(testDataset.getDatatype())); + } + + @Test + void filterNonSelectedMembers_innerCompoundKeepsAllMembers() + { + // The dataset's selected-member list names only id/tags/nested, never p/q, so + // the isTopLevel=false path must skip the filter entirely. + Datatype innerCompound = memberType("nested").getDatatypeBase(); + + List filtered = + DataFactoryUtils.filterNonSelectedMembers(testDataset, innerCompound, false); + + assertNotNull(filtered); + assertEquals(2, filtered.size(), "inner compound's members must survive unfiltered"); + } + + @Test + void filterNonSelectedMembers_topLevelFiltersAgainstSelection() + { + Datatype idType = memberType("id"); + + // All members are selected by default after init(). + List filtered = + DataFactoryUtils.filterNonSelectedMembers(testDataset, testDataset.getDatatype(), true); + assertEquals(3, filtered.size()); + + // Deselect everything except "id" and confirm the top-level filter honors it. + List names = testDataset.getDatatype().getCompoundMemberNames(); + testDataset.setAllMemberSelection(false); + testDataset.selectMember(names.indexOf("id")); + + try { + filtered = + DataFactoryUtils.filterNonSelectedMembers(testDataset, testDataset.getDatatype(), true); + + assertEquals(1, filtered.size()); + assertSame(idType, filtered.get(0)); + } + finally { + // testDataset is shared across test methods - leave selection as init() set it. + testDataset.setAllMemberSelection(true); + } + } +} diff --git a/object/src/main/java/hdf/object/h5/H5CompoundDS.java b/object/src/main/java/hdf/object/h5/H5CompoundDS.java index e9c349297..ed321bc2b 100644 --- a/object/src/main/java/hdf/object/h5/H5CompoundDS.java +++ b/object/src/main/java/hdf/object/h5/H5CompoundDS.java @@ -929,6 +929,7 @@ private Object compoundDatasetCommonIO(H5File.IO_TYPE ioType, Object writeBuf) t */ private Object compoundTypeIO(H5File.IO_TYPE ioType, long did, long[] spaceIDs, int nSelPoints, final H5Datatype cmpdType, Object writeBuf, int[] globalMemberIndex) + throws Exception { Object theData = null; @@ -944,39 +945,38 @@ private Object compoundTypeIO(H5File.IO_TYPE ioType, long did, long[] spaceIDs, } else if (cmpdType.isVLEN() && !cmpdType.isVarStr()) { /* - * Top-level VLEN-of-compound is displayed as a SINGLE column showing the whole - * sequence (e.g. [{1, 2}, {3, 4}]), consistent with how a vlen member of a - * compound is shown. Read the whole vlen in one H5DreadVL call; the JNI parses - * each compound element into nested Lists (e.g. [[1, 2], [3, 4]]), exactly the - * shape VlenDataProvider/VlenDataDisplayConverter render. The dataset enumerates - * a single member (see H5Datatype.extractCompoundInfo), so we return one column. + * A top-level VLEN-of-compound is one column holding the whole sequence, so + * transfer it in a single call; the JNI parses each compound element into a + * nested List. The dataset enumerates a single member, hence one column. */ - if (ioType == H5File.IO_TYPE.READ) { - long wholeTid = -1; - try { - wholeTid = H5.H5Dget_type(did); - @SuppressWarnings("rawtypes") - ArrayList[] vlBuf = new ArrayList[nSelPoints]; - H5.H5DreadVL(did, wholeTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, vlBuf); - globalMemberIndex[0]++; - theData = vlBuf; - } - catch (HDF5DataFiltersException exfltr) { - log.debug("compoundTypeIO(): top-level VLEN read failure: ", exfltr); - throw new HDF5Exception("Filter not available exception: " + exfltr.getMessage()); - } - catch (Exception ex) { - log.debug("compoundTypeIO(): top-level VLEN read failure: ", ex); - throw new HDF5Exception("failed to read VLEN-of-compound dataset: " + ex.getMessage()); - } - finally { - if (wholeTid >= 0) { - try { - H5.H5Tclose(wholeTid); - } - catch (Exception ex) { - log.debug("compoundTypeIO(): H5Tclose(wholeTid {}) failure: ", wholeTid, ex); - } + if (ioType != H5File.IO_TYPE.READ) + throw new UnsupportedOperationException( + "writing a VLEN of compound is rejected by compoundDatasetCommonIO"); + + long wholeTid = -1; + try { + wholeTid = H5.H5Dget_type(did); + @SuppressWarnings("rawtypes") + ArrayList[] vlBuf = new ArrayList[nSelPoints]; + H5.H5DreadVL(did, wholeTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, vlBuf); + globalMemberIndex[0]++; + theData = vlBuf; + } + catch (HDF5DataFiltersException exfltr) { + log.debug("compoundTypeIO(): top-level VLEN read failure: ", exfltr); + throw new HDF5Exception("Filter not available exception: " + exfltr.getMessage()); + } + catch (Exception ex) { + log.debug("compoundTypeIO(): top-level VLEN read failure: ", ex); + throw new HDF5Exception("failed to read VLEN-of-compound dataset: " + ex.getMessage()); + } + finally { + if (wholeTid >= 0) { + try { + H5.H5Tclose(wholeTid); + } + catch (Exception ex) { + log.debug("compoundTypeIO(): H5Tclose(wholeTid {}) failure: ", wholeTid, ex); } } } @@ -1174,6 +1174,10 @@ else if ( catch (Exception ex) { log.debug("compoundTypeIO(): failed to write member[{}]: ", i, ex); globalMemberIndex[0]++; + // The member was not persisted; do not report success. + throw new Exception("failed to write compound member '" + memberName + + "': " + ex.getMessage(), + ex); } } } // (i = 0, writeListIndex = 0; i < atomicTypeList.size(); i++) @@ -1181,6 +1185,10 @@ else if ( catch (Exception ex) { log.debug("compoundTypeIO(): failure: ", ex); memberDataList = null; + // A failed write leaves data unpersisted and must be reported; a + // failed read yields a null compound. + if (ioType == H5File.IO_TYPE.WRITE) + throw ex; } theData = memberDataList; @@ -1241,8 +1249,7 @@ private Object readSingleCompoundMember(long dsetID, long[] spaceIDs, int nSelPo (spaceIDs[0] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[0], (spaceIDs[1] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[1]); - // Slots are left null: H5DreadVL installs a freshly - // allocated ArrayList into each slot. + // Slots are left null; H5DreadVL allocates each list. @SuppressWarnings("rawtypes") ArrayList[] vlBuf = new ArrayList[nSelPoints]; @@ -1279,8 +1286,7 @@ private Object readSingleCompoundMember(long dsetID, long[] spaceIDs, int nSelPo H5.H5Dread_VLStrings(dsetID, compTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, (Object[])memberData); } - else if (memberType.isVLEN() || - (memberType.isArray() && memberType.getDatatypeBase().isVLEN())) { + else if (H5Datatype.containsVlenData(memberType)) { log.trace( "readSingleCompoundMember(): H5DreadVL did={} compTid={} spaceIDs[0]={} spaceIDs[1]={}", dsetID, compTid, @@ -1290,15 +1296,15 @@ else if (memberType.isVLEN() || H5.H5DreadVL(dsetID, compTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, (Object[])memberData); - // H5DreadVL was called with a single-field compound transfer type, so - // each row comes back wrapped in a one-element record. Unwrap to expose - // the payload directly. - if (memberType.isVLEN()) { - Object[] rows = (Object[])memberData; - for (int r = 0; r < rows.length; r++) { - if (rows[r] instanceof java.util.ArrayList rec && rec.size() == 1) - rows[r] = rec.get(0); - } + /* + * The single-field compound transfer type wraps each row in a + * one-element record holding the member's value; unwrap it. + */ + Object[] rows = (Object[])memberData; + for (int r = 0; r < rows.length; r++) { + if (rows[r] instanceof java.util.ArrayList rec && rec.size() == 1 && + rec.get(0) instanceof java.util.List) + rows[r] = rec.get(0); } } else { diff --git a/object/src/main/java/hdf/object/h5/H5Datatype.java b/object/src/main/java/hdf/object/h5/H5Datatype.java index ddd37689b..3b31667ae 100644 --- a/object/src/main/java/hdf/object/h5/H5Datatype.java +++ b/object/src/main/java/hdf/object/h5/H5Datatype.java @@ -1974,6 +1974,37 @@ else if (datatypeSize == 8) { return tid; } + /** + * Returns true when this datatype contains a variable-length sequence or + * variable-length string at any depth. Mirrors h5str_detect_vlen() in the JNI, + * which decides there whether an I/O call uses the variable-length object model. + * + * @param dtype the datatype to inspect + * + * @return true if the type contains variable-length data + */ + public static boolean containsVlenData(final Datatype dtype) + { + if (dtype == null) + return false; + + if (dtype.isVarStr() || dtype.getDatatypeClass() == CLASS_VLEN) + return true; + + if (dtype.isCompound()) { + List members = dtype.getCompoundMemberTypes(); + if (members != null) { + for (Datatype member : members) { + if (containsVlenData(member)) + return true; + } + } + return false; + } + + return containsVlenData(dtype.getDatatypeBase()); + } + /** * Allocates a one-dimensional array of byte, short, int, long, float, double, or String to store data in * memory. @@ -2166,31 +2197,42 @@ else if ((typeClass == CLASS_STRING) || (typeClass == CLASS_REFERENCE)) { else if (dtype.isVLEN()) { log.trace("allocateArray(): isVLEN"); - // Slots are left null: the JNI read routines (H5DreadVL/H5AreadVL) - // install a freshly allocated ArrayList into each slot. Callers must - // not pre-allocate the per-element lists. + // Slots are left null; the read routines allocate each list. data = new ArrayList[numPoints]; } else if (typeClass == CLASS_ARRAY) { log.trace("allocateArray(): class CLASS_ARRAY"); - try { - log.trace("allocateArray(): ArrayRank={}", dtype.getArrayDims().length); + /* + * Per the JNI buffer data model (see "Buffer data model" in H5.java), an + * array containing variable-length data is read as one slot per selected + * point, each holding an ArrayList of the array's elements. The read + * routines allocate those lists, so the slots are left null, and the + * container must be Object[] rather than a narrowly-typed array. + */ + if (containsVlenData(dtype)) { + log.trace("allocateArray(): CLASS_ARRAY contains variable-length data"); + data = new Object[numPoints]; + } + else { + try { + log.trace("allocateArray(): ArrayRank={}", dtype.getArrayDims().length); - // Use the base datatype to define the array - long[] arrayDims = dtype.getArrayDims(); - int asize = numPoints; - for (int j = 0; j < arrayDims.length; j++) { - log.trace("allocateArray(): Array dims[{}]={}", j, arrayDims[j]); + // Use the base datatype to define the array + long[] arrayDims = dtype.getArrayDims(); + int asize = numPoints; + for (int j = 0; j < arrayDims.length; j++) { + log.trace("allocateArray(): Array dims[{}]={}", j, arrayDims[j]); - asize *= arrayDims[j]; - } + asize *= arrayDims[j]; + } - if (baseType != null) - data = H5Datatype.allocateArray(baseType, asize); - } - catch (Exception ex) { - log.debug("allocateArray(): CLASS_ARRAY class failure: ", ex); + if (baseType != null) + data = H5Datatype.allocateArray(baseType, asize); + } + catch (Exception ex) { + log.debug("allocateArray(): CLASS_ARRAY class failure: ", ex); + } } } else if (typeClass == CLASS_COMPLEX) { diff --git a/object/src/main/java/hdf/object/h5/H5ScalarAttr.java b/object/src/main/java/hdf/object/h5/H5ScalarAttr.java index 96dcad4fd..784ef2c15 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarAttr.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarAttr.java @@ -1210,8 +1210,7 @@ else if (dsDatatype.isCompound()) { else if (dsDatatype.isVLEN()) { log.trace("attributeCommonIO():read ioType:VLEN-REF H5Aread isArray()={}", dsDatatype.isArray()); - // Slots are left null: H5AreadVL installs a freshly - // allocated ArrayList into each slot. + // Slots are left null; H5AreadVL allocates each list. theData = new ArrayList[(int)lsize]; try { diff --git a/object/src/main/java/hdf/object/h5/H5ScalarDS.java b/object/src/main/java/hdf/object/h5/H5ScalarDS.java index 4af594577..c42ba7787 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarDS.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarDS.java @@ -940,8 +940,7 @@ private Object scalarDatasetCommonIO(H5File.IO_TYPE ioType, Object writeBuf) thr } } else if (dsDatatype.isVLEN()) { - // Slots are left null: H5DreadVL installs a freshly - // allocated ArrayList into each slot. + // Slots are left null; H5DreadVL allocates each list. theData = new ArrayList[(int)totalSelectedSpacePoints]; } else if ((originalBuf == null) || dsDatatype.isEnum() || dsDatatype.isText() || @@ -974,9 +973,7 @@ else if ((originalBuf == null) || dsDatatype.isEnum() || dsDatatype.isText() || tid = dsDatatype.createNative(); log.trace("scalarDatasetCommonIO(): native type created tid={}", tid); - if (dsDatatype.isVarStr() || - (dsDatatype.isArray() && dsDatatype.getDatatypeBase() != null && - dsDatatype.getDatatypeBase().isVarStr())) { + if (dsDatatype.isVarStr()) { log.trace( "scalarDatasetCommonIO(): H5Dread_VLStrings did={} tid={} spaceIDs[0]={} spaceIDs[1]={}", did, tid, @@ -986,8 +983,7 @@ else if ((originalBuf == null) || dsDatatype.isEnum() || dsDatatype.isText() || H5.H5Dread_VLStrings(did, tid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, (Object[])theData); } - else if (dsDatatype.isVLEN() || - (dsDatatype.isArray() && dsDatatype.getDatatypeBase().isVLEN())) { + else if (H5Datatype.containsVlenData(dsDatatype)) { // Check for unsupported VLEN complex combination H5Datatype baseType = (H5Datatype)dsDatatype.getDatatypeBase(); if (baseType != null && baseType.isComplex()) { @@ -1127,9 +1123,7 @@ else if (dsDatatype.isFloat() && dsDatatype.getDatatypeSize() == 16) { try { tid = dsDatatype.createNative(); - if (dsDatatype.isVarStr() || - (dsDatatype.isArray() && dsDatatype.getDatatypeBase() != null && - dsDatatype.getDatatypeBase().isVarStr())) { + if (dsDatatype.isVarStr()) { log.trace( "scalarDatasetCommonIO(): H5Dwrite_VLStrings did={} tid={} spaceIDs[0]={} spaceIDs[1]={}", did, tid, @@ -1139,8 +1133,7 @@ else if (dsDatatype.isFloat() && dsDatatype.getDatatypeSize() == 16) { H5.H5Dwrite_VLStrings(did, tid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, (Object[])tmpData); } - else if (dsDatatype.isVLEN() || - (dsDatatype.isArray() && dsDatatype.getDatatypeBase().isVLEN())) { + else if (H5Datatype.containsVlenData(dsDatatype)) { log.trace( "scalarDatasetCommonIO(): H5DwriteVL did={} tid={} spaceIDs[0]={} spaceIDs[1]={}", did, tid, diff --git a/object/src/test/java/object/TestNestedDatatypeShapes.java b/object/src/test/java/object/TestNestedDatatypeShapes.java new file mode 100644 index 000000000..258d75cb7 --- /dev/null +++ b/object/src/test/java/object/TestNestedDatatypeShapes.java @@ -0,0 +1,552 @@ +package object; + +import static org.junit.jupiter.api.Assertions.*; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; + +import hdf.object.Dataset; +import hdf.object.FileFormat; +import hdf.object.h5.H5File; + +import hdf.hdf5lib.H5; +import hdf.hdf5lib.HDF5Constants; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Read tests for nested datatype shapes not covered by the shared UI test files. + * + * Fixtures are built at test time rather than committed as binaries, so adding a + * shape costs a few lines here. They are written with raw H5Dwrite/H5DwriteVL calls + * against the JNI's buffer data model; that path does not reach + * H5Datatype.allocateArray(), so the setup is independent of what is being read. + */ +@Tag("unit") +@DisplayName("Nested Datatype Shape Read Tests") +public class TestNestedDatatypeShapes { + + /* + * Written under target/ rather than a JUnit temporary directory: HDF5 can still + * hold a file open when the temporary directory is torn down, which Windows + * refuses to delete and JUnit then reports as a failure. + */ + private static Path workDir; + + private static final int WIDE_POINTS = 50; + private static final int WIDE_ELEMENTS = 230; + + private static H5File testFile; + private static int openIDsAtStart; + + @BeforeAll + static void createFile() throws Exception + { + openIDsAtStart = H5.getOpenIDCount(); + + workDir = Path.of("target", "nested-datatype-shapes"); + Files.createDirectories(workDir); + + String path = workDir.resolve("nested_shapes.h5").toString(); + long fid = H5.H5Fcreate(path, HDF5Constants.H5F_ACC_TRUNC, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT); + try { + writeVlenOfCompound(fid); + writeVlenOfVarStr(fid); + writeArrayOfVlenInt(fid); + writeArrayOfArrayVarStr(fid); + writeArrayOfArrayInt(fid); + writeArrayOfCompoundVarStr(fid); + writeCompoundOfVlenCompound(fid); + writeVlenOfFixedString(fid); + writeArrayOfFixedString(fid); + writeWideVlenOfCompound(fid); + writeCompoundWithReference(fid); + } + finally { + H5.H5Fclose(fid); + } + + testFile = (H5File)(new H5File()).createInstance(path, FileFormat.READ); + testFile.open(); + } + + @AfterAll + static void closeFile() throws Exception + { + if (testFile != null) + testFile.close(); + + assertEquals(openIDsAtStart, H5.getOpenIDCount(), "HDF5 identifiers leaked by this test class"); + } + + /** Variable-length string type; the caller closes it. */ + private static long varStrType() throws Exception + { + long tid = H5.H5Tcopy(HDF5Constants.H5T_C_S1); + H5.H5Tset_size(tid, HDF5Constants.H5T_VARIABLE); + return tid; + } + + /** Compound {p:int, q:int}; the caller closes it. */ + private static long pqCompoundType() throws Exception + { + long tid = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, 8); + H5.H5Tinsert(tid, "p", 0, HDF5Constants.H5T_NATIVE_INT); + H5.H5Tinsert(tid, "q", 4, HDF5Constants.H5T_NATIVE_INT); + return tid; + } + + private static void writeDataset(long fid, String name, long tid, int nPoints, Object buf, boolean vlen) + throws Exception + { + long sid = H5.H5Screate_simple(1, new long[] {nPoints}, null); + long did = H5.H5Dcreate(fid, name, tid, sid, HDF5Constants.H5P_DEFAULT, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT); + try { + if (vlen) + H5.H5DwriteVL(did, tid, HDF5Constants.H5S_ALL, HDF5Constants.H5S_ALL, + HDF5Constants.H5P_DEFAULT, (Object[])buf); + else + H5.H5Dwrite(did, tid, HDF5Constants.H5S_ALL, HDF5Constants.H5S_ALL, HDF5Constants.H5P_DEFAULT, + buf); + } + finally { + H5.H5Dclose(did); + H5.H5Sclose(sid); + } + } + + private static ArrayList list(Object... items) + { + ArrayList out = new ArrayList<>(); + for (Object item : items) + out.add(item); + return out; + } + + // ---- fixtures ------------------------------------------------------------- + + /** VLEN of COMPOUND{p,q}. */ + private static void writeVlenOfCompound(long fid) throws Exception + { + long inner = pqCompoundType(); + long tid = H5.H5Tvlen_create(inner); + try { + Object[] buf = {list(list(10, 11), list(20, 21)), list(list(30, 31))}; + writeDataset(fid, "vlen_of_compound", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(inner); + } + } + + /** VLEN of variable-length string. */ + private static void writeVlenOfVarStr(long fid) throws Exception + { + long vs = varStrType(); + long tid = H5.H5Tvlen_create(vs); + try { + Object[] buf = {list("a0", "a1"), list("b0")}; + writeDataset(fid, "vlen_of_varstr", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(vs); + } + } + + /** ARRAY[2] of VLEN of int. */ + private static void writeArrayOfVlenInt(long fid) throws Exception + { + long vt = H5.H5Tvlen_create(HDF5Constants.H5T_NATIVE_INT); + long tid = H5.H5Tarray_create(vt, 1, new long[] {2}); + try { + Object[] buf = {list(list(1, 2), list(3)), list(list(4), list(5, 6))}; + writeDataset(fid, "array_of_vlen_int", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(vt); + } + } + + /** ARRAY[2] of ARRAY[3] of variable-length string. */ + private static void writeArrayOfArrayVarStr(long fid) throws Exception + { + long vs = varStrType(); + long inner = H5.H5Tarray_create(vs, 1, new long[] {3}); + long tid = H5.H5Tarray_create(inner, 1, new long[] {2}); + try { + Object[] buf = {list(list("n000", "n001", "n002"), list("n010", "n011", "n012")), + list(list("n100", "n101", "n102"), list("n110", "n111", "n112"))}; + writeDataset(fid, "array_of_array_varstr", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(inner); + H5.H5Tclose(vs); + } + } + + /** ARRAY[2] of ARRAY[3] of int. */ + private static void writeArrayOfArrayInt(long fid) throws Exception + { + long inner = H5.H5Tarray_create(HDF5Constants.H5T_NATIVE_INT, 1, new long[] {3}); + long tid = H5.H5Tarray_create(inner, 1, new long[] {2}); + try { + int[] buf = {0, 1, 2, 10, 11, 12, 100, 101, 102, 110, 111, 112}; + writeDataset(fid, "array_of_array_int", tid, 2, buf, false); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(inner); + } + } + + /** ARRAY[2] of COMPOUND{n:int, s:varstr}. */ + private static void writeArrayOfCompoundVarStr(long fid) throws Exception + { + long vs = varStrType(); + long strSz = H5.H5Tget_size(vs); + long cmpd = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, 8 + strSz); + H5.H5Tinsert(cmpd, "n", 0, HDF5Constants.H5T_NATIVE_INT); + H5.H5Tinsert(cmpd, "s", 8, vs); + long tid = H5.H5Tarray_create(cmpd, 1, new long[] {2}); + try { + Object[] buf = {list(list(0, "x0"), list(1, "x1")), list(list(2, "y0"), list(3, "y1"))}; + writeDataset(fid, "array_of_compound_varstr", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(cmpd); + H5.H5Tclose(vs); + } + } + + /** COMPOUND{id:int, nested:VLEN of COMPOUND{p,q}}. */ + private static void writeCompoundOfVlenCompound(long fid) throws Exception + { + long inner = pqCompoundType(); + long vt = H5.H5Tvlen_create(inner); + long vtSize = H5.H5Tget_size(vt); + long tid = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, 8 + vtSize); + H5.H5Tinsert(tid, "id", 0, HDF5Constants.H5T_NATIVE_INT); + H5.H5Tinsert(tid, "nested", 8, vt); + try { + Object[] buf = {list(7, list(list(1, 2))), list(8, list(list(3, 4), list(5, 6)))}; + writeDataset(fid, "compound_of_vlen_compound", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(vt); + H5.H5Tclose(inner); + } + } + + /** VLEN of a one-character fixed-length string - a string stored as a sequence. */ + private static void writeVlenOfFixedString(long fid) throws Exception + { + long st = H5.H5Tcopy(HDF5Constants.H5T_C_S1); + long tid = H5.H5Tvlen_create(st); + try { + Object[] buf = {list("a", "b", "c", "d"), list("x", "y")}; + writeDataset(fid, "vlen_of_fixed_string", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(st); + } + } + + /** ARRAY[3] of fixed-length string - no variable-length data anywhere. */ + private static void writeArrayOfFixedString(long fid) throws Exception + { + long st = H5.H5Tcopy(HDF5Constants.H5T_C_S1); + H5.H5Tset_size(st, 8); + long tid = H5.H5Tarray_create(st, 1, new long[] {3}); + try { + byte[] buf = new byte[2 * 3 * 8]; + String[] values = {"alpha", "beta", "gamma", "delta", "epsilon", "zeta"}; + for (int i = 0; i < values.length; i++) + System.arraycopy(values[i].getBytes("US-ASCII"), 0, buf, i * 8, values[i].length()); + writeDataset(fid, "array_of_fixed_string", tid, 2, buf, false); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(st); + } + } + + /** VLEN of COMPOUND wide enough to catch anything that scales with element count. */ + private static void writeWideVlenOfCompound(long fid) throws Exception + { + long inner = pqCompoundType(); + long tid = H5.H5Tvlen_create(inner); + try { + Object[] buf = new Object[WIDE_POINTS]; + for (int i = 0; i < WIDE_POINTS; i++) { + ArrayList seq = new ArrayList<>(); + for (int j = 0; j < WIDE_ELEMENTS; j++) + seq.add(list(i, j)); + buf[i] = seq; + } + writeDataset(fid, "wide_vlen_of_compound", tid, WIDE_POINTS, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(inner); + } + } + + /** COMPOUND mixing a VLEN of compound, a variable-length string and an object reference. */ + private static void writeCompoundWithReference(long fid) throws Exception + { + long gid = H5.H5Gcreate(fid, "referenced_group", HDF5Constants.H5P_DEFAULT, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT); + H5.H5Gclose(gid); + byte[] ref = H5.H5Rcreate_object(fid, "referenced_group", HDF5Constants.H5P_DEFAULT); + + long inner = pqCompoundType(); + long vt = H5.H5Tvlen_create(inner); + long vs = varStrType(); + long rt = H5.H5Tcopy(HDF5Constants.H5T_STD_REF_OBJ); + long vtSize = H5.H5Tget_size(vt); + long vsSize = H5.H5Tget_size(vs); + long rtSize = H5.H5Tget_size(rt); + long tid = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, vtSize + vsSize + rtSize); + H5.H5Tinsert(tid, "seq", 0, vt); + H5.H5Tinsert(tid, "label", vtSize, vs); + H5.H5Tinsert(tid, "target", vtSize + vsSize, rt); + try { + byte[] shortRef = new byte[(int)rtSize]; + System.arraycopy(ref, 0, shortRef, 0, Math.min(ref.length, shortRef.length)); + Object[] buf = {list(list(list(1, 2), list(3, 4)), "first", shortRef), + list(list(list(5, 6)), "second", shortRef)}; + writeDataset(fid, "compound_with_reference", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(rt); + H5.H5Tclose(vs); + H5.H5Tclose(vt); + H5.H5Tclose(inner); + } + } + + // ---- helpers -------------------------------------------------------------- + + private static Dataset open(String name) throws Exception + { + Dataset dataset = (Dataset)testFile.get("/" + name); + assertNotNull(dataset, "Dataset not found: " + name); + dataset.init(); + return dataset; + } + + /** Renders a read buffer to a stable string. */ + private static String render(Object data) + { + if (data == null) + return "null"; + if (data instanceof String str) + return str; + if (data instanceof List list) { + StringBuilder sb = new StringBuilder("["); + for (int i = 0; i < list.size(); i++) { + if (i > 0) + sb.append(", "); + sb.append(render(list.get(i))); + } + return sb.append("]").toString(); + } + if (data.getClass().isArray()) { + int n = java.lang.reflect.Array.getLength(data); + StringBuilder sb = new StringBuilder("["); + for (int i = 0; i < n; i++) { + if (i > 0) + sb.append(", "); + sb.append(render(java.lang.reflect.Array.get(data, i))); + } + return sb.append("]").toString(); + } + return String.valueOf(data); + } + + // ---- tests ---------------------------------------------------------------- + + @Test + @DisplayName("VLEN of compound") + public void testVlenOfCompound() throws Exception + { + assertEquals("[[[10, 11], [20, 21]], [[30, 31]]]", render(open("vlen_of_compound").getData())); + } + + @Test + @DisplayName("VLEN of variable-length string") + public void testVlenOfVarStr() throws Exception + { + assertEquals("[[a0, a1], [b0]]", render(open("vlen_of_varstr").getData())); + } + + @Test + @DisplayName("Array of VLEN of int") + public void testArrayOfVlenInt() throws Exception + { + assertEquals("[[[1, 2], [3]], [[4], [5, 6]]]", render(open("array_of_vlen_int").getData())); + } + + @Test + @DisplayName("Array of array of variable-length string") + public void testArrayOfArrayVarStr() throws Exception + { + assertEquals("[[[n000, n001, n002], [n010, n011, n012]], " + + "[[n100, n101, n102], [n110, n111, n112]]]", + render(open("array_of_array_varstr").getData())); + } + + @Test + @DisplayName("Array of array of int") + public void testArrayOfArrayInt() throws Exception + { + Object data = open("array_of_array_int").getData(); + assertInstanceOf(int[].class, data, "No variable-length data, so the layout is flat"); + assertEquals("[0, 1, 2, 10, 11, 12, 100, 101, 102, 110, 111, 112]", render(data)); + } + + @Test + @DisplayName("Array of compound with a variable-length string member") + public void testArrayOfCompoundVarStr() throws Exception + { + // An array of compound is presented per member, not per element. + assertEquals("[[0, 1, 2, 3], [x0, x1, y0, y1]]", render(open("array_of_compound_varstr").getData())); + } + + @Test + @DisplayName("Compound with a VLEN of compound member") + public void testCompoundOfVlenCompound() throws Exception + { + String rendered = render(open("compound_of_vlen_compound").getData()); + assertTrue(rendered.contains("[1, 2]"), "Expected first record's nested element: " + rendered); + assertTrue(rendered.contains("[3, 4]"), "Expected second record's first element: " + rendered); + assertTrue(rendered.contains("[5, 6]"), "Expected second record's second element: " + rendered); + } + + @Test + @Disabled // depends on the JNI reading fixed-length strings past their element + @DisplayName("VLEN of fixed-length string") + public void testVlenOfFixedString() throws Exception + { + assertEquals("[[a, b, c, d], [x, y]]", render(open("vlen_of_fixed_string").getData())); + } + + @Test + @DisplayName("Array of fixed-length string") + public void testArrayOfFixedString() throws Exception + { + Object data = open("array_of_fixed_string").getData(); + + /* + * A fixed-length string is not variable-length data, so this takes the flat path + * and comes back as the raw bytes of every element rather than one list per + * point. Asserted because it is the boundary the variable-length detection has + * to get right: a string array that must not be treated as an object buffer. + */ + assertInstanceOf(byte[].class, data, "Expected a flat byte[]"); + assertEquals(2 * 3 * 8, ((byte[])data).length, "2 points x 3 elements x 8 bytes"); + + String text = new String((byte[])data, "US-ASCII"); + for (String value : new String[] {"alpha", "beta", "gamma", "delta", "epsilon", "zeta"}) + assertTrue(text.contains(value), "Expected element " + value); + } + + @Test + @DisplayName("VLEN of compound at width") + public void testWideVlenOfCompound() throws Exception + { + Object data = open("wide_vlen_of_compound").getData(); + + assertInstanceOf(Object[].class, data); + Object[] points = (Object[])data; + assertEquals(WIDE_POINTS, points.length, "point count"); + + for (int i = 0; i < points.length; i++) { + List seq = (List)points[i]; + assertEquals(WIDE_ELEMENTS, seq.size(), "element count at point " + i); + assertEquals(List.of(i, WIDE_ELEMENTS - 1), seq.get(WIDE_ELEMENTS - 1), + "last element of point " + i); + } + } + + @Test + @DisplayName("Compound holding a VLEN, a variable-length string and a reference") + public void testCompoundWithReference() throws Exception + { + Object data = open("compound_with_reference").getData(); + assertNotNull(data); + + List members = (List)data; + assertEquals(3, members.size(), "Expected members seq, label and target"); + + assertEquals("[[[1, 2], [3, 4]], [[5, 6]]]", render(members.get(0))); + assertEquals("[first, second]", render(members.get(1))); + assertNotNull(members.get(2), "reference member should be read"); + } + + @Test + @DisplayName("Writing a VLEN of compound is refused") + public void testVlenOfCompoundWriteRefused() throws Exception + { + /* + * Built and closed here rather than copied from the shared fixture, which stays + * open for the whole class: Windows will not copy a file another handle holds. + */ + Path target = workDir.resolve("refused.h5"); + long fid = H5.H5Fcreate(target.toString(), HDF5Constants.H5F_ACC_TRUNC, + HDF5Constants.H5P_DEFAULT, HDF5Constants.H5P_DEFAULT); + try { + writeVlenOfCompound(fid); + } + finally { + H5.H5Fclose(fid); + } + + H5File rw = (H5File)(new H5File()).createInstance(target.toString(), FileFormat.WRITE); + rw.open(); + try { + Dataset dataset = (Dataset)rw.get("/vlen_of_compound"); + dataset.init(); + Object data = dataset.getData(); + + /* + * There is no way to map an edited cell of this shape back to storage, so the + * write must fail rather than report success having stored nothing. + */ + assertThrows(Exception.class, + () -> dataset.write(data), "Writing a VLEN of compound should be refused"); + } + finally { + rw.close(); + } + + // The refusal must also leave the data as it was. + H5File check = (H5File)(new H5File()).createInstance(target.toString(), FileFormat.READ); + check.open(); + try { + Dataset dataset = (Dataset)check.get("/vlen_of_compound"); + dataset.init(); + assertEquals("[[[10, 11], [20, 21]], [[30, 31]]]", render(dataset.getData())); + } + finally { + check.close(); + } + } +} diff --git a/object/src/test/java/object/TestVlenArrayDatatypes.java b/object/src/test/java/object/TestVlenArrayDatatypes.java new file mode 100644 index 000000000..b95977a1d --- /dev/null +++ b/object/src/test/java/object/TestVlenArrayDatatypes.java @@ -0,0 +1,291 @@ +package object; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.util.ArrayList; +import java.util.List; + +import hdf.object.Dataset; +import hdf.object.Datatype; +import hdf.object.FileFormat; +import hdf.object.HObject; +import hdf.object.h5.H5File; + +import hdf.hdf5lib.H5; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +/** + * Read and write tests for datatypes holding variable-length data underneath an + * ARRAY or a COMPOUND, using the shared UI test files. + * + * Assertions are on element values rather than only on buffer shape, since a + * correctly shaped buffer can still be filled with the wrong contents. + */ +@Tag("unit") +@DisplayName("Variable-length Array and Compound Datatype Tests") +public class TestVlenArrayDatatypes { + + private static final String TEST_DIR = "../hdfview/src/test/resources/uitest/"; + + /** The three strings stored in every element of tvlenstr_array.h5. */ + private static final String[] EXPECTED_VLEN_STRINGS = { + "This is a variable-length test string.", "This test string is also variable-length.", + "A final test of variable-length strings. This string is longer than the others."}; + + private H5File testFile; + + private static int openIDsAtStart; + + @BeforeAll + public static void recordOpenIDs() throws Exception + { + openIDsAtStart = H5.getOpenIDCount(); + } + + @AfterAll + public static void checkIDs() throws Exception + { + assertEquals(openIDsAtStart, H5.getOpenIDCount(), "HDF5 identifiers leaked by this test class"); + } + + @AfterEach + public void tearDown() throws Exception + { + if (testFile != null) { + try { + testFile.close(); + } + catch (Exception e) { + // Ignore close errors + } + } + } + + private H5File openTestFile(String filename) throws Exception + { + File file = new File(TEST_DIR + filename); + assertTrue(file.exists(), "Test file not found: " + file.getAbsolutePath()); + + testFile = new H5File(file.getAbsolutePath(), FileFormat.READ); + assertNotNull(testFile, "Failed to open test file"); + testFile.open(); + + return testFile; + } + + private Dataset getDataset(String filename, String datasetPath) throws Exception + { + HObject obj = openTestFile(filename).get(datasetPath); + assertNotNull(obj, "Dataset not found: " + datasetPath); + assertInstanceOf(Dataset.class, obj, "Object is not a dataset: " + datasetPath); + + Dataset dataset = (Dataset)obj; + dataset.init(); + + return dataset; + } + + /** Collects every String in a nested read buffer, in order. */ + private static void collectStrings(Object data, List out) + { + if (data instanceof String str) + out.add(str); + else if (data instanceof List list) { + for (Object element : list) + collectStrings(element, out); + } + else if (data != null && data.getClass().isArray()) { + int length = java.lang.reflect.Array.getLength(data); + for (int i = 0; i < length; i++) + collectStrings(java.lang.reflect.Array.get(data, i), out); + } + } + + private static List allStrings(Object data) + { + List out = new ArrayList<>(); + collectStrings(data, out); + return out; + } + + @Test + @DisplayName("Array of variable-length string") + public void testArrayOfVlenString() throws Exception + { + Dataset dataset = getDataset("tvlenstr_array.h5", "/ScalarArrayOfVlenStr"); + + Datatype dtype = dataset.getDatatype(); + assertTrue(dtype.isArray(), "Expected an ARRAY datatype"); + assertTrue(dtype.getDatatypeBase().isVarStr(), "Expected a variable-length string base type"); + + Object data = dataset.getData(); + assertNotNull(data, "Data read returned null"); + + assertInstanceOf(Object[].class, data, "Expected one buffer slot per selected point"); + Object[] points = (Object[])data; + assertEquals(5, points.length, "Expected 5 points"); + + for (int i = 0; i < points.length; i++) { + assertInstanceOf(List.class, points[i], "Point " + i + " should hold a List of elements"); + + List elements = (List)points[i]; + assertEquals(EXPECTED_VLEN_STRINGS.length, elements.size(), "Point " + i + " element count"); + + for (int j = 0; j < EXPECTED_VLEN_STRINGS.length; j++) + assertEquals(EXPECTED_VLEN_STRINGS[j], elements.get(j), "Point " + i + " element " + j); + } + } + + @Test + @DisplayName("Compound with an array of variable-length string member") + public void testCompoundWithArrayOfVlenStringMember() throws Exception + { + Dataset dataset = getDataset("tvlenstr_array.h5", "/CompoundArrayOfVlenStr"); + + Object data = dataset.getData(); + assertNotNull(data, "Data read returned null"); + + // One entry per member, one slot per point, each slot a List of elements. + assertInstanceOf(List.class, data, "Expected one entry per compound member"); + List members = (List)data; + assertEquals(1, members.size(), "Expected the single member vlen_str_array"); + + assertInstanceOf(Object[].class, members.get(0), "Expected one slot per selected point"); + Object[] points = (Object[])members.get(0); + assertEquals(5, points.length, "Expected 5 points"); + + for (int i = 0; i < points.length; i++) { + assertInstanceOf(List.class, points[i], "Point " + i + " should hold a List of elements"); + + List elements = (List)points[i]; + assertEquals(EXPECTED_VLEN_STRINGS.length, elements.size(), "Point " + i + " element count"); + + for (int j = 0; j < EXPECTED_VLEN_STRINGS.length; j++) + assertEquals(EXPECTED_VLEN_STRINGS[j], elements.get(j), "Point " + i + " element " + j); + } + } + + @Test + @DisplayName("Compound mixing an array of variable-length string with fixed members") + public void testCompoundComplexVlenStringMember() throws Exception + { + Dataset dataset = getDataset("tcompound_complex.h5", "/CompoundComplex"); + + Object data = dataset.getData(); + assertNotNull(data, "Data read returned null"); + + // b_name is an ARRAY[4] of varstr and c_name a fixed-length string; both are + // collected, so assert on known values rather than a count. + List strings = allStrings(data); + assertFalse(strings.isEmpty(), "Expected string members to be read"); + assertTrue(strings.contains("A fight is a contract that takes two people to honor."), + "Expected the first variable-length string of b_name"); + assertTrue(strings.contains(" -- Professor Cheng Man-ch'ing"), + "Expected the last variable-length string of b_name"); + } + + @Test + @DisplayName("Array of compound") + public void testArrayOfCompound() throws Exception + { + Dataset dataset = getDataset("tarray4.h5", "/Dataset1"); + + Datatype dtype = dataset.getDatatype(); + assertTrue(dtype.isArray(), "Expected an ARRAY datatype"); + assertTrue(dtype.getDatatypeBase().isCompound(), "Expected a compound base type"); + + Object data = dataset.getData(); + assertNotNull(data, "Data read returned null"); + + // No variable-length data, so the layout is one flat primitive array per member. + assertInstanceOf(List.class, data, "Expected one entry per compound member"); + List members = (List)data; + assertEquals(2, members.size(), "Expected members i and f"); + + assertInstanceOf(int[].class, members.get(0), "Member i should be a flat int[]"); + assertInstanceOf(float[].class, members.get(1), "Member f should be a flat float[]"); + assertEquals(16, ((int[])members.get(0)).length, "Expected 4 points x 4 array elements"); + } + + @ParameterizedTest(name = "{0}{1}") + @CsvSource({"tvlenstr_array.h5, /ScalarArrayOfVlenStr", "tvlenstr_array.h5, /CompoundArrayOfVlenStr", + "tarray4.h5, /Dataset1", "tcompound_complex.h5, /CompoundComplex", + "tcompound_complex2.h5, /CompoundComplex1D", "tstr.h5, /comp1"}) + @DisplayName("Nested datatype read") + public void + testNestedDatatypeRead(String filename, String datasetPath) throws Exception + { + Dataset dataset = getDataset(filename, datasetPath); + + Object data = + assertDoesNotThrow(() -> dataset.getData(), "Reading " + filename + datasetPath + " threw"); + assertNotNull(data, "Data read returned null for " + filename + datasetPath); + } + + @Test + @DisplayName("Array of variable-length string write") + public void testArrayOfVlenStringWriteRoundTrip() throws Exception + { + Path source = new File(TEST_DIR + "tvlenstr_array.h5").toPath(); + assertTrue(Files.exists(source), "Test file not found: " + source); + + /* + * Written under target/ rather than a JUnit temporary directory: HDF5 can still + * hold a file open when the temporary directory is torn down, which Windows + * refuses to delete and JUnit then reports as a failure. + */ + Path workDir = Path.of("target", "vlen-array-datatypes"); + Files.createDirectories(workDir); + + Path target = workDir.resolve("tvlenstr_array_rw.h5"); + Files.copy(source, target, StandardCopyOption.REPLACE_EXISTING); + + final String replacement = "A replacement variable-length string."; + + testFile = new H5File(target.toString(), FileFormat.WRITE); + testFile.open(); + try { + Dataset dataset = (Dataset)testFile.get("/ScalarArrayOfVlenStr"); + dataset.init(); + + Object data = dataset.getData(); + @SuppressWarnings("unchecked") + List firstPoint = (List)((Object[])data)[0]; + firstPoint.set(1, replacement); + + dataset.write(data); + } + finally { + testFile.close(); + testFile = null; + } + + testFile = new H5File(target.toString(), FileFormat.READ); + testFile.open(); + + Dataset reopened = (Dataset)testFile.get("/ScalarArrayOfVlenStr"); + reopened.init(); + + Object[] points = (Object[])reopened.getData(); + List firstPoint = (List)points[0]; + + assertEquals(EXPECTED_VLEN_STRINGS[0], firstPoint.get(0), "Untouched element changed"); + assertEquals(replacement, firstPoint.get(1), "Edited element did not round-trip"); + assertEquals(EXPECTED_VLEN_STRINGS[2], firstPoint.get(2), "Untouched element changed"); + + List secondPoint = (List)points[1]; + assertEquals(EXPECTED_VLEN_STRINGS[1], secondPoint.get(1), "A different point was modified"); + } +} From 8e86ea3a9675a15af4a14dd8322b07144e2402c3 Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Fri, 2 Oct 2026 15:31:41 -0500 Subject: [PATCH 09/11] Address review feedback --- .../view/TableView/DataProviderFactory.java | 61 +++++----- .../TableView/DefaultCompoundDSTableView.java | 115 ++++-------------- .../view/TableView/DataFactoryUtilsTest.java | 20 ++- .../main/java/hdf/object/h5/H5CompoundDS.java | 12 +- .../main/java/hdf/object/h5/H5Datatype.java | 9 +- .../main/java/hdf/object/h5/H5ScalarAttr.java | 2 +- .../main/java/hdf/object/h5/H5ScalarDS.java | 2 +- .../java/object/TestNestedDatatypeShapes.java | 21 ++-- .../java/object/TestVlenArrayDatatypes.java | 4 +- 9 files changed, 80 insertions(+), 166 deletions(-) diff --git a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java index fbce3b6cc..8d1a521ee 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java @@ -979,9 +979,6 @@ public void setDataValue(int columnIndex, int rowIndex, Object newValue) isValueChanged = true; log.trace("CompoundDataProvider.setDataValue: SUCCESS"); } - catch (UnsupportedOperationException uoe) { - throw uoe; - } catch (Exception ex) { log.debug("CompoundDataProvider.setDataValue({}, {})=({}): cell value update failure: ", rowIndex, columnIndex, newValue, ex); @@ -1049,9 +1046,6 @@ public void setDataValue(int columnIndex, int rowIndex, Object bufObject, Object isValueChanged = true; log.trace("=== COMPOUND setDataValue(bufObject) SUCCESS ==="); } - catch (UnsupportedOperationException uoe) { - throw uoe; - } catch (Exception ex) { log.trace("setDataValue({}, {}, {})=({}): cell value update failure: ", rowIndex, columnIndex, bufObject, newValue, ex); @@ -1101,7 +1095,9 @@ private static class ArrayDataProvider extends HDFDataProvider { baseTypeDataProvider = getDataProvider(baseType, dataBuf, dataTransposed); - if (baseType.isVarStr()) + isVarStrBase = baseType.isVarStr(); + + if (isVarStrBase) arraySize = dtype.getArrayDims()[0]; else if (baseType.isBitField() || baseType.isOpaque()) arraySize = dtype.getDatatypeSize(); @@ -1117,8 +1113,6 @@ else if (baseType.isBitField() || baseType.isOpaque()) nCols = (int)arraySize * ((CompoundDataProvider)baseTypeDataProvider).nCols; else nCols = super.getColumnCount(); - - isVarStrBase = baseType.isVarStr(); } /** @@ -1138,7 +1132,12 @@ private Object[] retrieveObjectModelElements(Object objBuf, int pointIndex) for (int i = 0; i < values.length; i++) { Object element = elements.get(i); - // Opaque, reference and bitfield elements arrive as raw bytes. + /* + * The JNI hands opaque and reference values over as byte[], which the base + * provider knows how to format. Elements at this level are normally Strings + * or Lists, since an array of opaque or reference holds no variable-length + * data and so never reaches this path; the check is defensive. + */ if (element instanceof byte[]) values[i] = baseTypeDataProvider.getDataValue(element, 0); else @@ -1308,6 +1307,14 @@ public Object getDataValue(Object obj, int index) @Override public boolean isCellEditable(int columnIndex, int rowIndex) { + /* + * A point held as a List is an array containing variable-length data, edited by + * updateObjectModelElements(). That splits the cell text on ",[]" into a flat run + * of tokens and stores each token as a String. Only an array of variable-length + * strings can be rebuilt that way: when the elements are themselves sequences + * or compounds, the tokenizer discards the brackets marking where each element + * ends, and the tokens are not converted to the element type. + */ int bufIndex = physicalLocationToBufIndex(rowIndex, columnIndex); if (dataBuf instanceof Object[] slots && bufIndex >= 0 && bufIndex < slots.length && slots[bufIndex] instanceof List) @@ -1328,9 +1335,6 @@ public void setDataValue(int columnIndex, int rowIndex, Object newValue) updateArrayElements(dataBuf, newValue, columnIndex, bufIndex); } - catch (UnsupportedOperationException uoe) { - throw uoe; - } catch (Exception ex) { log.debug("setDataValue({}, {}, {}): cell value update failure: ", rowIndex, columnIndex, newValue, ex); @@ -1349,9 +1353,6 @@ public void setDataValue(int columnIndex, int rowIndex, Object bufObject, Object updateArrayElements(bufObject, newValue, columnIndex, (int)bufIndex); } - catch (UnsupportedOperationException uoe) { - throw uoe; - } catch (Exception ex) { log.debug("setDataValue({}, {}, {}, {}): cell value update failure: ", rowIndex, columnIndex, bufObject, newValue, ex); @@ -1377,7 +1378,7 @@ private boolean updateObjectModelElements(Object curBuf, Object newValue, int po if (!(slots[pointIndex] instanceof List elements)) return false; - // The bracketed cell text is only unambiguous when the elements are scalars. + // The flat tokenizing below can only rebuild string elements; see isCellEditable(). if (!isVarStrBase) throw new UnsupportedOperationException( "editing an array of variable-length data is only supported for strings"); @@ -1607,9 +1608,9 @@ private Object[] retrieveArrayOfCompoundElements(Object objBuf, int columnIndex, * A vlen-of-compound member is one column showing the whole sequence. The read * path hands each row a list of compound elements already parsed into nested * Lists (e.g. [[10, [11, 12]], [20, [21, 22]]]). Return the row's elements as an - * array; VlenDataDisplayConverter wraps them in [...] and the inner - * CompoundDataDisplayConverter renders each element as {...} (recursing for - * nested compounds). + * array, which VlenDataDisplayConverter wraps in [...] and the inner + * CompoundDataDisplayConverter renders each element of as {...}, recursing + * for nested compounds. */ ArrayList vlElements = ((ArrayList[])objBuf)[rowIndex]; return vlElements.toArray(); @@ -1693,18 +1694,18 @@ public Object getDataValue(Object obj, int index) @Override public boolean isCellEditable(int columnIndex, int rowIndex) { - return !(baseTypeDataProvider instanceof CompoundDataProvider); + // A vlen-of-compound cell is the whole sequence, which no per-member write + // can be derived from. + if (baseTypeDataProvider instanceof CompoundDataProvider) + return false; + + // A sequence is only as editable as what it holds, e.g. a vlen of vlen of compound. + return baseTypeDataProvider.isCellEditable(columnIndex, rowIndex); } @Override public void setDataValue(int columnIndex, int rowIndex, Object newValue) { - // A vlen-of-compound cell is the whole sequence, which no per-member write - // can be derived from. - if (baseTypeDataProvider instanceof CompoundDataProvider) - throw new UnsupportedOperationException( - "editing a variable-length sequence of compound values is not supported"); - try { int bufIndex = physicalLocationToBufIndex(rowIndex, columnIndex); @@ -1724,12 +1725,6 @@ public void setDataValue(int columnIndex, int rowIndex, Object newValue) @Override public void setDataValue(int columnIndex, int rowIndex, Object bufObject, Object newValue) { - // A vlen-of-compound cell is the whole sequence, which no per-member write - // can be derived from. - if (baseTypeDataProvider instanceof CompoundDataProvider) - throw new UnsupportedOperationException( - "editing a variable-length sequence of compound values is not supported"); - try { long vlSize = Array.getLength(bufObject); log.trace("setDataValue(): vlSize={} for [c{}, r{}]", vlSize, columnIndex, rowIndex); diff --git a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java index dee9c9d8c..0c9f37db4 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java +++ b/hdfview/src/main/java/hdf/view/TableView/DefaultCompoundDSTableView.java @@ -692,15 +692,14 @@ public void handleLayerEvent(ILayerEvent e) CompoundDataFormat dataFormat = (CompoundDataFormat)dataObject; Datatype cmpdType = dataObject.getDatatype(); - // Resolve VLEN(compound) to compound base type - if (cmpdType.isVLEN() && !cmpdType.isVarStr() && cmpdType.getDatatypeBase() != null && - cmpdType.getDatatypeBase().isCompound()) { - cmpdType = cmpdType.getDatatypeBase(); - } - + // A top-level vlen is one column and has no members to filter, as in + // CompoundDSColumnHeaderDataProvider. Datatype[] selectedMemberTypes = dataFormat.getSelectedMemberTypes(); - List localSelectedTypes = - DataFactoryUtils.filterNonSelectedMembers(dataFormat, cmpdType); + List localSelectedTypes; + if (cmpdType.isVLEN() && !cmpdType.isVarStr()) + localSelectedTypes = new ArrayList<>(java.util.Collections.singletonList(cmpdType)); + else + localSelectedTypes = DataFactoryUtils.filterNonSelectedMembers(dataFormat, cmpdType); HashMap[] maps = null; try { @@ -752,38 +751,12 @@ public void handleLayerEvent(ILayerEvent e) log.trace("CompoundDSCellSelectionListener: CellSelected fieldIndex={}:{}", rowIdx, fieldIndex); - /* - * The display may cover more columns than the compound has members, so a - * column can fall outside the map. Reference detection is best-effort; - * the label and value field below are updated either way. - */ - Integer bIndexObj = baseIndexMap.get(fieldIndex - 1); - if (bIndexObj == null) { - log.debug("CompoundDSCellSelectionListener: no member mapping for column {}", - fieldIndex - 1); - } - else if (bIndexObj < 0 || bIndexObj >= selectedMemberTypes.length) { - log.debug("CompoundDSCellSelectionListener: member index {} out of range (max {})", - bIndexObj, selectedMemberTypes.length); - } - else { - int bIndex = bIndexObj; + int bIndex = baseIndexMap.get(fieldIndex - 1); + Datatype selectedType = selectedMemberTypes[bIndex]; - if (dataValue instanceof List) { - Object colValue = ((List)dataValue).get(bIndex); - if (colValue == null) - log.debug( - "CompoundDSCellSelectionListener: CellSelected colValue is null for Idx={}", - bIndex); - } - - Datatype selectedType = selectedMemberTypes[bIndex]; - - if (selectedType.isRef()) { - valIsRegRef = - (selectedType.getDatatypeSize() == HDF5Constants.H5R_DSET_REG_REF_BUF_SIZE); - valIsObjRef = (selectedType.getDatatypeSize() == HDF5Constants.H5R_OBJ_REF_BUF_SIZE); - } + if (selectedType.isRef()) { + valIsRegRef = (selectedType.getDatatypeSize() == HDF5Constants.H5R_DSET_REG_REF_BUF_SIZE); + valIsObjRef = (selectedType.getDatatypeSize() == HDF5Constants.H5R_OBJ_REF_BUF_SIZE); } int rowStart = ((RowHeaderDataProvider)rowHeaderDataProvider).start; @@ -979,8 +952,8 @@ else if (curDtype.isVLEN() && !curDtype.isVarStr()) { } else if (curDtype.isCompound()) { /* - * memberNames is flat leaf names, memberTypes top-level member types; - * each type consumes countLeafNames() of the names. + * memberNames is flat leaf names and memberTypes top-level member + * types, so each type consumes countLeafNames() of the names. */ ListIterator localIt = memberNames.listIterator(); int topIdx = 0; @@ -1013,35 +986,23 @@ else if (curDtype.isCompound()) { } /* - * For ARRAY of COMPOUND and VLEN of COMPOUND types, we repeat the compound - * members n times, where n is the number of array or vlen elements. + * For ARRAY of COMPOUND types, we repeat the compound members n times, + * where n is the number of array elements. The flat name list holds the + * header followed by each inner leaf name. */ if (nestedArrayOfCompound) { List selTypes = DataFactoryUtils.filterNonSelectedMembers( dataFormat, nestedArrayOfCompoundType, false); - List selMemberNames; - int namesConsumed; - if (curType.isVLEN()) { - // A vlen contributes only a header name, so synthesize the - // inner leaf names from it. - String baseName = curName.replaceAll(CompoundDS.SEPARATOR, "->"); - selMemberNames = buildInnerCompoundLeafNames(nestedArrayOfCompoundType, baseName); - namesConsumed = 1; - } - else { - // Array-of-compound: flat list has header + each inner leaf name. - selMemberNames = new ArrayList<>(selTypes.size()); - int arrCmpdLen = calcArrayOfCompoundLen(selTypes); - selMemberNames.add(curName); - for (int i = 1; i < arrCmpdLen; i++) - selMemberNames.add(localIt.next()); - namesConsumed = arrCmpdLen; - } + List selMemberNames = new ArrayList<>(selTypes.size()); + int arrCmpdLen = calcArrayOfCompoundLen(selTypes); + selMemberNames.add(curName); + for (int i = 1; i < arrCmpdLen; i++) + selMemberNames.add(localIt.next()); recursiveColumnHeaderSetup(outColNames, dataFormat, curType, selMemberNames, selTypes); - remainingLeavesInTop -= namesConsumed; + remainingLeavesInTop -= arrCmpdLen; } else if (curType.isVLEN() && !curType.isVarStr()) { // A vlen member is one column holding the whole sequence. @@ -1060,36 +1021,6 @@ else if (curType.isVLEN() && !curType.isVarStr()) { } } - /** Produce "prefix->leaf" names for each leaf of {@code innerCompound}. */ - private List buildInnerCompoundLeafNames(Datatype innerCompound, String prefix) - { - List out = new ArrayList<>(); - appendInnerCompoundLeafNames(innerCompound, prefix, out); - return out; - } - - private void appendInnerCompoundLeafNames(Datatype t, String prefix, List out) - { - if (t == null) - return; - if (t.isCompound()) { - List names = t.getCompoundMemberNames(); - List children = t.getCompoundMemberTypes(); - if (names == null || children == null) - return; - for (int i = 0; i < names.size(); i++) { - String childName = prefix + "->" + names.get(i); - Datatype childT = children.get(i); - if (childT != null && childT.isCompound()) - appendInnerCompoundLeafNames(childT, childName, out); - else - out.add(childName); - } - return; - } - out.add(prefix); - } - private int calcArrayOfCompoundLen(List datatypes) { int count = 0; diff --git a/hdfview/src/test/java/hdf/view/TableView/DataFactoryUtilsTest.java b/hdfview/src/test/java/hdf/view/TableView/DataFactoryUtilsTest.java index f89910992..77a94e3e5 100644 --- a/hdfview/src/test/java/hdf/view/TableView/DataFactoryUtilsTest.java +++ b/hdfview/src/test/java/hdf/view/TableView/DataFactoryUtilsTest.java @@ -36,8 +36,7 @@ /** * Tests for DataFactoryUtils leaf counting and member filtering against a compound - * of {int, vlen-of-int, vlen-of-compound}: an inner compound reached by recursion - * keeps all of its members, and a vlen counts as one leaf whatever it wraps. + * of {int, vlen-of-int, vlen-of-compound} */ public class DataFactoryUtilsTest { @@ -134,8 +133,7 @@ void countLeafNames_vlenOfAtomicIsOne() @Test void countLeafNames_vlenOfCompoundIsOne() { - // A vlen member is always a single column, regardless of what it wraps - - // it must NOT be flattened into its base compound's leaf count. + // A vlen member is always a single column, regardless of what it wraps Datatype nested = memberType("nested"); assertTrue(nested.isVLEN()); assertTrue(nested.getDatatypeBase().isCompound()); @@ -153,8 +151,8 @@ void countLeafNames_plainCompoundSumsItsChildren() @Test void countLeafNames_topLevelCompoundCountsTopLevelMembersOnly() { - // id(1) + tags:VLEN(1) + nested:VLEN(1) = 3 - NOT the fully-flattened leaf - // count (which would incorrectly be 1 + 1 + 2 = 4). + // id(1) + tags:VLEN(1) + nested:VLEN(1) = 3, not the fully-flattened leaf + // count assertEquals(3, DataFactoryUtils.countLeafNames(testDataset.getDatatype())); } @@ -162,10 +160,8 @@ void countLeafNames_topLevelCompoundCountsTopLevelMembersOnly() void countLeafNames_agreesWithExtractCompoundInfoFlatNameList() { // countLeafNames exists to mirror the flat leaf-name list that - // H5Datatype.extractCompoundInfo produces; recursiveColumnHeaderSetup walks that - // list using these counts to decide which top-level member each name belongs to. - // If the two ever disagree, column headers silently misalign - so assert the - // relationship itself rather than a hardcoded number. + // H5Datatype.extractCompoundInfo produces, which recursiveColumnHeaderSetup + // walks using these counts to decide which top-level member each name belongs to. assertEquals(testDataset.getSelectedMemberNames().length, DataFactoryUtils.countLeafNames(testDataset.getDatatype())); } @@ -173,7 +169,7 @@ void countLeafNames_agreesWithExtractCompoundInfoFlatNameList() @Test void filterNonSelectedMembers_innerCompoundKeepsAllMembers() { - // The dataset's selected-member list names only id/tags/nested, never p/q, so + // The dataset's selected-member list names only id/tags/nested, so // the isTopLevel=false path must skip the filter entirely. Datatype innerCompound = memberType("nested").getDatatypeBase(); @@ -207,7 +203,7 @@ void filterNonSelectedMembers_topLevelFiltersAgainstSelection() assertSame(idType, filtered.get(0)); } finally { - // testDataset is shared across test methods - leave selection as init() set it. + // testDataset is shared across test methods, so leave selection as init() set it. testDataset.setAllMemberSelection(true); } } diff --git a/object/src/main/java/hdf/object/h5/H5CompoundDS.java b/object/src/main/java/hdf/object/h5/H5CompoundDS.java index ed321bc2b..09f1b0c80 100644 --- a/object/src/main/java/hdf/object/h5/H5CompoundDS.java +++ b/object/src/main/java/hdf/object/h5/H5CompoundDS.java @@ -946,8 +946,8 @@ private Object compoundTypeIO(H5File.IO_TYPE ioType, long did, long[] spaceIDs, else if (cmpdType.isVLEN() && !cmpdType.isVarStr()) { /* * A top-level VLEN-of-compound is one column holding the whole sequence, so - * transfer it in a single call; the JNI parses each compound element into a - * nested List. The dataset enumerates a single member, hence one column. + * transfer it in a single call. The JNI parses each compound element into a + * nested List. */ if (ioType != H5File.IO_TYPE.READ) throw new UnsupportedOperationException( @@ -1174,7 +1174,6 @@ else if ( catch (Exception ex) { log.debug("compoundTypeIO(): failed to write member[{}]: ", i, ex); globalMemberIndex[0]++; - // The member was not persisted; do not report success. throw new Exception("failed to write compound member '" + memberName + "': " + ex.getMessage(), ex); @@ -1185,8 +1184,7 @@ else if ( catch (Exception ex) { log.debug("compoundTypeIO(): failure: ", ex); memberDataList = null; - // A failed write leaves data unpersisted and must be reported; a - // failed read yields a null compound. + // A failed write leaves data unpersisted and must be reported if (ioType == H5File.IO_TYPE.WRITE) throw ex; } @@ -1249,7 +1247,7 @@ private Object readSingleCompoundMember(long dsetID, long[] spaceIDs, int nSelPo (spaceIDs[0] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[0], (spaceIDs[1] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[1]); - // Slots are left null; H5DreadVL allocates each list. + // Slots are left null since the read allocates each list. @SuppressWarnings("rawtypes") ArrayList[] vlBuf = new ArrayList[nSelPoints]; @@ -1298,7 +1296,7 @@ else if (H5Datatype.containsVlenData(memberType)) { /* * The single-field compound transfer type wraps each row in a - * one-element record holding the member's value; unwrap it. + * one-element record holding the member's value, so unwrap it here. */ Object[] rows = (Object[])memberData; for (int r = 0; r < rows.length; r++) { diff --git a/object/src/main/java/hdf/object/h5/H5Datatype.java b/object/src/main/java/hdf/object/h5/H5Datatype.java index 3b31667ae..4c6ac5ae8 100644 --- a/object/src/main/java/hdf/object/h5/H5Datatype.java +++ b/object/src/main/java/hdf/object/h5/H5Datatype.java @@ -2197,18 +2197,18 @@ else if ((typeClass == CLASS_STRING) || (typeClass == CLASS_REFERENCE)) { else if (dtype.isVLEN()) { log.trace("allocateArray(): isVLEN"); - // Slots are left null; the read routines allocate each list. + // Slots are left null since the read routines allocate each list. data = new ArrayList[numPoints]; } else if (typeClass == CLASS_ARRAY) { log.trace("allocateArray(): class CLASS_ARRAY"); /* - * Per the JNI buffer data model (see "Buffer data model" in H5.java), an + * Per the JNI buffer data model, an * array containing variable-length data is read as one slot per selected * point, each holding an ArrayList of the array's elements. The read * routines allocate those lists, so the slots are left null, and the - * container must be Object[] rather than a narrowly-typed array. + * container must be the generic Object[]. */ if (containsVlenData(dtype)) { log.trace("allocateArray(): CLASS_ARRAY contains variable-length data"); @@ -3019,7 +3019,8 @@ else if (dtype.isVLEN() && !dtype.isVarStr()) { /* * A vlen (including vlen-of-compound) is a single leaf: it is displayed as one * column showing the whole sequence as a string. Do NOT recurse into the base - * compound - that would enumerate the inner members as separate columns. + * compound, since that would enumerate the inner members as separate + * columns. */ log.trace("extractCompoundInfo(): variable-length type - adding as a single leaf"); if (names != null) diff --git a/object/src/main/java/hdf/object/h5/H5ScalarAttr.java b/object/src/main/java/hdf/object/h5/H5ScalarAttr.java index 784ef2c15..ef4de39f5 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarAttr.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarAttr.java @@ -1210,7 +1210,7 @@ else if (dsDatatype.isCompound()) { else if (dsDatatype.isVLEN()) { log.trace("attributeCommonIO():read ioType:VLEN-REF H5Aread isArray()={}", dsDatatype.isArray()); - // Slots are left null; H5AreadVL allocates each list. + // Slots are left null since the read allocates each list. theData = new ArrayList[(int)lsize]; try { diff --git a/object/src/main/java/hdf/object/h5/H5ScalarDS.java b/object/src/main/java/hdf/object/h5/H5ScalarDS.java index c42ba7787..1ae27712f 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarDS.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarDS.java @@ -940,7 +940,7 @@ private Object scalarDatasetCommonIO(H5File.IO_TYPE ioType, Object writeBuf) thr } } else if (dsDatatype.isVLEN()) { - // Slots are left null; H5DreadVL allocates each list. + // Slots are left null since the read allocates each list. theData = new ArrayList[(int)totalSelectedSpacePoints]; } else if ((originalBuf == null) || dsDatatype.isEnum() || dsDatatype.isText() || diff --git a/object/src/test/java/object/TestNestedDatatypeShapes.java b/object/src/test/java/object/TestNestedDatatypeShapes.java index 258d75cb7..7a14091c8 100644 --- a/object/src/test/java/object/TestNestedDatatypeShapes.java +++ b/object/src/test/java/object/TestNestedDatatypeShapes.java @@ -22,22 +22,15 @@ import org.junit.jupiter.api.Test; /** - * Read tests for nested datatype shapes not covered by the shared UI test files. + * Read tests for nested datatype shapes. * - * Fixtures are built at test time rather than committed as binaries, so adding a - * shape costs a few lines here. They are written with raw H5Dwrite/H5DwriteVL calls - * against the JNI's buffer data model; that path does not reach - * H5Datatype.allocateArray(), so the setup is independent of what is being read. + * Fixtures are built at test time, written with raw H5Dwrite/H5DwriteVL calls + * against the JNI's buffer data model. */ @Tag("unit") @DisplayName("Nested Datatype Shape Read Tests") public class TestNestedDatatypeShapes { - /* - * Written under target/ rather than a JUnit temporary directory: HDF5 can still - * hold a file open when the temporary directory is torn down, which Windows - * refuses to delete and JUnit then reports as a failure. - */ private static Path workDir; private static final int WIDE_POINTS = 50; @@ -87,7 +80,7 @@ static void closeFile() throws Exception assertEquals(openIDsAtStart, H5.getOpenIDCount(), "HDF5 identifiers leaked by this test class"); } - /** Variable-length string type; the caller closes it. */ + /** Variable-length string type, which the caller closes. */ private static long varStrType() throws Exception { long tid = H5.H5Tcopy(HDF5Constants.H5T_C_S1); @@ -95,7 +88,7 @@ private static long varStrType() throws Exception return tid; } - /** Compound {p:int, q:int}; the caller closes it. */ + /** Compound {p:int, q:int}, which the caller closes. */ private static long pqCompoundType() throws Exception { long tid = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, 8); @@ -252,7 +245,7 @@ private static void writeCompoundOfVlenCompound(long fid) throws Exception } } - /** VLEN of a one-character fixed-length string - a string stored as a sequence. */ + /** VLEN of a one-character fixed-length string, as a string stored as a sequence. */ private static void writeVlenOfFixedString(long fid) throws Exception { long st = H5.H5Tcopy(HDF5Constants.H5T_C_S1); @@ -267,7 +260,7 @@ private static void writeVlenOfFixedString(long fid) throws Exception } } - /** ARRAY[3] of fixed-length string - no variable-length data anywhere. */ + /** ARRAY[3] of fixed-length string, so no variable-length data anywhere. */ private static void writeArrayOfFixedString(long fid) throws Exception { long st = H5.H5Tcopy(HDF5Constants.H5T_C_S1); diff --git a/object/src/test/java/object/TestVlenArrayDatatypes.java b/object/src/test/java/object/TestVlenArrayDatatypes.java index b95977a1d..0ef9d7c71 100644 --- a/object/src/test/java/object/TestVlenArrayDatatypes.java +++ b/object/src/test/java/object/TestVlenArrayDatatypes.java @@ -186,8 +186,8 @@ public void testCompoundComplexVlenStringMember() throws Exception Object data = dataset.getData(); assertNotNull(data, "Data read returned null"); - // b_name is an ARRAY[4] of varstr and c_name a fixed-length string; both are - // collected, so assert on known values rather than a count. + // b_name is an ARRAY[4] of varstr and c_name a fixed-length string, and both + // are collected, so assert on known values rather than a count. List strings = allStrings(data); assertFalse(strings.isEmpty(), "Expected string members to be read"); assertTrue(strings.contains("A fight is a contract that takes two people to honor."), From 5327e98c4014da30c74df65aa895f5303c61c41f Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Mon, 5 Oct 2026 12:07:00 -0500 Subject: [PATCH 10/11] Allocate all variable-length data as one slot per point --- .../view/TableView/DataProviderFactory.java | 22 +-- .../view/dialog/NewScalarAttributeDialog.java | 5 + .../main/java/hdf/object/h5/H5CompoundDS.java | 22 ++- .../main/java/hdf/object/h5/H5Datatype.java | 63 ++++---- .../main/java/hdf/object/h5/H5ScalarAttr.java | 36 ++--- .../main/java/hdf/object/h5/H5ScalarDS.java | 6 +- .../java/object/TestNestedDatatypeShapes.java | 135 ++++++++++++++++++ 7 files changed, 198 insertions(+), 91 deletions(-) diff --git a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java index 8d1a521ee..b4124522b 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java @@ -1612,14 +1612,14 @@ private Object[] retrieveArrayOfCompoundElements(Object objBuf, int columnIndex, * CompoundDataDisplayConverter renders each element of as {...}, recursing * for nested compounds. */ - ArrayList vlElements = ((ArrayList[])objBuf)[rowIndex]; + ArrayList vlElements = (ArrayList)((Object[])objBuf)[rowIndex]; return vlElements.toArray(); } private Object[] retrieveArrayOfArrayElements(Object objBuf, int columnIndex, int startRowIndex) { log.trace("retrieveArrayOfArrayElements(): objBuf={}", objBuf); - ArrayList vlElements = ((ArrayList[])objBuf)[startRowIndex]; + ArrayList vlElements = (ArrayList)((Object[])objBuf)[startRowIndex]; log.trace("retrieveArrayOfArrayElements(): vlElements={}", vlElements); long vlSize = vlElements.size(); log.trace("retrieveArrayOfArrayElements(): vlSize={} length={}", vlSize, vlElements.size()); @@ -1646,7 +1646,7 @@ private Object[] retrieveArrayOfArrayElements(Object objBuf, int columnIndex, in private Object[] retrieveArrayOfComplexElements(Object objBuf, int columnIndex, int startRowIndex) { log.trace("retrieveArrayOfComplexElements(): objBuf={}", objBuf); - ArrayList vlElements = ((ArrayList[])objBuf)[startRowIndex]; + ArrayList vlElements = (ArrayList)((Object[])objBuf)[startRowIndex]; log.trace("retrieveArrayOfComplexElements(): vlElements={}", vlElements); long vlSize = vlElements.size(); log.trace("retrieveArrayOfComplexElements(): vlSize={} length={}", vlSize, vlElements.size()); @@ -1673,7 +1673,7 @@ private Object[] retrieveArrayOfComplexElements(Object objBuf, int columnIndex, private Object[] retrieveArrayOfAtomicElements(Object objBuf, int rowStartIdx) { - ArrayList vlElements = ((ArrayList[])objBuf)[rowStartIdx]; + ArrayList vlElements = (ArrayList)((Object[])objBuf)[rowStartIdx]; long vlSize = vlElements.size(); log.trace("retrieveArrayOfAtomicElements(): vlSize={}", vlSize); Object[] tempArray = new Object[(int)vlSize]; @@ -1771,7 +1771,7 @@ private void updateArrayOfCompoundElements(Object newValue, Object curBuf, int c private void updateArrayOfArrayElements(Object newValue, Object curBuf, int columnIndex, int rowIndex) { - ArrayList vlElements = ((ArrayList[])curBuf)[rowIndex]; + ArrayList vlElements = (ArrayList)((Object[])curBuf)[rowIndex]; log.trace("updateArrayOfArrayElements(): vlElements={}", vlElements); long vlSize = vlElements.size(); log.trace("updateArrayOfArrayElements(): vlSize={}", vlSize); @@ -1810,12 +1810,12 @@ private void updateArrayOfArrayElements(Object newValue, Object curBuf, int colu isValueChanged = isValueChanged || baseTypeDataProvider.getIsValueChanged(); } vlElements = new ArrayList<>(Arrays.asList(abuffer)); - ((ArrayList[])curBuf)[rowIndex] = vlElements; + ((Object[])curBuf)[rowIndex] = vlElements; } private void updateArrayOfAtomicElements(Object newValue, Object curBuf, int rowStartIdx) { - ArrayList vlElements = ((ArrayList[])curBuf)[rowStartIdx]; + ArrayList vlElements = (ArrayList)((Object[])curBuf)[rowStartIdx]; long vlSize = vlElements.size(); log.trace("updateArrayOfAtomicElements(): vlSize={}", vlSize); @@ -1857,7 +1857,7 @@ private void updateArrayOfAtomicElements(Object newValue, Object curBuf, int row log.trace("updateArrayOfAtomicElements(): abuffer cname={} of data cname={}", bname, cname); vlElements = new ArrayList<>(Arrays.asList(abuffer)); log.trace("updateArrayOfAtomicElements(): new vlSize={}", vlElements.size()); - ((ArrayList[])curBuf)[rowStartIdx] = vlElements; + ((Object[])curBuf)[rowStartIdx] = vlElements; } } @@ -1927,7 +1927,7 @@ public void setDataValue(int index, Object bufObject, Object newValue) private void updateStringBytes(Object curBuf, Object newValue, int bufStartIndex) { - if (curBuf instanceof String[]) { + if (curBuf instanceof Object[]) { Array.set(curBuf, bufStartIndex, newValue); } else if (curBuf instanceof byte[]) { @@ -2485,7 +2485,7 @@ private void updateArrayElements(Object curBuf, Object newValue, int columnIndex private void updateArrayOfAtomicElements(Object newValue, Object curBuf, int rowStartIdx) { - ArrayList vlElements = ((ArrayList[])curBuf)[rowStartIdx]; + ArrayList vlElements = (ArrayList)((Object[])curBuf)[rowStartIdx]; StringTokenizer st = new StringTokenizer((String)newValue, "+i"); int newcnt = st.countTokens(); @@ -2499,7 +2499,7 @@ private void updateArrayOfAtomicElements(Object newValue, Object curBuf, int row log.trace("updateArrayOfAtomicElements(): buffer cname={} of data cname={}", bname, cname); vlElements = new ArrayList<>(Arrays.asList(abuffer)); log.trace("updateArrayOfAtomicElements(): new vlSize={}", vlElements.size()); - ((ArrayList[])curBuf)[rowStartIdx] = vlElements; + ((Object[])curBuf)[rowStartIdx] = vlElements; } } } diff --git a/hdfview/src/main/java/hdf/view/dialog/NewScalarAttributeDialog.java b/hdfview/src/main/java/hdf/view/dialog/NewScalarAttributeDialog.java index 278b375cd..dcf2cf19e 100644 --- a/hdfview/src/main/java/hdf/view/dialog/NewScalarAttributeDialog.java +++ b/hdfview/src/main/java/hdf/view/dialog/NewScalarAttributeDialog.java @@ -14,6 +14,7 @@ package hdf.view.dialog; +import java.util.Arrays; import java.util.List; import java.util.StringTokenizer; @@ -322,6 +323,10 @@ private boolean createAttribute() else attr = (Attribute) new H5ScalarAttr(parentObj, attrName, datatype, dims); Object value = H5Datatype.allocateArray(datatype, (int)lsize); + // allocateArray leaves variable-length string slots null; start the new + // attribute with empty strings instead. + if (datatype.isVarStr()) + Arrays.fill((Object[])value, ""); attr.setAttributeData(value); log.trace("writeMetadata() via write()"); diff --git a/object/src/main/java/hdf/object/h5/H5CompoundDS.java b/object/src/main/java/hdf/object/h5/H5CompoundDS.java index 09f1b0c80..8ce822694 100644 --- a/object/src/main/java/hdf/object/h5/H5CompoundDS.java +++ b/object/src/main/java/hdf/object/h5/H5CompoundDS.java @@ -956,8 +956,7 @@ else if (cmpdType.isVLEN() && !cmpdType.isVarStr()) { long wholeTid = -1; try { wholeTid = H5.H5Dget_type(did); - @SuppressWarnings("rawtypes") - ArrayList[] vlBuf = new ArrayList[nSelPoints]; + Object[] vlBuf = (Object[])H5Datatype.allocateArray(cmpdType, nSelPoints); H5.H5DreadVL(did, wholeTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, vlBuf); globalMemberIndex[0]++; theData = vlBuf; @@ -1247,9 +1246,7 @@ private Object readSingleCompoundMember(long dsetID, long[] spaceIDs, int nSelPo (spaceIDs[0] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[0], (spaceIDs[1] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[1]); - // Slots are left null since the read allocates each list. - @SuppressWarnings("rawtypes") - ArrayList[] vlBuf = new ArrayList[nSelPoints]; + Object[] vlBuf = (Object[])H5Datatype.allocateArray(dsDatatype, nSelPoints); H5.H5DreadVL(dsetID, compTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, vlBuf); @@ -1386,17 +1383,17 @@ else if (memberType.isVarStr() && memberName.contains(CompoundDS.SEPARATOR)) { } /* - * Converts VLEN data (ArrayList[]) returned by H5DreadVL into a String[] where each + * Converts VLEN data (one List per point) returned by H5DreadVL into a String[] where each * element is a brace-enclosed, comma-separated list of values for that row. * E.g., for VLEN(compound{x:f64}) with row 0 having two records (1.0, 3.0), * the result for row 0 would be "{1.0, 3.0}". */ @SuppressWarnings("rawtypes") - private String[] convertVlenMemberToStrings(ArrayList[] vlBuf, int nSelPoints, H5Datatype memberType) + private String[] convertVlenMemberToStrings(Object[] vlBuf, int nSelPoints, H5Datatype memberType) { String[] result = new String[nSelPoints]; for (int j = 0; j < nSelPoints; j++) { - ArrayList vlElements = vlBuf[j]; + List vlElements = (List)vlBuf[j]; StringBuilder sb = new StringBuilder("{"); for (int k = 0; k < vlElements.size(); k++) { @@ -1486,7 +1483,8 @@ private void writeSingleCompoundMember(long dsetID, long[] spaceIDs, int nSelPoi tmpData = convertToUnsignedC(theData, null); } } - else if (memberType.isString() && (Array.get(theData, 0) instanceof String)) { + else if (memberType.isString() && !memberType.isVarStr() && + (Array.get(theData, 0) instanceof String)) { log.trace("writeSingleCompoundMember(): converting string array to byte array"); tmpData = stringToByte((String[])theData, (int)memberType.getDatatypeSize()); } @@ -1525,12 +1523,12 @@ else if (memberType.isEnum() && (Array.get(theData, 0) instanceof String)) { try { if (memberType.isVarStr()) { log.trace( - "writeSingleCompoundMember(): H5Dwrite_string did={} compTid={} spaceIDs[0]={} spaceIDs[1]={}", + "writeSingleCompoundMember(): H5Dwrite_VLStrings did={} compTid={} spaceIDs[0]={} spaceIDs[1]={}", dsetID, compTid, (spaceIDs[0] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[0], (spaceIDs[1] == HDF5Constants.H5P_DEFAULT) ? "H5P_DEFAULT" : spaceIDs[1]); - H5.H5Dwrite_string(dsetID, compTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, - (String[])tmpData); + H5.H5Dwrite_VLStrings(dsetID, compTid, spaceIDs[0], spaceIDs[1], HDF5Constants.H5P_DEFAULT, + (Object[])tmpData); } else { log.trace( diff --git a/object/src/main/java/hdf/object/h5/H5Datatype.java b/object/src/main/java/hdf/object/h5/H5Datatype.java index 4c6ac5ae8..21d19d4ab 100644 --- a/object/src/main/java/hdf/object/h5/H5Datatype.java +++ b/object/src/main/java/hdf/object/h5/H5Datatype.java @@ -2062,12 +2062,16 @@ public static final Object allocateArray(final H5Datatype dtype, int numPoints) log.trace("allocateArray(): tclass={} : tsize={}", typeClass, typeSize); - if (dtype.isVarStr()) { - log.trace("allocateArray(): is_variable_str={}", dtype.isVarStr()); - - data = new String[numPoints]; - for (int i = 0; i < numPoints; i++) - ((String[])data)[i] = ""; + /* + * Variable-length data follows the JNI buffer data model (see "Buffer data model" in + * H5.java): one slot per selected point, which the read routines fill with a String + * (variable-length string) or a nested ArrayList (vlen, or an array containing + * variable-length data). The slots are left null. A compound keeps its per-member + * List below, since compound members are read one at a time. + */ + if (containsVlenData(dtype) && typeClass != CLASS_COMPOUND) { + log.trace("allocateArray(): contains variable-length data"); + data = new Object[numPoints]; } else if (typeClass == CLASS_INTEGER) { log.trace("allocateArray(): class CLASS_INTEGER"); @@ -2194,45 +2198,26 @@ else if ((typeClass == CLASS_STRING) || (typeClass == CLASS_REFERENCE)) { data = new byte[(int)(numPoints * typeSize)]; } - else if (dtype.isVLEN()) { - log.trace("allocateArray(): isVLEN"); - - // Slots are left null since the read routines allocate each list. - data = new ArrayList[numPoints]; - } else if (typeClass == CLASS_ARRAY) { log.trace("allocateArray(): class CLASS_ARRAY"); - /* - * Per the JNI buffer data model, an - * array containing variable-length data is read as one slot per selected - * point, each holding an ArrayList of the array's elements. The read - * routines allocate those lists, so the slots are left null, and the - * container must be the generic Object[]. - */ - if (containsVlenData(dtype)) { - log.trace("allocateArray(): CLASS_ARRAY contains variable-length data"); - data = new Object[numPoints]; - } - else { - try { - log.trace("allocateArray(): ArrayRank={}", dtype.getArrayDims().length); - - // Use the base datatype to define the array - long[] arrayDims = dtype.getArrayDims(); - int asize = numPoints; - for (int j = 0; j < arrayDims.length; j++) { - log.trace("allocateArray(): Array dims[{}]={}", j, arrayDims[j]); + try { + log.trace("allocateArray(): ArrayRank={}", dtype.getArrayDims().length); - asize *= arrayDims[j]; - } + // Use the base datatype to define the array + long[] arrayDims = dtype.getArrayDims(); + int asize = numPoints; + for (int j = 0; j < arrayDims.length; j++) { + log.trace("allocateArray(): Array dims[{}]={}", j, arrayDims[j]); - if (baseType != null) - data = H5Datatype.allocateArray(baseType, asize); - } - catch (Exception ex) { - log.debug("allocateArray(): CLASS_ARRAY class failure: ", ex); + asize *= arrayDims[j]; } + + if (baseType != null) + data = H5Datatype.allocateArray(baseType, asize); + } + catch (Exception ex) { + log.debug("allocateArray(): CLASS_ARRAY class failure: ", ex); } } else if (typeClass == CLASS_COMPLEX) { diff --git a/object/src/main/java/hdf/object/h5/H5ScalarAttr.java b/object/src/main/java/hdf/object/h5/H5ScalarAttr.java index ef4de39f5..bd9567a51 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarAttr.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarAttr.java @@ -1180,19 +1180,22 @@ public Object attributeCommonIO(long attrID, H5File.IO_TYPE ioType, Object objBu log.trace("attributeCommonIO():read ioType dtSize={} lsize={}", dtSize, lsize); try { - if (dsDatatype.isVarStr()) { - String[] strs = new String[(int)lsize]; - for (int j = 0; j < lsize; j++) - strs[j] = ""; + if (H5Datatype.containsVlenData(dsDatatype) && !dsDatatype.isCompound()) { + theData = H5Datatype.allocateArray(dsDatatype, (int)lsize); try { - log.trace("attributeCommonIO():read ioType H5Aread_VLStrings"); - H5.H5Aread_VLStrings(attrID, tid, strs); + if (dsDatatype.isVarStr()) { + log.trace("attributeCommonIO():read ioType H5Aread_VLStrings"); + H5.H5Aread_VLStrings(attrID, tid, (Object[])theData); + } + else { + log.trace("attributeCommonIO():read ioType H5AreadVL"); + H5.H5AreadVL(attrID, tid, (Object[])theData); + } } catch (Exception ex) { - log.debug("attributeCommonIO():read ioType H5Aread_VLStrings failure: ", ex); + log.debug("attributeCommonIO():read ioType variable-length failure: ", ex); ex.printStackTrace(); } - theData = strs; } else if (dsDatatype.isCompound()) { String[] strs = new String[(int)lsize]; @@ -1207,20 +1210,6 @@ else if (dsDatatype.isCompound()) { } theData = strs; } - else if (dsDatatype.isVLEN()) { - log.trace("attributeCommonIO():read ioType:VLEN-REF H5Aread isArray()={}", - dsDatatype.isArray()); - // Slots are left null since the read allocates each list. - theData = new ArrayList[(int)lsize]; - - try { - H5.H5AreadVL(attrID, tid, (Object[])theData); - } - catch (Exception ex) { - log.debug("attributeCommonIO():read ioType:VLEN-REF H5Aread failure: ", ex); - ex.printStackTrace(); - } - } else { Object attrData = null; try { @@ -1351,8 +1340,7 @@ else if (dsDatatype.isFloat() && dsDatatype.getDatatypeSize() == 16) { H5.H5Awrite_VLStrings(attrID, tid, (Object[])tmpData); } - else if (dsDatatype.isVLEN() || - (dsDatatype.isArray() && dsDatatype.getDatatypeBase().isVLEN())) { + else if (H5Datatype.containsVlenData(dsDatatype)) { log.trace("attributeCommonIO(): H5AwriteVL aid={} tid={}", attrID, tid); H5.H5AwriteVL(attrID, tid, (Object[])tmpData); diff --git a/object/src/main/java/hdf/object/h5/H5ScalarDS.java b/object/src/main/java/hdf/object/h5/H5ScalarDS.java index 1ae27712f..74bf86658 100644 --- a/object/src/main/java/hdf/object/h5/H5ScalarDS.java +++ b/object/src/main/java/hdf/object/h5/H5ScalarDS.java @@ -930,7 +930,7 @@ private Object scalarDatasetCommonIO(H5File.IO_TYPE ioType, Object writeBuf) thr "scalarDatasetCommonIO():read ioType isNamed={} isEnum={} isText={} isRefObj={}", dsDatatype.isNamed(), dsDatatype.isEnum(), dsDatatype.isText(), dsDatatype.isRefObj()); - if (dsDatatype.isVarStr()) { + if (H5Datatype.containsVlenData(dsDatatype)) { try { theData = H5Datatype.allocateArray(dsDatatype, (int)totalSelectedSpacePoints); } @@ -939,10 +939,6 @@ private Object scalarDatasetCommonIO(H5File.IO_TYPE ioType, Object writeBuf) thr throw new HDF5Exception("Out Of Memory"); } } - else if (dsDatatype.isVLEN()) { - // Slots are left null since the read allocates each list. - theData = new ArrayList[(int)totalSelectedSpacePoints]; - } else if ((originalBuf == null) || dsDatatype.isEnum() || dsDatatype.isText() || dsDatatype.isRefObj() || ((originalBuf != null) && (totalSelectedSpacePoints != nPoints))) { diff --git a/object/src/test/java/object/TestNestedDatatypeShapes.java b/object/src/test/java/object/TestNestedDatatypeShapes.java index 7a14091c8..c552cdfa5 100644 --- a/object/src/test/java/object/TestNestedDatatypeShapes.java +++ b/object/src/test/java/object/TestNestedDatatypeShapes.java @@ -62,6 +62,8 @@ static void createFile() throws Exception writeArrayOfFixedString(fid); writeWideVlenOfCompound(fid); writeCompoundWithReference(fid); + writeVarStr(fid); + writeArrayOfVlenIntAttribute(fid); } finally { H5.H5Fclose(fid); @@ -374,6 +376,61 @@ private static String render(Object data) return String.valueOf(data); } + /** Variable-length string. */ + private static void writeVarStr(long fid) throws Exception + { + long tid = varStrType(); + long sid = H5.H5Screate_simple(1, new long[] {2}, null); + long did = H5.H5Dcreate(fid, "varstr", tid, sid, HDF5Constants.H5P_DEFAULT, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT); + try { + H5.H5Dwrite_VLStrings(did, tid, HDF5Constants.H5S_ALL, HDF5Constants.H5S_ALL, + HDF5Constants.H5P_DEFAULT, new Object[] {"x,y", "z"}); + } + finally { + H5.H5Dclose(did); + H5.H5Sclose(sid); + H5.H5Tclose(tid); + } + } + + /** ARRAY[2] of VLEN of int, as an attribute on the root group. */ + private static void writeArrayOfVlenIntAttribute(long fid) throws Exception + { + long vt = H5.H5Tvlen_create(HDF5Constants.H5T_NATIVE_INT); + long tid = H5.H5Tarray_create(vt, 1, new long[] {2}); + long sid = H5.H5Screate_simple(1, new long[] {2}, null); + long aid = H5.H5Acreate(fid, "array_of_vlen_int_attr", tid, sid, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT); + try { + Object[] buf = {list(list(1, 2), list(3)), list(list(4), list(5, 6))}; + H5.H5AwriteVL(aid, tid, buf); + } + finally { + H5.H5Aclose(aid); + H5.H5Sclose(sid); + H5.H5Tclose(tid); + H5.H5Tclose(vt); + } + } + + /** COMPOUND{id:int, name:variable-length string}. */ + private static void writeCompoundWithVarStr(long fid) throws Exception + { + long vs = varStrType(); + long tid = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, 8 + H5.H5Tget_size(vs)); + try { + H5.H5Tinsert(tid, "id", 0, HDF5Constants.H5T_NATIVE_INT); + H5.H5Tinsert(tid, "name", 8, vs); + Object[] buf = {list(1, "first"), list(2, "second")}; + writeDataset(fid, "compound_with_varstr", tid, 2, buf, true); + } + finally { + H5.H5Tclose(tid); + H5.H5Tclose(vs); + } + } + // ---- tests ---------------------------------------------------------------- @Test @@ -542,4 +599,82 @@ public void testVlenOfCompoundWriteRefused() throws Exception check.close(); } } + + @Test + @DisplayName("Variable-length data is read into one slot per point") + public void testVariableLengthBufferShape() throws Exception + { + /* + * Every type containing variable-length data shares one buffer shape, the JNI's: + * an Object[] with one slot per point, holding a String or a nested List. + */ + String[][] cases = {{"varstr", "String"}, {"vlen_of_varstr", "List"}, {"array_of_vlen_int", "List"}}; + for (String[] c : cases) { + Object data = open(c[0]).getData(); + assertEquals(Object[].class, data.getClass(), c[0] + " buffer class"); + assertEquals(2, ((Object[])data).length, c[0] + " slot count"); + Object slot = ((Object[])data)[0]; + if (c[1].equals("String")) + assertInstanceOf(String.class, slot, c[0] + " slot type"); + else + assertInstanceOf(List.class, slot, c[0] + " slot type"); + } + assertEquals("[x,y, z]", render(open("varstr").getData())); + } + + @Test + @DisplayName("Attribute holding an array of VLEN of int") + public void testArrayOfVlenIntAttribute() throws Exception + { + hdf.object.Group root = (hdf.object.Group)testFile.get("/"); + hdf.object.Attribute attr = null; + for (Object a : ((hdf.object.MetaDataContainer)root).getMetadata()) + if (((hdf.object.Attribute)a).getAttributeName().equals("array_of_vlen_int_attr")) + attr = (hdf.object.Attribute)a; + assertNotNull(attr, "Attribute not found"); + + assertEquals("[[[1, 2], [3]], [[4], [5, 6]]]", render(attr.getAttributeData())); + } + + @Test + @DisplayName("Write a variable-length string compound member") + public void testCompoundVarStrMemberWriteRoundTrip() throws Exception + { + Path target = workDir.resolve("compound_varstr_rw.h5"); + long fid = H5.H5Fcreate(target.toString(), HDF5Constants.H5F_ACC_TRUNC, + HDF5Constants.H5P_DEFAULT, HDF5Constants.H5P_DEFAULT); + try { + writeCompoundWithVarStr(fid); + } + finally { + H5.H5Fclose(fid); + } + + H5File rw = (H5File)(new H5File()).createInstance(target.toString(), FileFormat.WRITE); + rw.open(); + try { + Dataset dataset = (Dataset)rw.get("/compound_with_varstr"); + dataset.init(); + @SuppressWarnings("unchecked") + List members = (List)dataset.getData(); + Object[] names = (Object[])members.get(1); + assertEquals("first", names[0]); + names[0] = "changed"; + dataset.write(members); + } + finally { + rw.close(); + } + + H5File check = (H5File)(new H5File()).createInstance(target.toString(), FileFormat.READ); + check.open(); + try { + Dataset dataset = (Dataset)check.get("/compound_with_varstr"); + dataset.init(); + assertEquals("[[1, 2], [changed, second]]", render(dataset.getData())); + } + finally { + check.close(); + } + } } From 7d0e18dfc2fadce199678fc93bac06b2858c728b Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Mon, 5 Oct 2026 15:15:50 -0500 Subject: [PATCH 11/11] Display variable-length members of nested compounds --- .../view/TableView/DataProviderFactory.java | 2 + .../view/dialog/NewScalarAttributeDialog.java | 2 +- .../TableView/NestedCompoundDisplayTest.java | 152 ++++++++++++++++++ .../main/java/hdf/object/h5/H5CompoundDS.java | 16 +- .../java/object/TestNestedDatatypeShapes.java | 35 ++++ 5 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 hdfview/src/test/java/hdf/view/TableView/NestedCompoundDisplayTest.java diff --git a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java index b4124522b..140c4751f 100644 --- a/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java +++ b/hdfview/src/main/java/hdf/view/TableView/DataProviderFactory.java @@ -850,6 +850,8 @@ public Object getDataValue(Object obj, int columnIndex, int rowIndex) colValue, columnIndex - relCmpdStartIndexMap.get(columnIndex), rowIndex); else if (base instanceof ArrayDataProvider) theValue = base.getDataValue(colValue, columnIndex, rowIndex); + else if (base instanceof VlenDataProvider) + theValue = base.getDataValue(colValue, columnIndex, rowIndex); else theValue = base.getDataValue(colValue, rowIndex); } diff --git a/hdfview/src/main/java/hdf/view/dialog/NewScalarAttributeDialog.java b/hdfview/src/main/java/hdf/view/dialog/NewScalarAttributeDialog.java index dcf2cf19e..82e932ea2 100644 --- a/hdfview/src/main/java/hdf/view/dialog/NewScalarAttributeDialog.java +++ b/hdfview/src/main/java/hdf/view/dialog/NewScalarAttributeDialog.java @@ -323,7 +323,7 @@ private boolean createAttribute() else attr = (Attribute) new H5ScalarAttr(parentObj, attrName, datatype, dims); Object value = H5Datatype.allocateArray(datatype, (int)lsize); - // allocateArray leaves variable-length string slots null; start the new + // allocateArray leaves variable-length string slots null, so start the new // attribute with empty strings instead. if (datatype.isVarStr()) Arrays.fill((Object[])value, ""); diff --git a/hdfview/src/test/java/hdf/view/TableView/NestedCompoundDisplayTest.java b/hdfview/src/test/java/hdf/view/TableView/NestedCompoundDisplayTest.java new file mode 100644 index 000000000..87764ed82 --- /dev/null +++ b/hdfview/src/test/java/hdf/view/TableView/NestedCompoundDisplayTest.java @@ -0,0 +1,152 @@ +/***************************************************************************** + * Copyright by The HDF Group. * + * All rights reserved. * + * * + * This file is part of the HDF Java Products distribution. * + * The full copyright notice, including terms governing use, modification, * + * and redistribution, is contained in the COPYING file, which can be found * + * at the root of the source code distribution tree, * + * or in https://www.hdfgroup.org/licenses. * + * If you do not have access to either file, you may request a copy from * + * help@hdfgroup.org. * + ****************************************************************************/ + +package hdf.view.TableView; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.lang.reflect.Proxy; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; + +import hdf.object.FileFormat; +import hdf.object.h5.H5CompoundDS; +import hdf.object.h5.H5File; + +import hdf.hdf5lib.H5; +import hdf.hdf5lib.HDF5Constants; + +import hdf.view.TableView.DataDisplayConverterFactory.HDFDisplayConverter; +import hdf.view.TableView.DataProviderFactory.HDFDataProvider; + +import org.eclipse.nebula.widgets.nattable.layer.cell.ILayerCell; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Cell text for a compound whose variable-length member sits inside a nested compound, + * COMPOUND{id:int, inner:COMPOUND{v:VLEN of int}}, run through the same data provider and + * display converter the table uses. + */ +public class NestedCompoundDisplayTest { + + @TempDir + static Path tempDir; + + private static final String DATASET_NAME = "compound_of_nested_vlen"; + + private static H5File testFile; + private static HDFDataProvider provider; + private static HDFDisplayConverter converter; + + private static ArrayList list(Object... items) { return new ArrayList<>(Arrays.asList(items)); } + + @BeforeAll + static void createFile() throws Exception + { + String path = tempDir.resolve("nested_compound_display_test.h5").toString(); + + long vt = -1, inner = -1, outer = -1, fid = -1, sid = -1, did = -1; + try { + vt = H5.H5Tvlen_create(HDF5Constants.H5T_NATIVE_INT); + inner = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, H5.H5Tget_size(vt)); + H5.H5Tinsert(inner, "v", 0, vt); + + long innerOffset = H5.H5Tget_size(HDF5Constants.H5T_NATIVE_INT); + outer = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, innerOffset + H5.H5Tget_size(inner)); + H5.H5Tinsert(outer, "id", 0, HDF5Constants.H5T_NATIVE_INT); + H5.H5Tinsert(outer, "inner", innerOffset, inner); + + fid = H5.H5Fcreate(path, HDF5Constants.H5F_ACC_TRUNC, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT); + sid = H5.H5Screate_simple(1, new long[] {2}, null); + did = H5.H5Dcreate(fid, DATASET_NAME, outer, sid, HDF5Constants.H5P_DEFAULT, + HDF5Constants.H5P_DEFAULT, HDF5Constants.H5P_DEFAULT); + + Object[] buf = {list(1, list(list(10, 11))), list(2, list(list(20)))}; + H5.H5DwriteVL(did, outer, HDF5Constants.H5S_ALL, HDF5Constants.H5S_ALL, HDF5Constants.H5P_DEFAULT, + buf); + } + finally { + if (did >= 0) + H5.H5Dclose(did); + if (sid >= 0) + H5.H5Sclose(sid); + if (fid >= 0) + H5.H5Fclose(fid); + if (outer >= 0) + H5.H5Tclose(outer); + if (inner >= 0) + H5.H5Tclose(inner); + if (vt >= 0) + H5.H5Tclose(vt); + } + + testFile = (H5File)(new H5File()).createInstance(path, FileFormat.READ); + testFile.open(); + + H5CompoundDS dataset = (H5CompoundDS)testFile.get("/" + DATASET_NAME); + dataset.init(); + Object data = dataset.getData(); + + provider = DataProviderFactory.getDataProvider(dataset, data, false); + converter = DataDisplayConverterFactory.getDataDisplayConverter(dataset); + } + + @AfterAll + static void closeFile() throws Exception + { + if (testFile != null) + testFile.close(); + } + + /** A table cell reduced to the row and column indices the converter reads. */ + private static ILayerCell cell(int row, int column) + { + return (ILayerCell)Proxy.newProxyInstance( + ILayerCell.class.getClassLoader(), new Class[] {ILayerCell.class}, (proxy, method, args) -> { + switch (method.getName()) { + case "getRowIndex": + return row; + case "getColumnIndex": + return column; + default: + throw new UnsupportedOperationException(method.getName()); + } + }); + } + + /** The text the table shows for one cell. */ + private static String cellText(int row, int column) + { + Object value = provider.getDataValue(column, row); + return String.valueOf(converter.canonicalToDisplayValue(cell(row, column), null, value)); + } + + @Test + void fixedMemberBesideNestedCompound() + { + assertEquals("1", cellText(0, 0)); + assertEquals("2", cellText(1, 0)); + } + + @Test + void vlenMemberInsideNestedCompound() + { + assertEquals("[10, 11]", cellText(0, 1)); + assertEquals("[20]", cellText(1, 1)); + } +} diff --git a/object/src/main/java/hdf/object/h5/H5CompoundDS.java b/object/src/main/java/hdf/object/h5/H5CompoundDS.java index 8ce822694..894413f86 100644 --- a/object/src/main/java/hdf/object/h5/H5CompoundDS.java +++ b/object/src/main/java/hdf/object/h5/H5CompoundDS.java @@ -1293,13 +1293,21 @@ else if (H5Datatype.containsVlenData(memberType)) { /* * The single-field compound transfer type wraps each row in a - * one-element record holding the member's value, so unwrap it here. + * one-element record holding the member's value. A member of a nested + * compound is read through one such record per level of nesting + * (see createCompoundFieldType), so unwrap once per level. */ + int levels = 1 + (memberName.length() - memberName.replace(CompoundDS.SEPARATOR, "").length()) / + CompoundDS.SEPARATOR.length(); Object[] rows = (Object[])memberData; for (int r = 0; r < rows.length; r++) { - if (rows[r] instanceof java.util.ArrayList rec && rec.size() == 1 && - rec.get(0) instanceof java.util.List) - rows[r] = rec.get(0); + for (int l = 0; l < levels; l++) { + if (rows[r] instanceof java.util.ArrayList rec && rec.size() == 1 && + rec.get(0) instanceof java.util.List) + rows[r] = rec.get(0); + else + break; + } } } else { diff --git a/object/src/test/java/object/TestNestedDatatypeShapes.java b/object/src/test/java/object/TestNestedDatatypeShapes.java index c552cdfa5..246bba92c 100644 --- a/object/src/test/java/object/TestNestedDatatypeShapes.java +++ b/object/src/test/java/object/TestNestedDatatypeShapes.java @@ -64,6 +64,7 @@ static void createFile() throws Exception writeCompoundWithReference(fid); writeVarStr(fid); writeArrayOfVlenIntAttribute(fid); + writeCompoundOfNestedVlen(fid); } finally { H5.H5Fclose(fid); @@ -431,6 +432,29 @@ private static void writeCompoundWithVarStr(long fid) throws Exception } } + /** COMPOUND{id:int, inner:COMPOUND{v:VLEN of int}}. */ + private static void writeCompoundOfNestedVlen(long fid) throws Exception + { + long vt = H5.H5Tvlen_create(HDF5Constants.H5T_NATIVE_INT); + long inner = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, H5.H5Tget_size(vt)); + long outer = -1; + try { + H5.H5Tinsert(inner, "v", 0, vt); + long innerOffset = H5.H5Tget_size(HDF5Constants.H5T_NATIVE_INT); + outer = H5.H5Tcreate(HDF5Constants.H5T_COMPOUND, innerOffset + H5.H5Tget_size(inner)); + H5.H5Tinsert(outer, "id", 0, HDF5Constants.H5T_NATIVE_INT); + H5.H5Tinsert(outer, "inner", innerOffset, inner); + Object[] buf = {list(1, list(list(10, 11))), list(2, list(list(20)))}; + writeDataset(fid, "compound_of_nested_vlen", outer, 2, buf, true); + } + finally { + if (outer >= 0) + H5.H5Tclose(outer); + H5.H5Tclose(inner); + H5.H5Tclose(vt); + } + } + // ---- tests ---------------------------------------------------------------- @Test @@ -677,4 +701,15 @@ public void testCompoundVarStrMemberWriteRoundTrip() throws Exception check.close(); } } + + @Test + @DisplayName("Compound with a VLEN member inside a nested compound") + public void testCompoundOfNestedVlen() throws Exception + { + /* + * The member is read through a nested single-field compound type, so the JNI wraps + * each row once per level of nesting; all of those wrappers must come off. + */ + assertEquals("[[1, 2], [[[10, 11], [20]]]]", render(open("compound_of_nested_vlen").getData())); + } }