This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push: new 1b6df63689 `Colorizer.forCategories(Map)` should not keep a reference to the user-supplied map. 1b6df63689 is described below commit 1b6df63689fb461f5780d0fecb32cc3d396c60da Author: Martin Desruisseaux <martin.desruisse...@geomatys.com> AuthorDate: Sat Apr 1 20:25:35 2023 +0200 `Colorizer.forCategories(Map)` should not keep a reference to the user-supplied map. --- .../main/java/org/apache/sis/image/Colorizer.java | 17 ++++++++-- .../java/org/apache/sis/image/Visualization.java | 7 ++--- .../internal/coverage/j2d/ColorModelBuilder.java | 11 +++---- .../internal/coverage/j2d/ColorModelFactory.java | 12 ++++---- .../sis/internal/coverage/j2d/ColorsForRange.java | 36 ++++++++++++++-------- 5 files changed, 52 insertions(+), 31 deletions(-) diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/Colorizer.java b/core/sis-feature/src/main/java/org/apache/sis/image/Colorizer.java index f926f3e84e..6d32896d96 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/image/Colorizer.java +++ b/core/sis-feature/src/main/java/org/apache/sis/image/Colorizer.java @@ -16,6 +16,8 @@ */ package org.apache.sis.image; +import java.util.ArrayList; +import java.util.AbstractMap.SimpleImmutableEntry; import java.util.Map; import java.util.List; import java.util.Objects; @@ -214,11 +216,20 @@ public interface Colorizer extends Function<Colorizer.Target, Optional<ColorMode * @see ImageProcessor#visualize(RenderedImage) */ public static Colorizer forRanges(final Map<NumberRange<?>,Color[]> colors) { - // Can not use `Map.copyOf(colors)` because it may contain null values. - final var factory = ColorModelFactory.piecewise(colors); + final var list = new ArrayList<Map.Entry<NumberRange<?>,Color[]>>(colors.size()); + for (final Map.Entry<NumberRange<?>,Color[]> entry : colors.entrySet()) { + var range = entry.getKey(); + var value = entry.getValue(); + if (value != null) { + value = value.clone(); + } + list.add(new SimpleImmutableEntry<>(range, value)); + } + final var entries = List.copyOf(list); + final var factory = ColorModelFactory.piecewise(entries); return (target) -> { if (target instanceof Visualization.Target) { - ((Visualization.Target) target).rangeColors = colors; + ((Visualization.Target) target).rangeColors = entries; } else { final OptionalInt visibleBand = target.getVisibleBand(); if (!visibleBand.isEmpty()) { diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/Visualization.java b/core/sis-feature/src/main/java/org/apache/sis/image/Visualization.java index ed6313022e..a741724ca9 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/image/Visualization.java +++ b/core/sis-feature/src/main/java/org/apache/sis/image/Visualization.java @@ -74,7 +74,7 @@ final class Visualization extends ResampledImage { /** * Colors to apply on the sample value ranges, as supplied by user. */ - Map<NumberRange<?>,Color[]> rangeColors; + List<Map.Entry<NumberRange<?>,Color[]>> rangeColors; /** * Colors to apply on the sample dimensions, as supplied by user. @@ -299,9 +299,8 @@ final class Visualization extends ResampledImage { */ boolean initialized; final ColorModelBuilder builder; - final var rangeColors = target.rangeColors; - if (rangeColors != null && !rangeColors.isEmpty()) { - builder = new ColorModelBuilder(rangeColors.entrySet(), sourceCM); + if (target.rangeColors != null) { + builder = new ColorModelBuilder(target.rangeColors, sourceCM); initialized = true; } else { /* diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorModelBuilder.java b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorModelBuilder.java index 2243187865..a4c9a2d541 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorModelBuilder.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorModelBuilder.java @@ -168,13 +168,13 @@ public final class ColorModelBuilder { private final ColorModel inheritedColors; /** - * Creates a new colorizer which will apply colors on the given range of values in source image. + * Creates a new colorizer which will apply colors on the given ranges of values in source image. * The {@code ColorModelBuilder} is considered initialized after this constructor; * callers shall <strong>not</strong> invoke an {@code initialize(…)} method. * * <p>The {@code colors} map shall not be null or empty but may contain {@code null} values. * Null values default to a fully transparent color when the range contains a single value, - * and to grayscale colors otherwise. + * and to grayscale colors otherwise, unless {@code inherited} is non-null. * Empty arrays of colors are interpreted as explicitly transparent.</p> * * @param colors the colors to use for each range of values in source image. @@ -182,7 +182,6 @@ public final class ColorModelBuilder { * Should be non-null only for styling an exiting image before visualization. */ public ColorModelBuilder(final Collection<Map.Entry<NumberRange<?>,Color[]>> colors, final ColorModel inherited) { - ArgumentChecks.ensureNonEmpty("colors", colors); entries = ColorsForRange.list(colors, inherited); inheritedColors = inherited; this.colors = GRAYSCALE; @@ -517,12 +516,12 @@ reuse: if (source != null) { * computing a transfer function, but those categories should not be returned to user. */ if (Double.isNaN(value)) { - builder.mapQualitative(entry.name, targetRange, (float) value); + builder.mapQualitative(entry.name(), targetRange, (float) value); } else { if (value == entry.sampleRange.getMaxDouble()) { sourceRange = NumberRange.create(value - 0.5, true, value + 0.5, false); } - builder.addQuantitative(entry.name, targetRange, sourceRange); + builder.addQuantitative(entry.name(), targetRange, sourceRange); themes = (themes != null) ? themes.unionAny(sourceRange) : sourceRange; } } @@ -578,7 +577,7 @@ reuse: if (source != null) { } final NumberRange<Integer> samples = NumberRange.create(lower, true, upper, false); if (mapper.put(samples, entry) == null) { - builder.addQuantitative(entry.name, samples, entry.sampleRange); + builder.addQuantitative(entry.name(), samples, entry.sampleRange); } lower = upper; } diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorModelFactory.java b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorModelFactory.java index 69410a46a4..631d6663d2 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorModelFactory.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorModelFactory.java @@ -18,6 +18,7 @@ package org.apache.sis.internal.coverage.j2d; import java.util.Map; import java.util.Arrays; +import java.util.Collection; import java.util.Comparator; import java.awt.Transparency; import java.awt.Color; @@ -299,17 +300,15 @@ public final class ColorModelFactory { /** * Prepares a factory of color models interpolated for the ranges in the given map entries. * The {@link ColorModel} instances will be shared among all callers in the running virtual machine. - * The {@code colors} map shall not be null or empty but may contain {@code null} values. + * The {@code colors} list shall not be null or empty but may contain {@code null} values. * Null values default to fully transparent color when the range contains a single value, * and to grayscale otherwise. Empty arrays of colors are interpreted as explicitly transparent. * * @param colors the colors to use for each range of sample values. * @return a factory of color model suitable for {@link RenderedImage} objects with values in the given ranges. */ - public static ColorModelFactory piecewise(final Map<NumberRange<?>, Color[]> colors) { - final var entries = colors.entrySet(); - ArgumentChecks.ensureNonEmpty("colors", entries); - final var ranges = ColorsForRange.list(entries, null); + public static ColorModelFactory piecewise(final Collection<Map.Entry<NumberRange<?>, Color[]>> colors) { + final var ranges = ColorsForRange.list(colors, null); return PIECEWISES.intern(new ColorModelFactory(DataBuffer.TYPE_BYTE, 0, DEFAULT_VISIBLE_BAND, ranges)); } @@ -465,7 +464,8 @@ public final class ColorModelFactory { /** * Returns a color model interpolated for the given range of values. - * This is a convenience method for {@link #piecewise(Map)} when the map contains only one element. + * This is a convenience method for {@link #piecewise(Collection)} + * when the map contains only one element. * * @param dataType the color model type. * @param numBands the number of bands for the color model (usually 1). diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorsForRange.java b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorsForRange.java index 76dc35f90a..d40c20196b 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorsForRange.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/ColorsForRange.java @@ -42,10 +42,12 @@ import org.apache.sis.util.ArraysExt; */ final class ColorsForRange implements Comparable<ColorsForRange> { /** - * A name identifying the range of values. the category name is used if available, - * otherwise this is a string representation of the range. + * A name identifying the range of values, or {@code null} if not yet computed. + * The category name is used if available, otherwise this is a string representation of the range. + * + * @see #name() */ - final CharSequence name; + private CharSequence name; /** * The range of sample values on which the colors will be applied. Shall never be null. @@ -110,18 +112,18 @@ final class ColorsForRange implements Comparable<ColorsForRange> { /** * Creates a new instance for the given range of values. * - * @param name a name identifying the range of values, or {@code null} for automatic. - * @param sampleRange range of sample values on which the colors will be applied. - * @param colors colors to apply on the range of sample values, or {@code null} for default. - * @param isData whether this entry should be taken as main data (not fill values). - * @param inherited the original colors to use as fallback, or {@code null} if none. - * Should be non-null only for styling an exiting image before visualization. + * @param name a name identifying the range of values, or {@code null} for automatic. + * @param sampleRange range of sample values on which the colors will be applied. + * @param colors colors to apply on the range of sample values, or {@code null} for default. + * @param isData whether this entry should be taken as main data (not fill values). + * @param inherited the original colors to use as fallback, or {@code null} if none. + * Should be non-null only for styling an exiting image before visualization. */ ColorsForRange(final CharSequence name, final NumberRange<?> sampleRange, final Color[] colors, final boolean isData, final ColorModel inherited) { ArgumentChecks.ensureNonNull("sampleRange", sampleRange); - this.name = (name != null) ? name : sampleRange.toString(); + this.name = name; this.sampleRange = originalSampleRange = sampleRange; this.isData = isData; this.colors = colors; @@ -146,12 +148,12 @@ final class ColorsForRange implements Comparable<ColorsForRange> { * The {@link #category} of each entry is left to null. * * @param colors the colors to use for each range of sample values. - * A {@code null} entry value means transparent. * @param inherited the original color model from which to inherit undefined colors, or {@code null} if none. * @return colors to use for each range of values in the source image. * Never null and does not contain null elements. */ static ColorsForRange[] list(final Collection<Map.Entry<NumberRange<?>,Color[]>> colors, final ColorModel inherited) { + ArgumentChecks.ensureNonEmpty("colors", colors); final ColorsForRange[] entries = new ColorsForRange[colors.size()]; int n = 0; for (final Map.Entry<NumberRange<?>,Color[]> entry : colors) { @@ -162,12 +164,22 @@ final class ColorsForRange implements Comparable<ColorsForRange> { return ArraysExt.resize(entries, n); // `resize` should not be needed, but we are paranoiac. } + /** + * Returns the name pf this range of colors. + */ + final CharSequence name() { + if (name == null) { + name = sampleRange.toString(); + } + return name; + } + /** * Returns a string representation for debugging purposes. */ @Override public String toString() { - final StringBuilder buffer = new StringBuilder(name).append(": ").append(sampleRange); + final StringBuilder buffer = new StringBuilder(name()).append(": ").append(sampleRange); appendColorRange(buffer, toARGB(2)); return buffer.toString(); }