-
Notifications
You must be signed in to change notification settings - Fork 106
Fix for clipping and rounding issues #267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 8.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package org.esa.snap.core.dataop.resamp; | ||
|
|
||
| /** | ||
| * Resampling decorator. Ensures that the decorated resampling yields non-negative values | ||
| * by clipping any negative value to zero. | ||
| * | ||
| * @author Ralf Quast | ||
| */ | ||
| public class NonNegative implements Resampling { | ||
|
|
||
| private final Resampling resampling; | ||
|
|
||
| /** | ||
| * Creates a new "non-negative" resampling from a given resampling type. | ||
| * | ||
| * @param resampling the resampling type. | ||
| */ | ||
| public NonNegative(Resampling resampling) { | ||
| this.resampling = resampling; | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| return String.format("NON_NEGATIVE_%s", resampling.getName()); | ||
| } | ||
|
|
||
| @Override | ||
| public Index createIndex() { | ||
| return resampling.createIndex(); | ||
| } | ||
|
|
||
| @Override | ||
| public void computeIndex(double x, double y, int width, int height, Index index) { | ||
| resampling.computeIndex(x, y, width, height, index); | ||
| } | ||
|
|
||
| @Override | ||
| public void computeCornerBasedIndex(double x, double y, int width, int height, Index index) { | ||
| resampling.computeCornerBasedIndex(x, y, width, height, index); | ||
| } | ||
|
|
||
| @Override | ||
| public double resample(Raster raster, Index index) throws Exception { | ||
| return Math.max(0.0, resampling.resample(raster, index)); | ||
| } | ||
|
|
||
| @Override | ||
| public int getKernelSize() { | ||
| return resampling.getKernelSize(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| /* | ||
| * Copyright (C) 2014 Brockmann Consult GmbH (info@brockmann-consult.de) | ||
| * | ||
| * This program is free software; you can redistribute it and/or modify it | ||
| * under the terms of the GNU General Public License as published by the Free | ||
| * Software Foundation; either version 3 of the License, or (at your option) | ||
| * any later version. | ||
| * This program is distributed in the hope that it will be useful, but WITHOUT | ||
| * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or | ||
| * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for | ||
| * more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License along | ||
| * with this program; if not, see http://www.gnu.org/licenses/ | ||
| */ | ||
|
|
||
| package org.esa.snap.core.dataop.resamp; | ||
|
|
||
| import junit.framework.TestCase; | ||
|
|
||
| public class NonNegativeBicubicInterpolationResamplingTest extends TestCase { | ||
|
|
||
| final Resampling resampling = Resampling.NON_NEGATIVE_BICUBIC_INTERPOLATION; | ||
| final TestRaster raster = new TestRaster(); | ||
|
|
||
| public void testCreateIndex() { | ||
| final Resampling.Index index = resampling.createIndex(); | ||
| assertNotNull(index); | ||
| assertNotNull(index.i); | ||
| assertNotNull(index); | ||
| assertNotNull(index.i); | ||
| assertNotNull(index.j); | ||
| assertNotNull(index.ki); | ||
| assertNotNull(index.kj); | ||
| assertEquals(2, index.i.length); | ||
| assertEquals(2, index.j.length); | ||
| assertEquals(1, index.ki.length); | ||
| assertEquals(1, index.kj.length); | ||
| } | ||
|
|
||
| public void testIndexAndSample() throws Exception { | ||
| final Resampling.Index index = resampling.createIndex(); | ||
|
|
||
| testIndexAndSample( | ||
| index, | ||
| 2.2f, 2.3f, | ||
| 1.0, 2.0, | ||
| 1.0, 2.0, | ||
| 0.7f, | ||
| 0.8f, | ||
| 25.0616f); | ||
| } | ||
|
|
||
| private void testIndexAndSample( | ||
| final Resampling.Index index, | ||
| float x, float y, | ||
| double i1Exp, double i2Exp, | ||
| double j1Exp, double j2Exp, | ||
| float ki1Exp, | ||
| float kj1Exp, | ||
| float sampleExp) throws Exception { | ||
|
|
||
| resampling.computeIndex(x, y, raster.getWidth(), raster.getHeight(), index); | ||
|
|
||
| assertEquals(i1Exp, index.i[0]); | ||
| assertEquals(i2Exp, index.i[1]); | ||
|
|
||
| assertEquals(j1Exp, index.j[0]); | ||
| assertEquals(j2Exp, index.j[1]); | ||
|
|
||
| assertEquals(ki1Exp, index.ki[0], 1e-5f); | ||
| assertEquals(kj1Exp, index.kj[0], 1e-5f); | ||
|
|
||
| double sample = resampling.resample(raster, index); | ||
| assertEquals(sampleExp, sample, 1e-5f); | ||
| } | ||
|
|
||
| public void testCornerBasedIndex() throws Exception { | ||
| testCornerIndex(2.2f, 2.3f); | ||
| } | ||
|
|
||
| private void testCornerIndex(final float x, final float y) { | ||
|
|
||
| final Resampling.Index index = resampling.createIndex(); | ||
| resampling.computeCornerBasedIndex(x, y, raster.getWidth(), raster.getHeight(), index); | ||
|
|
||
| final Resampling.Index indexExp = resampling.createIndex(); | ||
| computeExpectedIndex(x, y, raster.getWidth(), raster.getHeight(), indexExp); | ||
|
|
||
| assertEquals(indexExp.i[0], index.i[0]); | ||
| assertEquals(indexExp.i[1], index.i[1]); | ||
| assertEquals(indexExp.j[0], index.j[0]); | ||
| assertEquals(indexExp.j[1], index.j[1]); | ||
| assertEquals(indexExp.ki[0], index.ki[0]); | ||
| assertEquals(indexExp.kj[0], index.kj[0]); | ||
| } | ||
|
|
||
| private void computeExpectedIndex( | ||
| final double x, final double y, final int width, final int height, final Resampling.Index index) { | ||
| index.x = x; | ||
| index.y = y; | ||
| index.width = width; | ||
| index.height = height; | ||
|
|
||
|
|
||
| final int i0 = (int) Math.floor(x); | ||
| final int j0 = (int) Math.floor(y); | ||
|
|
||
| index.i0 = i0; | ||
| index.j0 = j0; | ||
|
|
||
| index.i[0] = Resampling.Index.crop(i0, width - 1); | ||
| index.i[1] = Math.min(i0 + 1, width - 1); | ||
| index.ki[0] = x - i0; | ||
| index.j[0] = Resampling.Index.crop(j0, height - 1); | ||
| index.j[1] = Math.min(j0 + 1, height - 1); | ||
| index.kj[0] = y - j0; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,6 +121,17 @@ public double toGeoPhysical(double sample) { | |
| return rasterDataNode.scale(sample); | ||
| } | ||
|
|
||
| private int toRaw(int sample) { | ||
| final double rawSample = rasterDataNode.scaleInverse(sample); | ||
| if (rawSample < -2147483648.0) { | ||
| return Integer.MIN_VALUE; | ||
| } | ||
| if (rawSample > 2147483647.0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the same as above |
||
| return Integer.MAX_VALUE; | ||
| } | ||
| return (int) Math.round(rawSample); | ||
| } | ||
|
|
||
| @Override | ||
| public float toRaw(float sample) { | ||
| return (float) rasterDataNode.scaleInverse(sample); | ||
|
|
@@ -492,7 +503,6 @@ public int getSampleInt(int x, int y) { | |
| int sample = raster.getSample(x, y, 0); | ||
| // handle unsigned data types, see also [BEAM-1147] (nf - 20100527) | ||
| if (signedByte) { | ||
| //noinspection SillyAssignment | ||
| sample = (byte) sample; | ||
| } | ||
| if (scaled) { | ||
|
|
@@ -504,17 +514,39 @@ public int getSampleInt(int x, int y) { | |
| @Override | ||
| public void setSample(int x, int y, int sample) { | ||
| if (scaled) { | ||
| sample = (int) Math.floor(toRaw((double) sample) + 0.5); | ||
| sample = toRaw(sample); | ||
| } | ||
| switch (rasterDataNode.getDataType()) { | ||
| case ProductData.TYPE_INT8: | ||
| sample = clipOrRound(sample, -128, 127); | ||
| break; | ||
| case ProductData.TYPE_INT16: | ||
| sample = clipOrRound(sample, -32768, 32767); | ||
| break; | ||
| case ProductData.TYPE_UINT8: | ||
| sample = clipOrRound(sample, 0, 255); | ||
| break; | ||
| case ProductData.TYPE_UINT16: | ||
| sample = clipOrRound(sample, 0, 65535); | ||
| break; | ||
| } | ||
| writableRaster.setSample(x, y, 0, sample); | ||
| } | ||
|
|
||
| private int clipOrRound(int sample, int lowerBound, int upperBound) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only a clip without round |
||
| if (sample < lowerBound) { | ||
| sample = lowerBound; | ||
| } else if (sample > upperBound) { | ||
| sample = upperBound; | ||
| } | ||
| return sample; | ||
| } | ||
|
|
||
| @Override | ||
| public float getSampleFloat(int x, int y) { | ||
| float sample = raster.getSampleFloat(x, y, 0); | ||
| // handle unsigned data types, see also [BEAM-1147] (nf - 20100527) | ||
| if (signedByte) { | ||
| //noinspection SillyAssignment | ||
| sample = (byte) sample; | ||
| } | ||
| if (scaled) { | ||
|
|
@@ -528,16 +560,38 @@ public void setSample(int x, int y, float sample) { | |
| if (scaled) { | ||
| sample = toRaw(sample); | ||
| } | ||
| switch (rasterDataNode.getDataType()) { | ||
| case ProductData.TYPE_INT8: | ||
| sample = clipOrRound(sample, -128.0f, 127.0f); | ||
| break; | ||
| case ProductData.TYPE_INT16: | ||
| sample = clipOrRound(sample, -32768.0f, 32767.0f); | ||
| break; | ||
| case ProductData.TYPE_UINT8: | ||
| sample = clipOrRound(sample, 0.0f, 255.0f); | ||
| break; | ||
| case ProductData.TYPE_UINT16: | ||
| sample = clipOrRound(sample, 0.0f, 65535.0f); | ||
| break; | ||
| } | ||
| writableRaster.setSample(x, y, 0, sample); | ||
| } | ||
|
|
||
| private float clipOrRound(float sample, float lowerBound, float upperBound) { | ||
| if (sample < lowerBound) { | ||
| return lowerBound; | ||
| } | ||
| if (sample > upperBound) { | ||
| return upperBound; | ||
| } | ||
| return (float) Math.rint(sample); | ||
| } | ||
|
|
||
| @Override | ||
| public double getSampleDouble(int x, int y) { | ||
| double sample = raster.getSampleDouble(x, y, 0); | ||
| // handle unsigned data types, see also [BEAM-1147] (nf - 20100527) | ||
| if (signedByte) { | ||
| //noinspection SillyAssignment | ||
| sample = (byte) sample; | ||
| } | ||
| if (scaled) { | ||
|
|
@@ -551,9 +605,33 @@ public void setSample(int x, int y, double sample) { | |
| if (scaled) { | ||
| sample = toRaw(sample); | ||
| } | ||
| switch (rasterDataNode.getDataType()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If raw data run into a clipping there must be something wrong with geopysical data because they run outside the covered valid value range, defined by raw data type and scaling. In this case clipping the raw data value then turns a wrong geophysical value into a valid value. Clipping or other types of handling values outside the range should be done at the place where the TileImpl is used, because only the context knows how to handle values outside the range. Clipping then should be done that way: An integer cast is neither a floor nor a round nor a cail operation.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I disagree.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thats exactly what I meant. If the value is in a scientific valid range, but can not be saved, then the configuration of the store must be wrong. Then either the raw data type or the scaling must be wrong. If a value is a scientific valid value, a raw clipping changes this scientific valid value. Does this make sence? Ideally a raw date store should not manipulate data. Also a raw data store shall not know anything of the valid scientific value range. A raw data store shall only be able to tore store all the scientivic valid data of a cerain use case. For this a raw data store has to be configured the right way. Then scientific valid data can be stored using the scaling but without manipulation. |
||
| case ProductData.TYPE_INT8: | ||
| sample = clipOrRound(sample, -128.0, 127.0); | ||
| break; | ||
| case ProductData.TYPE_INT16: | ||
| sample = clipOrRound(sample, -32768.0, 32767.0); | ||
| break; | ||
| case ProductData.TYPE_UINT8: | ||
| sample = clipOrRound(sample, 0.0, 255.0); | ||
| break; | ||
| case ProductData.TYPE_UINT16: | ||
| sample = clipOrRound(sample, 0.0, 65535.0); | ||
| break; | ||
| } | ||
| writableRaster.setSample(x, y, 0, sample); | ||
| } | ||
|
|
||
| private double clipOrRound(double sample, double lowerBound, double upperBound) { | ||
| if (sample < lowerBound) { | ||
| return lowerBound; | ||
| } | ||
| if (sample > upperBound) { | ||
| return upperBound; | ||
| } | ||
| return Math.rint(sample); | ||
| } | ||
|
|
||
| @Override | ||
| public boolean getSampleBit(int x, int y, int bitIndex) { | ||
| long sample = raster.getSample(x, y, 0); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which miracle Number ist this?
Why not
if (rawSample < Integer.MIN_VALUE)