You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2023/04/01 18:26:14 UTC

[sis] branch geoapi-4.0 updated: `Colorizer.forCategories(Map)` should not keep a reference to the user-supplied map.

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 <ma...@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();
     }