Skip to content

Commit

Permalink
topcat: simplify the way coordinate autopopulation works
Browse files Browse the repository at this point in the history
CoordPanels now have their coordinates filled in to sensible default
values only when explicitly commanded to by GangLayerControl,
rather than autopopulation being a property of the coordpanel itself.
This is more sensible and easier to manage.
  • Loading branch information
mbtaylor committed Dec 12, 2013
1 parent 3dc769a commit 444b4aa
Show file tree
Hide file tree
Showing 14 changed files with 60 additions and 66 deletions.
35 changes: 21 additions & 14 deletions topcat/src/main/uk/ac/starlink/topcat/plot2/CoordPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,17 @@ public class CoordPanel {
private final Coord[] coords_;
private final ActionForwarder forwarder_;
private final JComboBox[][] colSelectors_;
private final boolean autoPopulate_;
private final JComponent panel_;

/**
* Constructor.
*
* @param coords coordinate definitions for which values are required
* @param autoPopulate if true, some attempt will be made to
* fill in the fields with non-blank values
* when a table is selected
*/
public CoordPanel( Coord[] coords, boolean autoPopulate ) {
public CoordPanel( Coord[] coords ) {
panel_ = new JPanel( new BorderLayout() );
coords_ = coords;
forwarder_ = new ActionForwarder();
autoPopulate_ = autoPopulate;

/* Place entry components for each required coordinate. */
int nc = coords.length;
Expand Down Expand Up @@ -135,8 +130,8 @@ public void setTable( TopcatModel tcModel ) {
int nu = colsels.length;
for ( int iu = 0; iu < nu; iu++ ) {
JComboBox cs = colsels[ iu ];
cs.setSelectedItem( null );
if ( tcModel == null ) {
cs.setSelectedItem( null );
cs.setEnabled( false );
}
else {
Expand All @@ -145,19 +140,31 @@ public void setTable( TopcatModel tcModel ) {
userInfos[ iu ]
.getContentClass(), true );
cs.setModel( model );

/* In case of autopopulate, just pick the first few
* columns in the table and fill them in for the
* required coordinates. */
if ( autoPopulate_ && is < model.getSize() ) {
cs.setSelectedIndex( is++ );
}
cs.setEnabled( true );
}
}
}
}

/**
* Makes some attempt to fill in the fields with non-blank values.
* The default implementation fills in the first few suitable columns,
* but subclasses are encouraged to override this behaviour if something
* smarter is possible.
*/
public void autoPopulate() {
int is = 1;
for ( int ic = 0; ic < coords_.length; ic++ ) {
JComboBox[] colsels = colSelectors_[ ic ];
for ( int iu = 0; iu < colsels.length; iu++ ) {
JComboBox cs = colsels[ iu ];
if ( is < cs.getItemCount() ) {
cs.setSelectedIndex( is++ );
}
}
}
}

/**
* Returns the coordinate values currently selected in this panel.
* If there is insufficient information to contribute to a plot
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ private static class CubePlotTypeGui
}
public PositionCoordPanel createPositionCoordPanel( int npos ) {
return SimplePositionCoordPanel
.createPanel( PLOT_TYPE.getPointDataGeoms()[ 0 ],
npos, npos == 1 );
.createPanel( PLOT_TYPE.getPointDataGeoms()[ 0 ], npos );
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ private GangLayerControl createGangControl( int npos, Icon icon ) {
return plotterList != null && plotterList.size() > 0
? new GangLayerControl( plotTypeGui_
.createPositionCoordPanel( npos ),
npos == 1,
plotterList.toArray( new Plotter[ 0 ] ),
baseConfigger_, nextSupplier_,
tcListener_, icon )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
public class GangLayerControl extends TabberControl implements LayerControl {

private final PositionCoordPanel posCoordPanel_;
private final boolean autoPopulate_;
private final Configger baseConfigger_;
private final ConfigKey[] subsetKeys_;
private final TablesListComboBox tableSelector_;
Expand All @@ -74,6 +75,9 @@ public class GangLayerControl extends TabberControl implements LayerControl {
*
* @param posCoordPanel panel for entering table and basic positional
* coordinates
* @param autoPopulate if true, when the table is changed an attempt
* will be made to initialise the coordinate fields
* with some suitable values
* @param plotters plotter objects providing different plot layer
* type options
* @param baseConfigger configuration source for some global config
Expand All @@ -85,11 +89,13 @@ public class GangLayerControl extends TabberControl implements LayerControl {
* @param controlIcon icon for control stack
*/
public GangLayerControl( PositionCoordPanel posCoordPanel,
boolean autoPopulate,
Plotter[] plotters, Configger baseConfigger,
NextSupplier nextSupplier,
TopcatListener tcListener, Icon controlIcon ) {
super( null, controlIcon );
posCoordPanel_ = posCoordPanel;
autoPopulate_ = autoPopulate;
baseConfigger_ = baseConfigger;
subsetKeys_ = nextSupplier.getKeys();
final TopcatListener externalTcListener = tcListener;
Expand Down Expand Up @@ -418,6 +424,9 @@ private void tableChanged() {
/* Message the position selection panel and form controls so they
* can update their lists of selectable columns etc. */
posCoordPanel_.setTable( tcModel_ );
if ( autoPopulate_ ) {
posCoordPanel_.autoPopulate();
}
FormControl[] fcs = getActiveFormControls();
for ( int ifc = 0; ifc < fcs.length; ifc++ ) {
fcs[ ifc ].setTable( tcModel_, subsetManager_ );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ public ModeFormControl( Configger baseConfigger, ModePlotter[] plotters,
* and prepare a panel for entering them. */
List<Coord> commonExtraCoordList = getCommonCoords( plotters );
commonExtraCoordPanel_ =
new CoordPanel( commonExtraCoordList.toArray( new Coord[ 0 ] ),
false );
new CoordPanel( commonExtraCoordList.toArray( new Coord[ 0 ] ) );
if ( ! commonExtraCoordList.isEmpty() ) {
commonExtraCoordPanel_
.getComponent()
Expand Down Expand Up @@ -336,7 +335,7 @@ private static class ModeState {
ModeState( ModePlotter plotter, Coord[] modeCoords,
ConfigKey[] configKeys ) {
plotter_ = plotter;
modeCoordPanel_ = new CoordPanel( modeCoords, false );
modeCoordPanel_ = new CoordPanel( modeCoords );
if ( modeCoords.length > 0 ) {
modeCoordPanel_.getComponent()
.setBorder( BorderFactory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ private static class PlanePlotTypeGui
}
public PositionCoordPanel createPositionCoordPanel( int npos ) {
return SimplePositionCoordPanel
.createPanel( PLOT_TYPE.getPointDataGeoms()[ 0 ],
npos, npos == 1 );
.createPanel( PLOT_TYPE.getPointDataGeoms()[ 0 ], npos );
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ else if ( plotter.getPositionCount() == 0 ) {
protected LayerControl createLayerControl() {
PositionCoordPanel coordPanel =
new SimplePositionCoordPanel( plotter.getExtraCoords(),
false, null );
null );
return new BasicCoordLayerControl( plotter, coordPanel );
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,9 @@ public abstract class PositionCoordPanel extends CoordPanel {
* Constructor.
*
* @param coords coordinate definitions for which values are required
* @param autoPopulate if true, some attempt will be made to
* fill in the fields with non-blank values
* when a table is selected
*/
protected PositionCoordPanel( Coord[] coords, boolean autoPopulate ) {
super( coords, autoPopulate );
protected PositionCoordPanel( Coord[] coords ) {
super( coords );
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public SimpleFormControl( Configger baseConfigger, Plotter plotter ) {
super( baseConfigger );
plotter_ = plotter;
Coord[] extraCoords = plotter.getExtraCoords();
extraCoordPanel_ = new CoordPanel( extraCoords, false );
extraCoordPanel_ = new CoordPanel( extraCoords );
if ( extraCoords.length > 0 ) {
extraCoordPanel_.getComponent()
.setBorder( AuxWindow
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@ public class SimplePositionCoordPanel extends PositionCoordPanel {
* Constructor.
*
* @param coords coordinate definitions for which values are required
* @param autoPopulate if true, some attempt will be made to
* fill in the fields with non-blank values
* when a table is selected
* @param geom fixed data geom
*/
public SimplePositionCoordPanel( Coord[] coords, boolean autoPopulate,
DataGeom geom ) {
super( coords, autoPopulate );
public SimplePositionCoordPanel( Coord[] coords, DataGeom geom ) {
super( coords );
geom_ = geom;
}

Expand All @@ -40,13 +36,10 @@ public DataGeom getDataGeom() {
*
* @param geom provides description of positional coordinates
* @param npos number of positional groups to include
* @param autoPopulate if true, some attempt may be made to
* fill in the fields with non-blank values
* when a table is selected
*/
public static SimplePositionCoordPanel createPanel( DataGeom geom, int npos,
boolean autoPopulate ) {
public static SimplePositionCoordPanel createPanel( DataGeom geom,
int npos ) {
Coord[] coords = multiplyCoords( geom.getPosCoords(), npos );
return new SimplePositionCoordPanel( coords, autoPopulate, geom );
return new SimplePositionCoordPanel( coords, geom );
}
}
33 changes: 13 additions & 20 deletions topcat/src/main/uk/ac/starlink/topcat/plot2/SkyPlotWindow.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private static class SkyPlotTypeGui
}

public PositionCoordPanel createPositionCoordPanel( int npos ) {
return new SkyPositionCoordPanel( npos, npos == 1 ) {
return new SkyPositionCoordPanel( npos ) {
SkySys getViewSystem() {
return axisControl_.getViewSystem();
}
Expand All @@ -84,22 +84,17 @@ SkySys getViewSystem() {
private static abstract class SkyPositionCoordPanel
extends PositionCoordPanel {

private final boolean autoPopulate_;
private final Specifier<SkySys> dataSysSpecifier_;
private final JComponent panel_;

/**
* Constructor.
*
* @param npos number of groups of positional coordinates for entry
* @param autoPopulate true if it should be filled in with
* coordinates from an available table when possible
*/
SkyPositionCoordPanel( int npos, boolean autoPopulate ) {
SkyPositionCoordPanel( int npos ) {
super( multiplyCoords( SkyDataGeom.createGeom( null, null )
.getPosCoords(), npos ),
autoPopulate );
autoPopulate_ = autoPopulate;
.getPosCoords(), npos ) );

/* But add a data sky system selector. */
ConfigSpecifier cspec =
Expand Down Expand Up @@ -138,18 +133,16 @@ public DataGeom getDataGeom() {
return SkyDataGeom.createGeom( getDataSystem(), getViewSystem() );
}

public void setTable( TopcatModel table ) {
super.setTable( table );
if ( autoPopulate_ ) {
ColumnDataComboBoxModel lonModel = getColumnSelector( 0, 0 );
ColumnDataComboBoxModel latModel = getColumnSelector( 0, 1 );
ColPopulator cp = new ColPopulator( lonModel, latModel );
SkySys currentSys = dataSysSpecifier_.getSpecifiedValue();
SkySys sys = new ColPopulator( lonModel, latModel )
.attemptPopulate( currentSys );
if ( sys != null && sys != currentSys ) {
dataSysSpecifier_.setSpecifiedValue( sys );
}
@Override
public void autoPopulate() {
ColumnDataComboBoxModel lonModel = getColumnSelector( 0, 0 );
ColumnDataComboBoxModel latModel = getColumnSelector( 0, 1 );
ColPopulator cp = new ColPopulator( lonModel, latModel );
SkySys currentSys = dataSysSpecifier_.getSpecifiedValue();
SkySys sys = new ColPopulator( lonModel, latModel )
.attemptPopulate( currentSys );
if ( sys != null && sys != currentSys ) {
dataSysSpecifier_.setSpecifiedValue( sys );
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ public class SpectrogramLayerControl extends BasicCoordLayerControl {
*/
public SpectrogramLayerControl( SpectrogramPlotter plotter ) {
super( plotter,
new SimplePositionCoordPanel( plotter.getExtraCoords(),
false, null ) );
new SimplePositionCoordPanel( plotter.getExtraCoords(), null ) );
plotter_ = plotter;
assert plotter.getPositionCount() == 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ private static class SpherePlotTypeGui
}
public PositionCoordPanel createPositionCoordPanel( int npos ) {
return SimplePositionCoordPanel
.createPanel( PLOT_TYPE.getPointDataGeoms()[ 0 ],
npos, npos == 1 );
.createPanel( PLOT_TYPE.getPointDataGeoms()[ 0 ], npos );
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ private static class TimePlotTypeGui
}
public PositionCoordPanel createPositionCoordPanel( int npos ) {
return SimplePositionCoordPanel
.createPanel( PLOT_TYPE.getPointDataGeoms()[ 0 ],
npos, npos == 1 );
.createPanel( PLOT_TYPE.getPointDataGeoms()[ 0 ], npos );
}
}
}

0 comments on commit 444b4aa

Please sign in to comment.