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
commit c3ca073d5e90a798af9cad8b62352e9cbf41e23f
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Mon Apr 4 17:24:20 2022 +0200
Bug fix: colors applied to ASCII Grid images were wrongly applied
to other kind of images if they shared the same `ImageProcessor`.
---
.../org/apache/sis/coverage/grid/GridCoverage.java | 4 +-
.../sis/coverage/grid/GridCoverageBuilder.java | 2 +-
.../apache/sis/coverage/grid/ImageRenderer.java | 3 +-
.../java/org/apache/sis/image/Visualization.java | 4 +-
.../internal/coverage/j2d/ColorModelFactory.java | 2 +-
.../sis/internal/coverage/j2d/Colorizer.java | 74 ++++++++++++++++------
.../sis/internal/coverage/j2d/ColorsForRange.java | 61 +++++++++---------
.../sis/internal/coverage/j2d/ColorizerTest.java | 4 +-
8 files changed, 97 insertions(+), 57 deletions(-)
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverage.java
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverage.java
index 8232a77d9f..2d1ade3610 100644
---
a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverage.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverage.java
@@ -282,7 +282,9 @@ public abstract class GridCoverage extends BandedCoverage {
final int visibleBand = Math.max(0,
ImageUtilities.getVisibleBand(source));
final Colorizer colorizer = new Colorizer(Colorizer.GRAYSCALE);
final ColorModel colors;
- if (colorizer.initialize(sampleDimensions[visibleBand]) ||
colorizer.initialize(source.getColorModel())) {
+ if (colorizer.initialize(source.getSampleModel(),
sampleDimensions[visibleBand]) ||
+ colorizer.initialize(source.getColorModel()))
+ {
colors = colorizer.createColorModel(bandType.toDataBufferType(),
sampleDimensions.length, visibleBand);
} else {
colors = Colorizer.NULL_COLOR_MODEL;
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverageBuilder.java
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverageBuilder.java
index 5a83745739..1e437360e5 100644
---
a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverageBuilder.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridCoverageBuilder.java
@@ -473,7 +473,7 @@ public class GridCoverageBuilder {
final SampleModel sm = raster.getSampleModel();
final Colorizer colorizer = new Colorizer(Colorizer.GRAYSCALE);
final ColorModel colors;
- if (colorizer.initialize(bands.get(visibleBand)) ||
colorizer.initialize(sm, visibleBand)) {
+ if (colorizer.initialize(sm, bands.get(visibleBand)) ||
colorizer.initialize(sm, visibleBand)) {
colors =
colorizer.createColorModel(ImageUtilities.getBandType(sm), bands.size(),
visibleBand);
} else {
colors = Colorizer.NULL_COLOR_MODEL;
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ImageRenderer.java
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ImageRenderer.java
index bf2f424b91..9bb02bc94a 100644
---
a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ImageRenderer.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/ImageRenderer.java
@@ -731,7 +731,8 @@ public class ImageRenderer {
final Raster raster = createRaster();
final Colorizer colorizer = new Colorizer(colors);
final ColorModel colors;
- if (colorizer.initialize(bands[visibleBand]) ||
colorizer.initialize(raster.getSampleModel(), visibleBand)) {
+ final SampleModel sm = raster.getSampleModel();
+ if (colorizer.initialize(sm, bands[visibleBand]) ||
colorizer.initialize(sm, visibleBand)) {
colors = colorizer.createColorModel(buffer.getDataType(),
bands.length, visibleBand);
} else {
colors = Colorizer.NULL_COLOR_MODEL;
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 393f65fb1f..f749ebf56d 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
@@ -219,10 +219,10 @@ final class Visualization extends ResampledImage {
* in various ways: sample dimensions, scaled color model,
statistics in last resort.
*/
colorizer = new Colorizer(categoryColors);
- initialized = (sourceBands != null) &&
colorizer.initialize(sourceBands.get(visibleBand));
+ initialized = (sourceBands != null) &&
colorizer.initialize(source.getSampleModel(), sourceBands.get(visibleBand));
if (initialized) {
/*
- * If we have been able to configure Colorizer using the
SampleModel, apply an adjustment based
+ * If we have been able to configure Colorizer using
SampleDimension, apply an adjustment based
* on the ScaledColorModel if it exists. Use case: an
image is created with an IndexColorModel
* determined by the SampleModel, then user enhanced
contrast by a call to `stretchColorRamp(…)`
* above. We want to preserve that contrast enhancement.
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 bf0835fafb..c502db5fd5 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
@@ -398,7 +398,7 @@ public final class ColorModelFactory {
final double lower, final double
upper, final Color... colors)
{
return createPiecewise(dataType, numBands, visibleBand, new
ColorsForRange[] {
- new ColorsForRange(null, new NumberRange<>(Double.class, lower,
true, upper, false), colors)
+ new ColorsForRange(null, new NumberRange<>(Double.class, lower,
true, upper, false), colors, true)
});
}
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/Colorizer.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/Colorizer.java
index c39cadb2fd..715c521c2a 100644
---
a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/Colorizer.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/Colorizer.java
@@ -28,6 +28,7 @@ import java.awt.image.ColorModel;
import java.awt.image.DataBuffer;
import java.awt.image.IndexColorModel;
import java.awt.image.SampleModel;
+import org.opengis.util.InternationalString;
import org.opengis.referencing.operation.MathTransform1D;
import org.opengis.referencing.operation.NoninvertibleTransformException;
import org.apache.sis.referencing.operation.transform.MathTransforms;
@@ -58,8 +59,8 @@ import org.apache.sis.util.Debug;
* There is no {@code initialize(Raster)} or {@code initialize(RenderedImage)}
method because if those methods
* were present, users may expect them to iterate over sample values for
finding minimum and maximum values.
* We do not perform such iteration because they are potentially costly and
give unstable results:
- * the resulting color model varies from image to image, which is confusing
when they are images of the same
- * product as different depth or different time.
+ * the resulting color model varies from image to image, which is confusing
when many images exist
+ * for the same product at different times or at different depths.
*
* @author Martin Desruisseaux (Geomatys)
* @version 1.2
@@ -77,6 +78,21 @@ public final class Colorizer {
*/
public static final ColorModel NULL_COLOR_MODEL = null;
+ /**
+ * Names to use for the synthetic categories and sample dimensions created
for visualization purposes.
+ * Transparent pixel is usually 0 and opaque pixels are in the range 1 to
{@value #MAX_VALUE} inclusive.
+ *
+ * <p>For safety, we use names that are different than the default "No
data" and "Data" names assigned by
+ * {@link SampleDimension.Builder}. The "[No] data" default names are
often used by formats that are poor
+ * in metadata, for example ASCII Grid. If we were using the same names, a
{@link #colors} function could
+ * confuse synthetic categories with "real" categories with uninformative
name, and consequently apply
+ * wrong colors.</p>
+ */
+ private static final InternationalString
+ TRANSPARENT =
Vocabulary.formatInternational(Vocabulary.Keys.Transparent),
+ COLOR_INDEX =
Vocabulary.formatInternational(Vocabulary.Keys.ColorIndex),
+ VISUAL =
Vocabulary.formatInternational(Vocabulary.Keys.Visual);
+
/**
* Maximal index value which can be used with a 8 bits {@link
IndexColorModel}, inclusive.
* Sample values must be in that range for enabling the use of {@link
#TYPE_COMPACT}.
@@ -192,20 +208,33 @@ public final class Colorizer {
* the sample dimension, colors will be determined by a call to {@code
colors.apply(category)}
* where {@code colors} is the function specified at construction time.
*
+ * @param model the sample model used with the data, or {@code null} if
unknown.
* @param source description of range of values in the source image, or
{@code null}.
* @return {@code true} on success, or {@code false} if no range of values
has been found.
* @throws IllegalStateException if a sample dimension is already defined
on this colorizer.
*/
- public boolean initialize(final SampleDimension source) {
+ public boolean initialize(final SampleModel model, final SampleDimension
source) {
checkInitializationStatus(false);
if (source != null) {
this.source = source;
final List<Category> categories = source.getCategories();
if (!categories.isEmpty()) {
- final ColorsForRange[] entries = new
ColorsForRange[categories.size()];
+ boolean missingNodata = true;
+ ColorsForRange[] entries = new
ColorsForRange[categories.size()];
for (int i=0; i<entries.length; i++) {
final Category category = categories.get(i);
- entries[i] = new ColorsForRange(category,
category.getSampleRange(), colors.apply(category));
+ entries[i] = new ColorsForRange(category, colors);
+ missingNodata &= category.isQuantitative();
+ }
+ /*
+ * If the model uses floating point values and there is no "no
data" category, add one.
+ * We force a "no data" category because floating point values
may be NaN.
+ */
+ if (missingNodata && (model == null ||
!ImageUtilities.isIntegerType(model))) {
+ final int count = entries.length;
+ entries = Arrays.copyOf(entries, count + 1);
+ entries[count] = new
ColorsForRange(Vocabulary.formatInternational(Vocabulary.Keys.Nodata),
+
NumberRange.create(Float.class, Float.NaN), null, false);
}
// Leave `target` to null. It will be computed by `compact()`
if needed.
this.entries = entries;
@@ -271,8 +300,9 @@ public final class Colorizer {
* Applies colors on the given range of values. The 0 index will be
reserved for NaN value,
* and indices in the [1 … 255] will be mapped to the given range.
*
- * <p>This method is typically used as a last resort fallback when other
{@code initialize(…)}
- * methods failed or can not be applied.</p>
+ * <p>This method is typically used as a last resort fallback when all
other {@code initialize(…)}
+ * methods failed or can not be applied. This method assumes that no
{@link Category} information
+ * is available.</p>
*
* @param minimum minimum value, inclusive.
* @param maximum maximum value, inclusive.
@@ -284,15 +314,20 @@ public final class Colorizer {
ArgumentChecks.ensureFinite("maximum", maximum);
defaultRange = NumberRange.create(minimum, true, maximum, true);
target = new SampleDimension.Builder()
- .setBackground(null, 0)
- .addQuantitative(null, NumberRange.create(1, true, MAX_VALUE,
true), defaultRange).build();
-
+ .setBackground(TRANSPARENT, 0)
+ .addQuantitative(COLOR_INDEX, NumberRange.create(1, true,
MAX_VALUE, true), defaultRange)
+ .setName(VISUAL).build();
+ /*
+ * We created a synthetic `SampleDimension` with the specified range
of values.
+ * Recompute `source` and `entries` fields for consistency with the
new ranges.
+ * The `source` is recreated as a matter of principle, but will not be
used by
+ * `compact()` because `target` will take precedence.
+ */
source = target.forConvertedValues(true);
- final List<Category> categories = source.getCategories();
+ final List<Category> categories = target.getCategories();
final ColorsForRange[] entries = new ColorsForRange[categories.size()];
for (int i=0; i<entries.length; i++) {
- final Category category = categories.get(i);
- entries[i] = new ColorsForRange(category,
category.forConvertedValues(false).getSampleRange(), colors.apply(category));
+ entries[i] = new ColorsForRange(categories.get(i), GRAYSCALE);
}
this.entries = entries;
}
@@ -417,13 +452,12 @@ reuse: if (source != null) {
for (int i=0; i<count; i++) {
final ColorsForRange entry = entries[i];
NumberRange<?> sourceRange = entry.sampleRange;
- if (!entry.isData()) {
+ if (!entry.isData) {
if (lower >= MAX_VALUE) {
throw new
IllegalArgumentException(Resources.format(Resources.Keys.TooManyQualitatives));
}
final NumberRange<Integer> targetRange =
NumberRange.create(lower, true, ++lower, false);
if (mapper.put(targetRange, entry) == null) {
- final CharSequence name = entry.name();
final double value = sourceRange.getMinDouble();
/*
* In the usual case where we have a mix of quantitative
and qualitative categories,
@@ -433,12 +467,12 @@ reuse: if (source != null) {
* computing a transfer function, but those categories
should not be returned to user.
*/
if (Double.isNaN(value)) {
- builder.mapQualitative(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(name, targetRange,
sourceRange);
+ builder.addQuantitative(entry.name, targetRange,
sourceRange);
themes = (themes != null) ?
themes.unionAny(sourceRange) : sourceRange;
}
}
@@ -472,7 +506,7 @@ reuse: if (source != null) {
span += sourceRange.getSpan();
final ColorsForRange[] tmp = Arrays.copyOf(entries, ++count);
System.arraycopy(entries, deferred, tmp, ++deferred, count -
deferred);
- tmp[deferred-1] = new ColorsForRange(null, sourceRange, new
Color[] {Color.BLACK, Color.WHITE});
+ tmp[deferred-1] = new ColorsForRange(null, sourceRange, new
Color[] {Color.BLACK, Color.WHITE}, true);
entries = tmp;
}
}
@@ -494,7 +528,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;
}
@@ -505,7 +539,7 @@ reuse: if (source != null) {
if (source != null) {
builder.setName(source.getName());
} else {
- builder.setName(Vocabulary.format(Vocabulary.Keys.Visual));
+ builder.setName(VISUAL);
}
target = builder.build();
for (final Category category : target.getCategories()) {
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 3236904432..23a6bbd66b 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
@@ -18,6 +18,7 @@ package org.apache.sis.internal.coverage.j2d;
import java.util.Map;
import java.util.Collection;
+import java.util.function.Function;
import java.awt.Color;
import java.awt.image.IndexColorModel;
import org.apache.sis.coverage.Category;
@@ -31,7 +32,7 @@ import org.apache.sis.util.ArraysExt;
* the time needed for {@link ColorModelFactory#createColorModel(int, int,
int, ColorsForRange[])}.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.2
*
* @see ColorModelFactory#createColorModel(int, int, int, ColorsForRange[])
*
@@ -40,10 +41,10 @@ import org.apache.sis.util.ArraysExt;
*/
final class ColorsForRange implements Comparable<ColorsForRange> {
/**
- * If this {@code ColorsForRange} has been created for a category, that
category.
- * Otherwise {@code null}.
+ * A name identifying the range of values. the category name is used if
available,
+ * otherwise this is a string representation of the range.
*/
- private final Category category;
+ final CharSequence name;
/**
* The range of sample values on which the colors will be applied. Shall
never be null.
@@ -58,18 +59,40 @@ final class ColorsForRange implements
Comparable<ColorsForRange> {
*/
private final Color[] colors;
+ /**
+ * {@code true} if this entry should be taken as data, or {@code false} if
it should be ignored.
+ * Entry to ignore are entries associated to NaN values.
+ */
+ final boolean isData;
+
+ /**
+ * Creates a new instance for the given category.
+ *
+ * @param category the category for which this {@code ColorsForRange} is
created, or {@code null}.
+ * @param colors colors to apply on the category.
+ */
+ ColorsForRange(final Category category, final Function<Category,Color[]>
colors) {
+ final CharSequence name = category.getName();
+ this.name = (name != null) ? name : sampleRange.toString();
+ this.sampleRange = category.getSampleRange();
+ this.colors = colors.apply(category);
+ this.isData = category.isQuantitative();
+ }
+
/**
* Creates a new instance for the given range of values.
*
- * @param category the category for which this {@code ColorsForRange}
is created, or {@code null}.
+ * @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 transparent.
+ * @param isData whether this entry should be taken as main data
(not fill values).
*/
- ColorsForRange(final Category category, final NumberRange<?> sampleRange,
final Color[] colors) {
+ ColorsForRange(final CharSequence name, final NumberRange<?> sampleRange,
final Color[] colors, final boolean isData) {
ArgumentChecks.ensureNonNull("sampleRange", sampleRange);
- this.category = category;
+ this.name = (name != null) ? name : sampleRange.toString();
this.sampleRange = sampleRange;
this.colors = colors;
+ this.isData = isData;
}
/**
@@ -85,37 +108,17 @@ final class ColorsForRange implements
Comparable<ColorsForRange> {
final ColorsForRange[] entries = new ColorsForRange[colors.size()];
int n = 0;
for (final Map.Entry<NumberRange<?>,Color[]> entry : colors) {
- entries[n++] = new ColorsForRange(null, entry.getKey(),
entry.getValue());
+ entries[n++] = new ColorsForRange(null, entry.getKey(),
entry.getValue(), true);
}
return ArraysExt.resize(entries, n); // `resize` should not
be needed, but we are paranoiac.
}
- /**
- * Returns {@code true} if this entry should be taken as data, or {@code
false} if it should be ignored.
- * Entry to ignore are entries associated to NaN values.
- */
- final boolean isData() {
- return category == null || category.isQuantitative();
- }
-
- /**
- * Returns a name identifying the range of values. the category name is
used if available,
- * otherwise a string representation of the range is created.
- */
- final CharSequence name() {
- if (category != null) {
- final CharSequence name = category.getName();
- if (name != null) return name;
- }
- return sampleRange.toString();
- }
-
/**
* Returns a string representation for debugging purposes.
*/
@Override
public String toString() {
- return name().toString();
+ return name.toString();
}
/**
diff --git
a/core/sis-feature/src/test/java/org/apache/sis/internal/coverage/j2d/ColorizerTest.java
b/core/sis-feature/src/test/java/org/apache/sis/internal/coverage/j2d/ColorizerTest.java
index 5d37bb214d..8fc4c8a49d 100644
---
a/core/sis-feature/src/test/java/org/apache/sis/internal/coverage/j2d/ColorizerTest.java
+++
b/core/sis-feature/src/test/java/org/apache/sis/internal/coverage/j2d/ColorizerTest.java
@@ -38,7 +38,7 @@ import static org.junit.Assert.*;
* Tests {@link Colorizer}.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.2
* @since 1.1
* @module
*/
@@ -101,7 +101,7 @@ public final strictfp class ColorizerTest extends TestCase {
.setName("Temperature").build();
final Colorizer colorizer = new Colorizer(Colorizer.GRAYSCALE);
- assertTrue("initialize", colorizer.initialize(sd));
+ assertTrue("initialize", colorizer.initialize(null, sd));
final IndexColorModel cm = (IndexColorModel)
colorizer.compactColorModel(1, 0); // Must be first.
/*
* Test conversion of a few sample values to packed values.
