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 ee71343d8196366d507f36d7eea217be9953df5c
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Fri Jan 24 20:12:34 2020 +0100

    A little bit of formatting improvements.
---
 .../java/org/apache/sis/gui/coverage/GridRow.java  |   7 +
 .../org/apache/sis/gui/coverage/GridRowSkin.java   |  39 +++--
 .../java/org/apache/sis/gui/coverage/GridView.java | 177 +++++++++++++++++----
 .../org/apache/sis/gui/coverage/GridViewSkin.java  |  96 ++++++++++-
 4 files changed, 273 insertions(+), 46 deletions(-)

diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridRow.java 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridRow.java
index 9b17b0b..ad89f85 100644
--- 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridRow.java
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridRow.java
@@ -20,6 +20,8 @@ import java.awt.image.RenderedImage;
 import javafx.scene.control.IndexedCell;
 import javafx.scene.control.Skin;
 import javafx.scene.control.skin.VirtualFlow;
+import javafx.scene.text.Font;
+import javafx.scene.text.FontWeight;
 
 
 /**
@@ -81,6 +83,7 @@ final class GridRow extends IndexedCell<Void> {
         flow = (GridViewSkin.Flow) owner;
         view = (GridView) owner.getParent();
         setPrefWidth(view.getContentWidth());
+        setFont(Font.font(null, FontWeight.BOLD, -1));      // Apply only to 
the header column.
     }
 
     /**
@@ -95,6 +98,10 @@ final class GridRow extends IndexedCell<Void> {
         super.updateIndex(row);
         y = view.toImageY(row);
         tileRow = view.toTileRow(row);
+        final Skin<?> skin = getSkin();
+        if (skin != null) {
+            ((GridRowSkin) skin).setRowIndex(row);
+        }
     }
 
     /**
diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridRowSkin.java
 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridRowSkin.java
index bd54c0f..b085114 100644
--- 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridRowSkin.java
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridRowSkin.java
@@ -19,9 +19,10 @@ package org.apache.sis.gui.coverage;
 import java.util.List;
 import java.util.ArrayList;
 import javafx.scene.Node;
+import javafx.scene.text.Text;
 import javafx.scene.control.skin.CellSkinBase;
 import javafx.collections.ObservableList;
-import javafx.scene.text.Text;
+import javafx.geometry.Pos;
 
 
 /**
@@ -41,6 +42,15 @@ final class GridRowSkin extends CellSkinBase<GridRow> {
      */
     GridRowSkin(final GridRow owner) {
         super(owner);
+        setRowIndex(owner.getIndex());
+    }
+
+    /**
+     * Invoked when the index to show in the header column changed.
+     */
+    final void setRowIndex(final int index) {
+        final Text header = (Text) getChildren().get(0);
+        header.setText(getSkinnable().view.formatHeaderValue(index));
     }
 
     /**
@@ -69,9 +79,6 @@ final class GridRowSkin extends CellSkinBase<GridRow> {
          * The first child is a javafx.scene.text.Text instance, which we use 
for row header.
          */
         final GridRow row = getSkinnable();
-        final ObservableList<Node> children = getChildren();
-        final Text header = (Text) children.get(0);
-        header.setText(String.valueOf(row.getIndex()));     // TODO: format
         /*
          * Get the beginning (pos) and end (limit) of the region to render. We 
create only the amount
          * of GridCell instances needed for rendering this region. We should 
not create cells for the
@@ -83,13 +90,24 @@ final class GridRowSkin extends CellSkinBase<GridRow> {
         final double cellWidth   = row.view.cellWidth.get();            // 
Includes the cell spacing.
         final double cellSpacing = row.view.cellSpacing.get();
         final double available   = cellWidth - cellSpacing;
-        double pos = row.flow.getHorizontalPosition();
-        final double limit = pos + row.flow.getWidth();
-        int column = (int) (pos / cellWidth);
+        double pos = row.flow.getHorizontalPosition();                  // 
Horizontal position in the virtual view.
+        final double limit = pos + row.flow.getWidth();                 // 
Horizontal position where to stop.
+        int column = (int) (pos / cellWidth);                           // 
Column index in the RenderedImage.
+        /*
+         * Set the position of the header cell, but not its content. The 
content has been set by
+         * `setRowIndex(int)` and does not need to be recomputed even during 
horizontal scroll.
+         */
+        final ObservableList<Node> children = getChildren();
+        final Text header = (Text) children.get(0);
+        header.resizeRelocate(pos + cellSpacing, y, headerWidth - cellSpacing, 
height);
+        pos += headerWidth;
+        /*
+         * For sample value, we need to recompute both the values and the 
position. Note that even if
+         * the cells appear at the same positions visually (with different 
content), they moved in the
+         * virtual flow if some scrolling occurred.
+         */
         int childIndex = 0;
         List<GridCell> newChildren = null;
