Skip to content

Commit

Permalink
Addressed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Rylern
Rylern committed Sep 7, 2023
1 parent cb17472 commit 2183e13
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 49 deletions.
9 changes: 5 additions & 4 deletions ...-bioformats/src/main/java/qupath/lib/images/servers/bioformats/BioFormatsImageServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.net.URISyntaxException;
import java.util.*;
import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;

import javax.imageio.ImageIO;

Expand Down Expand Up @@ -137,7 +138,7 @@ public class BioFormatsImageServer extends AbstractTileableImageServer {
/**
* Pool of readers for use with this server.
*/
private final ReaderPool readerPool;
private final ReaderPool<BufferedImage> readerPool;

/**
* ColorModel to use with all BufferedImage requests.
Expand Down Expand Up @@ -206,7 +207,7 @@ private BioFormatsImageServer(URI uri, final BioFormatsServerOptions options, St
BioFormatsArgs bfArgs = BioFormatsArgs.parse(args);

var metadata = (OMEPyramidStore) MetadataTools.createOMEXMLMetadata();
readerPool = new ReaderPool(
readerPool = new ReaderPool<>(
Math.max(1, options == null ? Runtime.getRuntime().availableProcessors() : options.getMaxReaders()),
() -> {
logger.debug("Creating Bio-Formats reader wrapper");
Expand Down Expand Up @@ -379,7 +380,7 @@ private BioFormatsImageServer(URI uri, final BioFormatsServerOptions options, St
int nInstruments = meta.getInstrumentCount();
for (int i = 0; i < nInstruments; i++) {
int nObjectives = meta.getObjectiveCount(i);
for (int o = 0; 0 < nObjectives; o++) {
for (int o = 0; o < nObjectives; o++) {
if (objectiveID.equals(meta.getObjectiveID(i, o))) {
instrumentIndex = i;
objectiveIndex = o;
Expand Down Expand Up @@ -820,7 +821,7 @@ public int getSeries() {

@Override
public BufferedImage readTile(TileRequest tileRequest) throws IOException {
return readerPool.openImage(tileRequest, series, nChannels(), isRGB(), colorModel);
return readerPool.openImage(tileRequest, IntStream.range(0, nChannels()).toArray(), isRGB(), colorModel, series);
}

@Override
Expand Down
13 changes: 10 additions & 3 deletions ...ioformats/src/main/java/qupath/lib/images/servers/bioformats/BioFormatsServerOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,23 @@ enum UseBioformats {
// private boolean requestChannelZCorrectionVSI = false;

private BioFormatsServerOptions() {}


/**
* @return the maximal number of readers that can be created to read the image
*/
public int getMaxReaders() {
if (maxReaders <= 0) {
maxReaders = Math.min(Math.max(2, Runtime.getRuntime().availableProcessors()), 32);
logger.info("Setting max Bio-Formats readers to {}", maxReaders);
}
return requestParallelization ? maxReaders : 1;
}

void setMaxReaders(int maxReaders) {

/**
* Set the maximal number of readers that can be created to read the image
* @param maxReaders the new maximal number of readers
*/
public void setMaxReaders(int maxReaders) {
this.maxReaders = maxReaders;
}

Expand Down
4 changes: 2 additions & 2 deletions ...s/src/main/java/qupath/lib/images/servers/bioformats/readers/BioFormatsReaderWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ public byte[][] getPixelValues(TileRequest tileRequest, int series) throws IOExc
}

@Override
public ImageData getPixelValues(int series) throws IOException {
public ImageInfo getPixelValues(int series) throws IOException {
int previousSeries = reader.getSeries();

try {
Expand All @@ -273,7 +273,7 @@ public ImageData getPixelValues(int series) throws IOException {
}

//TODO: Handle color transforms here, or in the display of labels/macro images - in case this isn't RGB
return new ImageData(
return new ImageInfo(
reader.openBytes(reader.getIndex(0, 0, 0)),
reader.getSizeX(),
reader.getSizeY(),
Expand Down
2 changes: 1 addition & 1 deletion ...servers/bioformats/readers/ImageData.java → ...servers/bioformats/readers/ImageInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
* @param height the height of the image
* @param isInterleaved whether pixels are interleaved
*/
public record ImageData (byte[] data, int width, int height, boolean isInterleaved) {}
public record ImageInfo(byte[] data, int width, int height, boolean isInterleaved) {}
27 changes: 11 additions & 16 deletions ...oformats/src/main/java/qupath/lib/images/servers/bioformats/readers/OMEReaderWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,13 @@
* Wrapper suited for readers that return arrays of bytes when reading pixel values.
* </p>
*/
public abstract class OMEReaderWrapper implements ReaderWrapper {
public abstract class OMEReaderWrapper implements ReaderWrapper<BufferedImage> {

@Override
public BufferedImage getImage(TileRequest tileRequest, int series, int numberOfChannels, boolean isRGB, ColorModel colorModel) throws IOException {
if (tileRequest.getTileWidth() <= 0 || tileRequest.getTileHeight() <= 0) {
throw new IllegalArgumentException(
"Unable to request pixels for region with down sampled size " + tileRequest.getTileWidth() + " x " + tileRequest.getTileHeight()
);
}

public BufferedImage getImage(TileRequest tileRequest, int[] channels, boolean isRGB, ColorModel colorModel, int series) throws IOException {
byte[][] pixelValues = getPixelValues(tileRequest, series);

if ((isRGB() && isRGB) || numberOfChannels == 1) {
if ((isRGB() && isRGB) || channels.length == 1) {
return openImage(pixelValues[0], tileRequest.getTileWidth(), tileRequest.getTileHeight(), isInterleaved());
} else {
DataBuffer dataBuffer = bytesToDataBuffer(
Expand All @@ -43,7 +37,7 @@ public BufferedImage getImage(TileRequest tileRequest, int series, int numberOfC
createSampleModel(
tileRequest,
dataBuffer,
numberOfChannels,
channels.length,
series
),
dataBuffer,
Expand All @@ -56,13 +50,13 @@ public BufferedImage getImage(TileRequest tileRequest, int series, int numberOfC

@Override
public BufferedImage getImage(int series) throws IOException {
ImageData imageData = getPixelValues(series);
ImageInfo imageInfo = getPixelValues(series);

return openImage(
imageData.data(),
imageData.width(),
imageData.height(),
imageData.isInterleaved()
imageInfo.data(),
imageInfo.width(),
imageInfo.height(),
imageInfo.isInterleaved()
);
}

Expand Down Expand Up @@ -93,7 +87,7 @@ public BufferedImage getImage(int series) throws IOException {
* @return the pixel values corresponding to this parameter
* @throws IOException when a reading error occurs
*/
protected abstract ImageData getPixelValues(int series) throws IOException;
protected abstract ImageInfo getPixelValues(int series) throws IOException;

/**
* @return the byte order of the arrays returned by {@link #getPixelValues(TileRequest, int)}
Expand Down Expand Up @@ -294,6 +288,7 @@ private SampleModel createSampleModel(
ind++;
}
}

// TODO: Check this! It works for the only test image I have... (2 channels with 3 samples each)
// I would guess it fails if pixelStride does not equal nSamples, and if nSamples is different for different 'channels' -
// but I don't know if this occurs in practice.
Expand Down
52 changes: 35 additions & 17 deletions ...ion-bioformats/src/main/java/qupath/lib/images/servers/bioformats/readers/ReaderPool.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* It must be {@link #close() closed} once no longer used.
* </p>
*/
public class ReaderPool implements AutoCloseable {
public class ReaderPool<T> implements AutoCloseable {

private static final Logger logger = LoggerFactory.getLogger(ReaderPool.class);
private final static int MIN_NUMBER_OF_READERS = 2;
Expand All @@ -29,9 +29,9 @@ public class ReaderPool implements AutoCloseable {
private final AtomicInteger numberOfReaderWrappers = new AtomicInteger(0);
private volatile boolean isClosed = false;
private final int maxNumberOfReaders;
private final ArrayBlockingQueue<ReaderWrapper> availableReaderWrappers;
private final Callable<ReaderWrapper> readerWrapperSupplier;
private ReaderWrapper dedicatedReaderWrapper;
private final ArrayBlockingQueue<ReaderWrapper<T>> availableReaderWrappers;
private final Callable<ReaderWrapper<T>> readerWrapperSupplier;
private ReaderWrapper<T> dedicatedReaderWrapper;

/**
* Creates a new pool of readers.
Expand All @@ -41,7 +41,7 @@ public class ReaderPool implements AutoCloseable {
* @param readerWrapperSupplier the function that supplies {@link ReaderWrapper ReaderWrappers} to this pool.
* The supplied reader wrappers will be automatically closed when this reader pool is closed.
*/
public ReaderPool(int maxNumberOfReaders, Callable<ReaderWrapper> readerWrapperSupplier) {
public ReaderPool(int maxNumberOfReaders, Callable<ReaderWrapper<T>> readerWrapperSupplier) {
if (maxNumberOfReaders < MIN_NUMBER_OF_READERS) {
logger.warn(String.format(
"The specified maximum number of readers (%d) is less than %d. Setting it to %d.",
Expand All @@ -57,6 +57,7 @@ public ReaderPool(int maxNumberOfReaders, Callable<ReaderWrapper> readerWrapperS

@Override
public void close() {
logger.debug("Closing reader pool");
isClosed = true;

for (var cleanable : cleanables) {
Expand All @@ -83,7 +84,7 @@ public void close() {
* @throws IOException when the creation of the reader wrapper fails
* @throws InterruptedException when the wait for a reader wrapper is interrupted
*/
public ReaderWrapper getDedicatedReaderWrapper() throws IOException, InterruptedException {
public ReaderWrapper<T> getDedicatedReaderWrapper() throws IOException, InterruptedException {
if (dedicatedReaderWrapper == null) {
dedicatedReaderWrapper = getNextReaderWrapper();
}
Expand All @@ -94,23 +95,33 @@ public ReaderWrapper getDedicatedReaderWrapper() throws IOException, Interrupted
* Reads a tile of the image.
*
* @param tileRequest the parameters defining the tile
* @param series some images contain multiple image stacks or experiments within one file.
* The one to use is defined by this parameter
* @param numberOfChannels the number of channels of this image
* @param channels the channels of this image to retrieve. For now, this function only supports
* retrieving all channels and will throw an UnsupportedOperationException
* if not all channels are provided
* @param colorModel the color model to use with this image
* @param series some images contain multiple image stacks or experiments within one file.
* The one to use is defined by this parameter. This parameter is ignored
* if the reader only supports one image
* @return the image corresponding to these parameters
* @throws IllegalArgumentException when the tile dimensions are negative
* @throws IllegalStateException when no reader is available
* @throws UnsupportedOperationException when not all channels are requested
* @throws IOException when a reading error occurs
*/
public BufferedImage openImage(TileRequest tileRequest, int series, int numberOfChannels, boolean isRGB, ColorModel colorModel) throws IOException {
public T openImage(TileRequest tileRequest, int[] channels, boolean isRGB, ColorModel colorModel, int series) throws IOException {
if (tileRequest.getTileWidth() <= 0 || tileRequest.getTileHeight() <= 0) {
throw new IllegalArgumentException("Unable to request pixels for region with down sampled size " + tileRequest.getTileWidth() + " x " + tileRequest.getTileHeight());
}

for (int i=0; i<channels.length; ++i) {
if (i != channels[i]) {
throw new UnsupportedOperationException("You must request all channels");
}
}

try {
ReaderWrapper readerWrapper = getNextReaderWrapper();
BufferedImage image = readerWrapper.getImage(tileRequest, series, numberOfChannels, isRGB, colorModel);
ReaderWrapper<T> readerWrapper = getNextReaderWrapper();
T image = readerWrapper.getImage(tileRequest, channels, isRGB, colorModel, series);
availableReaderWrappers.add(readerWrapper);

return image;
Expand All @@ -129,10 +140,10 @@ public BufferedImage openImage(TileRequest tileRequest, int series, int numberOf
* @throws IllegalStateException when no reader is available
* @throws IOException when a reading error occurs
*/
public BufferedImage openSeries(int series) throws IOException {
public T openSeries(int series) throws IOException {
try {
var readerWrapper = getNextReaderWrapper();
BufferedImage image = readerWrapper.getImage(series);
T image = readerWrapper.getImage(series);
availableReaderWrappers.add(readerWrapper);

return image;
Expand All @@ -153,16 +164,23 @@ public BufferedImage openSeries(int series) throws IOException {
* @throws IOException when the creation of the reader wrapper fails
* @throws InterruptedException when the wait for a reader wrapper is interrupted
*/
private ReaderWrapper getNextReaderWrapper() throws IOException, InterruptedException {
private ReaderWrapper<T> getNextReaderWrapper() throws IOException, InterruptedException {
if (isClosed) {
throw new IllegalStateException("Reader pool is closed");
} else {
var nextReader = availableReaderWrappers.poll();
if (nextReader == null) {
logger.debug("No reader available");

var newReader = addReaderWrapper();
if (newReader.isPresent()) {
return newReader.get();
} else {
logger.debug(
"No reader available and the maximum number of readers has already been created." +
"Waiting for an existing reader to become available."
);

var reader = availableReaderWrappers.poll(READER_AVAILABILITY_WAITING_TIME, READER_AVAILABILITY_WAITING_TIME_UNIT);
if (reader == null) {
throw new IllegalStateException(
Expand All @@ -186,12 +204,12 @@ private ReaderWrapper getNextReaderWrapper() throws IOException, InterruptedExce
* @throws IllegalStateException when the reader pool is already closed
* @throws IOException when the creation of the reader wrapper fails
*/
private synchronized Optional<ReaderWrapper> addReaderWrapper() throws IOException {
private synchronized Optional<ReaderWrapper<T>> addReaderWrapper() throws IOException {
if (isClosed) {
throw new IllegalStateException("Reader pool is closed");
} else {
if (numberOfReaderWrappers.get() < maxNumberOfReaders) {
ReaderWrapper readerWrapper;
ReaderWrapper<T> readerWrapper;
try {
readerWrapper = readerWrapperSupplier.call();
} catch (Exception e) {
Expand Down
13 changes: 7 additions & 6 deletions ...-bioformats/src/main/java/qupath/lib/images/servers/bioformats/readers/ReaderWrapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,21 @@
* </p>
* <p>A reader must be {@link #close() closed} once no longer used.</p>
*/
public interface ReaderWrapper extends AutoCloseable {
public interface ReaderWrapper<T> extends AutoCloseable {

/**
* Reads a tile from the image.
*
* @param tileRequest the parameters defining the tile
* @param series some images contain multiple image stacks or experiments within one file.
* The one to use is defined by this parameter
* @param numberOfChannels the number of channels of this image
* @param channels the channels of this image to retrieve
* @param colorModel the color model to use with this image
* @param series some images contain multiple image stacks or experiments within one file.
* The one to use is defined by this parameter. This parameter is ignored
* if the reader only supports one image
* @return the image corresponding to these parameters
* @throws IOException when a reading error occurs
*/
BufferedImage getImage(TileRequest tileRequest, int series, int numberOfChannels, boolean isRGB, ColorModel colorModel) throws IOException;
T getImage(TileRequest tileRequest, int[] channels, boolean isRGB, ColorModel colorModel, int series) throws IOException;

/**
* Returns an 'associated image', e.g. a thumbnail or a slide overview images.
Expand All @@ -35,5 +36,5 @@ public interface ReaderWrapper extends AutoCloseable {
* @return the image corresponding to this parameter
* @throws IOException when a reading error occurs
*/
BufferedImage getImage(int series) throws IOException;
T getImage(int series) throws IOException;
}

0 comments on commit 2183e13

Please sign in to comment.