-        header.resizeRelocate(pos, y, headerWidth, height);
-        pos += headerWidth;
         final int count = children.size();
         while (pos < limit) {
             final GridCell cell;
@@ -97,6 +115,7 @@ final class GridRowSkin extends CellSkinBase<GridRow> {
                 cell = (GridCell) children.get(childIndex);
             } else {
                 cell = new GridCell();
+                cell.setAlignment(Pos.CENTER_RIGHT);
                 if (newChildren == null) {
                     newChildren = new ArrayList<>(1 + (int) ((limit - pos) / 
cellWidth));
                 }
@@ -104,7 +123,7 @@ final class GridRowSkin extends CellSkinBase<GridRow> {
             }
             final String value = row.getSampleValue(column++);
             cell.updateItem(value, value == GridView.OUT_OF_BOUNDS);           
 // Identity comparison is okay here.
-            cell.resizeRelocate(pos + cellSpacing, 0, available, height);
+            cell.resizeRelocate(pos, 0, available, height);
             pos += cellWidth;
         }
         /*
diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridView.java
 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridView.java
index 598b169..e7b9b6f 100644
--- 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridView.java
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridView.java
@@ -17,9 +17,12 @@
 package org.apache.sis.gui.coverage;
 
 import java.util.Arrays;
+import java.text.NumberFormat;
+import java.text.FieldPosition;
 import java.awt.image.Raster;
 import java.awt.image.RenderedImage;
 import java.awt.image.SampleModel;
+import java.awt.image.DataBuffer;
 import javafx.beans.DefaultProperty;
 import javafx.beans.property.DoubleProperty;
 import javafx.beans.property.IntegerProperty;
@@ -30,6 +33,8 @@ import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.value.ObservableValue;
 import javafx.scene.control.Control;
 import javafx.scene.control.Skin;
+import javafx.scene.paint.Color;
+import javafx.scene.paint.Paint;
 import org.apache.sis.util.ArgumentChecks;
 import org.apache.sis.coverage.grid.GridCoverage;
 
@@ -84,9 +89,10 @@ public class GridView extends Control {
     private int tileGridXOffset, tileGridYOffset;
 
     /**
-     * The {@link #imageProperty} tiles, fetched when first needed. All {@code 
Raster[]} array element and
-     * {@code Raster} sub-array elements are initially {@code null} and 
created when first needed.
-     * This field is null if and only if the image is null.
+     * The {@link #imageProperty} tiles (fetched when first needed), or {@code 
null} if the image is null.
+     * All {@code Raster[]} array element and {@code Raster} sub-array 
elements are initially {@code null}
+     * and initialized when first needed. We store {@link 
RenderedImage#getTile(int, int)} results because
+     * we do not know if calling that method causes a costly computation or 
not (it depends on image class).
      */
     private Raster[][] tiles;
 
@@ -140,22 +146,72 @@ public class GridView extends Control {
     public final DoubleProperty cellSpacing;
 
     /**
+     * The background color of row and column headers.
+     *
+     * <p>We do not define getter/setter for this property; use {@link 
ObjectProperty#set(Object)}
+     * directly instead. We omit the "Property" suffix for making this 
operation more natural.</p>
+     */
+    public final ObjectProperty<Paint> headerBackground;
+
+    /**
+     * The formatter to use for writing header values (row and column numbers) 
or the sample values.
+     */
+    private final NumberFormat headerFormat, cellFormat;
+
+    /**
+     * Required when invoking {@link #cellFormat} methods but not used by this 
class.
+     * This argument is not allowed to be {@code null}, so we create an 
instance once
+     * and reuse it at each method call.
+     */
+    private final FieldPosition formatField;
+
+    /**
+     * A buffer for writing sample values with {@link #cellFormat}, reused for 
each value to format.
+     */
+    private final StringBuffer buffer;
+
+    /**
+     * The last value formatted by {@link #cellFormat}. We keep this 
information because it happens often
+     * that the same value is repeated for many cells, especially in area 
containing fill or missing values.
+     * If the value is the same, we will reuse the {@link #lastValueAsText}.
+     *
+     * <p>Note: the use of {@code double} is sufficient since rendered images 
can not store {@code long} values,
+     * so there is no precision lost that we could have with conversions from 
{@code long} to {@code double}.</p>
+     */
+    private double lastValue;
+
+    /**
+     * The formatting of {@link #lastValue}.
+     */
+    private String lastValueAsText;
+
+    /**
+     * Whether the sample values are integers.
+     */
+    private boolean isInteger;
+
+    /**
      * Creates an initially empty grid view. The content can be set after
      * construction by a call to {@link #setImage(RenderedImage)}.
      */
     public GridView() {
-        imageProperty = new SimpleObjectProperty<>(this, "image");
-        bandProperty  = new SimpleIntegerProperty (this, "band");
-        headerWidth   = new SimpleDoubleProperty  (this, "headerWidth", 60);
-        cellWidth     = new SimpleDoubleProperty  (this, "cellWidth",   60);
-        cellHeight    = new SimpleDoubleProperty  (this, "cellHeight",  20);
-        cellSpacing   = new SimpleDoubleProperty  (this, "cellSpacing",  2);
+        imageProperty    = new SimpleObjectProperty<>(this, "image");
+        bandProperty     = new SimpleIntegerProperty (this, "band");
+        headerWidth      = new SimpleDoubleProperty  (this, "headerWidth", 60);
+        cellWidth        = new SimpleDoubleProperty  (this, "cellWidth",   60);
+        cellHeight       = new SimpleDoubleProperty  (this, "cellHeight",  20);
+        cellSpacing      = new SimpleDoubleProperty  (this, "cellSpacing",  4);
+        headerBackground = new SimpleObjectProperty<>(this, 
"headerBackground", Color.GAINSBORO);
+        headerFormat     = NumberFormat.getIntegerInstance();
+        cellFormat       = NumberFormat.getInstance();
+        formatField      = new FieldPosition(0);
+        buffer           = new StringBuffer();
+        tileWidth        = 1;
+        tileHeight       = 1;       // For avoiding division by zero.
 
+        setMinSize(120, 40);        // 2 cells on each dimension.
         imageProperty.addListener(this::startImageLoading);
         // Other listeners registered by GridViewSkin.Flow.
-
-        tileWidth  = 1;
-        tileHeight = 1;     // For avoiding division by zero.
     }
 
     /**
@@ -205,8 +261,9 @@ public class GridView extends Control {
      */
     public final void setBand(final int index) {
         final RenderedImage image = getImage();
-        if (image != null) {
-            ArgumentChecks.ensureBetween("band", 0, 
image.getSampleModel().getNumBands() - 1, index);
+        final SampleModel sm;
+        if (image != null && (sm = image.getSampleModel()) != null) {
+            ArgumentChecks.ensureBetween("band", 0, sm.getNumBands() - 1, 
index);
         } else {
             ArgumentChecks.ensurePositive("band", index);
         }
@@ -226,9 +283,10 @@ public class GridView extends Control {
     private void startImageLoading(final ObservableValue<? extends 
RenderedImage> property,
                                    final RenderedImage previous, final 
RenderedImage image)
     {
-        tiles  = null;       // Let garbage collector dispose the rasters.
-        width  = 0;
-        height = 0;
+        tiles     = null;       // Let garbage collector dispose the rasters.
+        width     = 0;
+        height    = 0;
+        isInteger = false;
         if (image != null) {
             width           = image.getWidth();
             height          = image.getHeight();
@@ -242,14 +300,53 @@ public class GridView extends Control {
             tileGridYOffset = Math.toIntExact(((long) 
image.getTileGridYOffset()) - minY + ((long) tileHeight) * minTileY);
             numXTiles       = image.getNumXTiles();
             tiles           = new Raster[image.getNumYTiles()][];
-            final int n     = image.getSampleModel().getNumBands();
-            if (bandProperty.get() >= n) {
-                bandProperty.set(n - 1);
-            }
-            final Skin<?> skin = getSkin();
-            if (skin instanceof GridViewSkin) {
-                ((GridViewSkin) skin).updateItemCount();
+            final SampleModel sm = image.getSampleModel();
+            if (sm != null) {                               // Should never be 
null, but we are paranoiac.
+                final int numBands = sm.getNumBands();
+                if (bandProperty.get() >= numBands) {
+                    bandProperty.set(numBands - 1);
+                }
+                final int dataType = sm.getDataType();
+                isInteger = (dataType >= DataBuffer.TYPE_BYTE && dataType <= 
DataBuffer.TYPE_INT);
+                if (isInteger) {
+                    cellFormat.setMaximumFractionDigits(0);
+                } else {
+                    /*
+                     * TODO: compute the number of fraction digits from a 
"sampleResolution" image property
+                     * (of type float[] or double[]) if present. Provide a 
widget allowing user to set pattern.
+                     */
+                    cellFormat.setMinimumFractionDigits(1);
+                    cellFormat.setMaximumFractionDigits(1);
+                }
+                formatChanged(false);
             }
+            contentChanged(true);
+        }
+    }
+
+    /**
+     * Invoked when the content may have changed. If {@code all} is {@code 
true}, then everything
+     * may have changed including the number of rows and columns. If {@code 
all} is {@code false}
+     * then the number of rows and columns is assumed the same.
+     */
+    final void contentChanged(final boolean all) {
+        final Skin<?> skin = getSkin();             // May be null if the view 
is not yet shown.
+        if (skin instanceof GridViewSkin) {         // Could be a user 
instance (not recommended).
+            ((GridViewSkin) skin).contentChanged(all);
+        }
+    }
+
+    /**
+     * Invoked when the {@link #cellFormat} configuration changed.
+     *
+     * @param  notify  whether to notify the renderer about the change. Can be 
{@code false}
+     *                 if the renderer is going to be notified anyway by 
another method call.
+     */
+    private void formatChanged(final boolean notify) {
+        buffer.setLength(0);
+        lastValueAsText = cellFormat.format(lastValue, buffer, 
formatField).toString();
+        if (notify) {
+            contentChanged(false);
         }
     }
 
@@ -264,6 +361,8 @@ public class GridView extends Control {
     /**
      * Returns the number of rows in the image. This is also the number of 
rows in the
      * {@link GridViewSkin} virtual flow, which is using a vertical primary 
direction.
+     *
+     * @see javafx.scene.control.skin.VirtualContainerBase#getItemCount()
      */
     final int getImageHeight() {
         return height;
@@ -324,16 +423,36 @@ public class GridView extends Control {
             }
             final int x = Math.addExact(column, minX);
             final int b = getBand();
-            final Number value;
-            // TODO: also return Float or Integer.
-            value = tile.getSampleDouble(x, y, b);
-            // TODO: format.
-            return value.toString();
+            buffer.setLength(0);
+            if (isInteger) {
+                final int  integer = tile.getSample(x, y, b);
+                final double value = integer;
+                if (Double.doubleToRawLongBits(value) != 
Double.doubleToRawLongBits(lastValue)) {
+                    // The `format` method invoked here is not the same than 
in `double` case.
+                    lastValueAsText = cellFormat.format(integer, buffer, 
formatField).toString();
+                    lastValue = value;
+                }
+            } else {
+                final double value = tile.getSampleDouble(x, y, b);
+                if (Double.doubleToRawLongBits(value) != 
Double.doubleToRawLongBits(lastValue)) {
+                    lastValueAsText = cellFormat.format(value, buffer, 
formatField).toString();
+                    lastValue = value;
+                }
+            }
+            return lastValueAsText;
         }
         return OUT_OF_BOUNDS;
     }
 
     /**
+     * Formats a row index or column index.
+     */
+    final String formatHeaderValue(final int index) {
+        buffer.setLength(0);
+        return headerFormat.format(index, buffer, formatField).toString();
+    }
+
+    /**
      * Creates a new instance of the skin responsible for rendering this grid 
view.
      * From the perspective of this {@link Control}, the {@link Skin} is a 
black box.
      * It listens and responds to changes in state of this grid view. This 
method is
diff --git 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridViewSkin.java
 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridViewSkin.java
index 7f88e60..8b81e0d 100644
--- 
a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridViewSkin.java
+++ 
b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/GridViewSkin.java
@@ -19,9 +19,12 @@ package org.apache.sis.gui.coverage;
 import java.awt.image.RenderedImage;
 import javafx.beans.value.ChangeListener;
 import javafx.beans.value.ObservableValue;
+import javafx.collections.ObservableList;
 import javafx.scene.Node;
+import javafx.scene.control.ScrollBar;
 import javafx.scene.control.skin.VirtualFlow;
 import javafx.scene.control.skin.VirtualContainerBase;
+import javafx.scene.shape.Rectangle;
 
 
 /**
@@ -56,32 +59,86 @@ final class GridViewSkin extends 
VirtualContainerBase<GridView, GridRow> {
         view.cellWidth  .addListener(this::cellWidthChanged);
         view.headerWidth.addListener(this::cellWidthChanged);
         /*
+         * Rectangles for filling the background of the cells in the header 
row and header column.
+         * Those rectangles will be resized when the GridView size changes or 
cells size changes.
+         */
+        final Rectangle topBackground  = new Rectangle();
+        final Rectangle leftBackground = new Rectangle();
+        topBackground.setHeight(view.cellHeight.get());
+        leftBackground.setY(topBackground.getHeight());
+        leftBackground.widthProperty().bind(view.headerWidth);
+        leftBackground.fillProperty() .bind(view.headerBackground);
+        topBackground .fillProperty() .bind(view.headerBackground);
+        /*
          * The list of children is initially empty. We need to
          * add the virtual flow, otherwise nothing will appear.
          */
-        getChildren().add(flow);
+        getChildren().addAll(topBackground, leftBackground, flow);
+        flow.widthProperty() .addListener(this::gridSizeChanged);
+        flow.heightProperty().addListener(this::gridSizeChanged);
+    }
+
+    /**
+     * Invoked when the width or height of {@link GridView} changed. This 
method recomputes the size of
+     * the rectangles used for painting backgrounds. We listen to changes in 
width and height together
+     * because a change of width may show or hide the horizontal scroll bar, 
which change the height
+     * (and conversely for the vertical scroll bar).
+     */
+    private void gridSizeChanged(ObservableValue<? extends Number> property, 
Number oldValue, Number newValue) {
+        final Flow flow = (Flow) getVirtualFlow();
+        final ObservableList<Node> children = getChildren();
+        Rectangle r;
+        r = (Rectangle) children.get(0); r.setWidth (flow.getVisibleWidth()  - 
r.getX());
+        r = (Rectangle) children.get(1); r.setHeight(flow.getVisibleHeight() - 
r.getY());
     }
 
     /**
-     * Invoked when the value of {@link GridView#cellHeight} property changed.
+     * Invoked when the value of {@link GridView#cellHeight} property changed. 
This method copies the new value
+     * into {@link VirtualFlow#fixedCellSizeProperty()} after bounds check, 
then adjusts the size and position
+     * of rectangles filling the header background.
      */
     private void cellHeightChanged(ObservableValue<? extends Number> property, 
Number oldValue, Number newValue) {
-        getVirtualFlow().setFixedCellSize(Math.max(GridView.MIN_CELL_SIZE, 
newValue.intValue()));
+        final Flow flow = (Flow) getVirtualFlow();
+        final ObservableList<Node> children = getChildren();
+        final double height = Math.max(GridView.MIN_CELL_SIZE, 
newValue.doubleValue());
+        flow.setFixedCellSize(height);
+        Rectangle r;
+        r = (Rectangle) children.get(0); r.setHeight(height);
+        r = (Rectangle) children.get(1); r.setHeight(flow.getVisibleHeight() - 
height); r.setY(height);
     }
 
     /**
-     * Invoked when the cell width or cell spacing changed. This method 
notifies all children of the new width.
+     * Invoked when the cell width or cell spacing changed.
+     * This method notifies all children about the new width.
      */
     private void cellWidthChanged(ObservableValue<? extends Number> property, 
Number oldValue, Number newValue) {
         final double width = getSkinnable().getContentWidth();
         for (final Node child : getChildren()) {
-            if (child instanceof GridRow) {             // The first instance 
if not a GridRow.
+            if (child instanceof GridRow) {             // The first instances 
are not a GridRow.
                 ((GridRow) child).setPrefWidth(width);
             }
         }
     }
 
     /**
+     * Invoked when the content may have changed. If {@code all} is {@code 
true}, then everything
+     * may have changed including the number of rows and columns. If {@code 
all} is {@code false}
+     * then the number of rows and columns is assumed the same.
+     *
+     * @see GridView#contentChanged(boolean)
+     */
+    final void contentChanged(final boolean all) {
+        if (all) {
+            updateItemCount();
+        }
+        /*
+         * Following call may be redundant with `updateItemCount()` except if 
the number of
+         * rows did not changed, in which case `updateItemCount()` may have 
sent no event.
+         */
+        ((Flow) getVirtualFlow()).changed(null, null, null);
+    }
+
+    /**
      * Creates the virtual flow used by this {@link GridViewSkin}. The virtual 
flow
      * created by this method registers a listener for horizontal scroll bar 
events.
      */
@@ -116,6 +173,30 @@ final class GridViewSkin extends 
VirtualContainerBase<GridView, GridRow> {
         }
 
         /**
+         * Returns the width of the view port area, not counting the vertical 
scroll bar.
+         */
+        final double getVisibleWidth() {
+            double width = getWidth();
+            final ScrollBar bar = getVbar();
+            if (bar.isVisible()) {
+                width -= bar.getWidth();
+            }
+            return width;
+        }
+
+        /**
+         * Returns the height of the view port area, not counting the 
horizontal scroll bar.
+         */
+        final double getVisibleHeight() {
+            double height = getHeight();
+            final ScrollBar bar = getHbar();
+            if (bar.isVisible()) {
+                height -= bar.getHeight();
+            }
+            return height;
+        }
+
+        /**
          * Invoked when the content to show changed because of a change in a 
property.
          * The most important event is a change in the position of horizontal 
scroll bar,
          * which is handled as a change of content because we will need to 
change values
@@ -160,8 +241,9 @@ final class GridViewSkin extends 
VirtualContainerBase<GridView, GridRow> {
     }
 
     /**
-     * Called during the layout pass of the scene graph. Current 
implementation sets the virtual
-     * flow size to the given size.
+     * Called during the layout pass of the scene graph. The (x,y) coordinates 
are usually zero
+     * and the (width, height) are the size of the control as shown (not the 
full content size).
+     * Current implementation sets the virtual flow size to the given size.
      */
     @Override
     protected void layoutChildren(final double x, final double y, final double 
width, final double height) {

Reply via email to