Skip to content
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

Set texture sampler properties of ImageryLayer #5890

Merged
merged 28 commits into from
Oct 23, 2017
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
28d2921
Added properties ImageryLayer.minificationFilter and .magnificationFi…
forman Oct 6, 2017
4aff9c2
Renamed local var as mipmapMinificationFilter
forman Oct 9, 2017
e80ba2b
Merge branch 'master' into new_texture_sampler_prop
forman Oct 9, 2017
c1e0e54
Added some more API doc
forman Oct 9, 2017
daa095f
Made TextureMagnification/MinificationFilter public and added API-docs.
forman Oct 10, 2017
cb90b62
Update wrt public TextureMagnification/MinificationFilter enums
forman Oct 10, 2017
b5fe609
avoid line breaks
forman Oct 12, 2017
2dd4fd3
Use @exports so enum doc appears as its own section in the documentation
forman Oct 12, 2017
b9b8ab7
added @type {Number} and @constant to enum values
forman Oct 12, 2017
722cd71
use upper case Boolean
forman Oct 12, 2017
d5ccbb8
made validate() methods @private
forman Oct 12, 2017
8d27fe1
added defaultMinificationFilter and defaultMagnificationFilter to Ima…
forman Oct 12, 2017
3ec59b0
Updated api docs minification/magnificationFilter props
forman Oct 12, 2017
e5b9b61
fixed order of constants
forman Oct 12, 2017
47f66ba
fixed coding style
forman Oct 12, 2017
88d3458
added new test for default texture filters of ImageryProvider
forman Oct 12, 2017
eb5ec0c
fix: turned context.cache.imageryLayer_<x>Sampler into context.cache.…
forman Oct 12, 2017
924dc1c
reverted experimental change
forman Oct 12, 2017
3d5bfaa
create mipmap textures only for linear mini/magni filters
forman Oct 13, 2017
151f8fe
added texture filter example to Sandcastle
forman Oct 13, 2017
270c805
Merge branch 'master' into new_texture_sampler_prop
forman Oct 13, 2017
dbf8d3a
fixed a harmless typo
forman Oct 13, 2017
3d632cd
be more explicit about checking if something is undefined
forman Oct 19, 2017
2a08e5d
Merge branch 'master' into new_texture_sampler_prop
forman Oct 19, 2017
c826ec9
merged upstream
forman Oct 19, 2017
356f677
Tweak comment wording
lilleyse Oct 19, 2017
d5cf453
added assertions for imagery.texture.sampler
forman Oct 23, 2017
cdc9045
Merge branch 'master' into new_texture_sampler_prop
forman Oct 23, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
Change Log
==========

### 1.39 - 2017-11-??

* Added two new properties to `ImageryLayer` that allow for adjusting the texture sampler used for up- and down-sampling
of image tiles, namely `minificationFilter` and `magnificationFilter` with possible values `LINEAR`
(the default) and `NEAREST` defined in `TextureMinificationFilter` and `TextureMagnificationFilter`.
[#5846](https://github.com/AnalyticalGraphicsInc/cesium/issues/5846)
Hence, the enums `TextureMagnificationFilter` and `TextureMinificationFilter` have been made public.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we don't use line breaks in CHANGES.md, one long line is fine.

Hence, the enums `TextureMagnificationFilter` and `TextureMinificationFilter` have been made public.

can be its own bullet.



### 1.38 - 2017-10-02

* Breaking changes
Expand Down
20 changes: 19 additions & 1 deletion Source/Renderer/TextureMagnificationFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,30 @@ define([
'use strict';

/**
* @private
* Enumerates all possible filters used when magnifying WebGL textures, which takes places when zooming
* into imagery. Provides the possible values for the {@link ImageryLayer#magnificationFilter} property.
*
* @alias TextureMagnificationFilter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use @exports TextureMagnificationFilter instead, so it appears as its own section in the documentation. Same for TextureMinificationFilter.

*
* @see TextureMinificationFilter
* @see ImageryLayer#magnificationFilter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lilleyse this @see - and the one in TextureMinificationFilter.js - is not needed and not customary. Can you please remove in master?

*/
var TextureMagnificationFilter = {
/**
* Nearest neighbor sampling of image pixels to texture.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this file and below, try to avoid providing reference doc that is just a repeat of the enum. @lilleyse could you add a tad more description and trade offs, e.g., visual quality vs speed?

*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For each of these also add

         * @type {Number}
         * @constant

NEAREST : WebGLConstants.NEAREST,
/**
* Bi-linear interpolation of image pixels to texture.
*/
LINEAR : WebGLConstants.LINEAR,

/**
* Validates the given <code>textureMinificationFilter</code> with respect to the possible enum values.
*
* @param textureMagnificationFilter
* @returns {boolean} <code>true</code> if <code>textureMagnificationFilter</code> is valid.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically make the type uppercase Boolean.

*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate can be marked as @private.

validate : function(textureMagnificationFilter) {
return ((textureMagnificationFilter === TextureMagnificationFilter.NEAREST) ||
(textureMagnificationFilter === TextureMagnificationFilter.LINEAR));
Expand Down
32 changes: 31 additions & 1 deletion Source/Renderer/TextureMinificationFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,46 @@ define([
'use strict';

/**
* @private
* Enumerates all possible filters used when minifying WebGL textures, which takes places when zooming
* out of imagery. Provides the possible values for the {@link ImageryLayer#minificationFilter} property.
*
* @alias TextureMinificationFilter
*
* @see TextureMagnificationFilter
* @see ImageryLayer#minificationFilter
*/
var TextureMinificationFilter = {
/**
* Nearest neighbor sampling of image pixels to texture.
*/
NEAREST : WebGLConstants.NEAREST,
/**
* Bi-linear interpolation of image pixels to texture.
*/
LINEAR : WebGLConstants.LINEAR,
/**
* WebGL <code>NEAREST_MIPMAP_NEAREST</code> interpolation of image pixels to texture.
*/
NEAREST_MIPMAP_NEAREST : WebGLConstants.NEAREST_MIPMAP_NEAREST,
/**
* WebGL <code>LINEAR_MIPMAP_NEAREST</code> interpolation of image pixels to texture.
*/
LINEAR_MIPMAP_NEAREST : WebGLConstants.LINEAR_MIPMAP_NEAREST,
/**
* WebGL <code>NEAREST_MIPMAP_LINEAR</code> interpolation of image pixels to texture.
*/
NEAREST_MIPMAP_LINEAR : WebGLConstants.NEAREST_MIPMAP_LINEAR,
/**
* WebGL <code>LINEAR_MIPMAP_LINEAR</code> interpolation of image pixels to texture.
*/
LINEAR_MIPMAP_LINEAR : WebGLConstants.LINEAR_MIPMAP_LINEAR,

/**
* Validates the given <code>textureMinificationFilter</code> with respect to the possible enum values.
*
* @param textureMinificationFilter
* @returns {boolean} <code>true</code> if <code>textureMinificationFilter</code> is valid.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comments for this section as above.

validate : function(textureMinificationFilter) {
return ((textureMinificationFilter === TextureMinificationFilter.NEAREST) ||
(textureMinificationFilter === TextureMinificationFilter.LINEAR) ||
Expand Down
71 changes: 64 additions & 7 deletions Source/Scene/ImageryLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ define([
* the gamma value to use for the tile. The function is executed for every
* frame and for every tile, so it must be fast.
* @param {ImagerySplitDirection|Function} [options.splitDirection=ImagerySplitDirection.NONE] The {@link ImagerySplitDirection} split to apply to this layer.
* @param {TextureMinificationFilter} [options.minificationFilter=TextureMinificationFilter.LINEAR] The
* texture minification filter to apply to this layer. Possible values
* are <code>TextureMinificationFilter.LINEAR</code> and
* <code>TextureMinificationFilter.NEAREST</code>.
* @param {TextureMagnificationFilter} [options.magnificationFilter=TextureMagnificationFilter.LINEAR] The
* texture minification filter to apply to this layer. Possible values
* are <code>TextureMagnificationFilter.LINEAR</code> and
* <code>TextureMagnificationFilter.NEAREST</code>.
* @param {Boolean} [options.show=true] True if the layer is shown; otherwise, false.
* @param {Number} [options.maximumAnisotropy=maximum supported] The maximum anisotropy level to use
* for texture filtering. If this parameter is not specified, the maximum anisotropy supported
Expand Down Expand Up @@ -211,6 +219,26 @@ define([
*/
this.splitDirection = defaultValue(options.splitDirection, defaultValue(imageryProvider.defaultSplit, ImageryLayer.DEFAULT_SPLIT));

/**
* The {@link TextureMinificationFilter} to apply to this layer.
* Possible values are {@link TextureMinificationFilter.LINEAR} (the default)
* and {@link TextureMinificationFilter.NEAREST}.
*
* @type {TextureMinificationFilter}
* @default {@link ImageryLayer.DEFAULT_MINIFICATION_FILTER}
*/
this.minificationFilter = defaultValue(options.minificationFilter, defaultValue(imageryProvider.defaultMinificationFilter, ImageryLayer.DEFAULT_MINIFICATION_FILTER));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to add defaultMinificationFilter and defaultMagnificationFilter to ImageryProvider?

Probably fine to not have it, but up to you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if I should add them, because ImageryProvider.defaultSplit is missing too. Adding them now...


/**
* The {@link TextureMagnificationFilter} to apply to this layer.
* Possible values are {@link TextureMagnificationFilter.LINEAR} (the default)
* and {@link TextureMagnificationFilter.NEAREST}.
*
* @type {TextureMagnificationFilter}
* @default {@link ImageryLayer.DEFAULT_MAGNIFICATION_FILTER}
*/
this.magnificationFilter = defaultValue(options.magnificationFilter, defaultValue(imageryProvider.defaultMagnificationFilter, ImageryLayer.DEFAULT_MAGNIFICATION_FILTER));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be noted somewhere that these values can be set immediately after adding the imagery layer but once a texture is loaded it won't be possible to change its filter.


/**
* Determines if this layer is shown.
*
Expand Down Expand Up @@ -309,13 +337,29 @@ define([
ImageryLayer.DEFAULT_GAMMA = 1.0;

/**
* This value is used as the default spliat for the imagery layer if one is not provided during construction
* This value is used as the default split for the imagery layer if one is not provided during construction
* or by the imagery provider.
* @type {ImagerySplitDirection}
* @default ImagerySplitDirection.NONE
*/
ImageryLayer.DEFAULT_SPLIT = ImagerySplitDirection.NONE;

/**
* This value is used as the default texture magnification filter for the imagery layer if one is not provided
* during construction or by the imagery provider.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you decide to leave texture filtering out of the imagery provider, tweak this and below's doc.

* @type {TextureMagnificationFilter}
* @default TextureMagnificationFilter.LINEAR
*/
ImageryLayer.DEFAULT_MAGNIFICATION_FILTER = TextureMagnificationFilter.LINEAR;

/**
* This value is used as the default texture minification filter for the imagery layer if one is not provided
* during construction or by the imagery provider.
* @type {TextureMinificationFilter}
* @default TextureMinificationFilter.LINEAR
*/
ImageryLayer.DEFAULT_MINIFICATION_FILTER = TextureMinificationFilter.LINEAR;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with the rest of the code, DEFAULT_MINIFICATION_FILTER should be above ImageryLayer.DEFAULT_MAGNIFICATION_FILTER


/**
* Gets a value indicating whether this layer is the base layer in the
* {@link ImageryLayerCollection}. The base layer is the one that underlies all
Expand Down Expand Up @@ -764,6 +808,11 @@ define([
}
}

var sampler = new Sampler({
minificationFilter : this.minificationFilter,
magnificationFilter : this.magnificationFilter
});

// Imagery does not need to be discarded, so upload it to WebGL.
var texture;
if (defined(image.internalFormat)) {
Expand All @@ -774,13 +823,15 @@ define([
height : image.height,
source : {
arrayBufferView : image.bufferView
}
},
sampler: sampler
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Format as sampler : sampler

});
} else {
texture = new Texture({
context : context,
source : image,
pixelFormat : imageryProvider.hasAlphaChannel ? PixelFormat.RGBA : PixelFormat.RGB
pixelFormat : imageryProvider.hasAlphaChannel ? PixelFormat.RGBA : PixelFormat.RGB,
sampler: sampler
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Format as sampler : sampler

});
}

Expand All @@ -799,11 +850,17 @@ define([
var mipmapSampler = context.cache.imageryLayer_mipmapSampler;
if (!defined(mipmapSampler)) {
var maximumSupportedAnisotropy = ContextLimits.maximumTextureFilterAnisotropy;
var mipmapMinificationFilter = imageryLayer.minificationFilter;
if (imageryLayer.minificationFilter === TextureMinificationFilter.NEAREST) {
mipmapMinificationFilter = TextureMinificationFilter.NEAREST_MIPMAP_NEAREST;
} else if (imageryLayer.minificationFilter === TextureMinificationFilter.LINEAR) {
mipmapMinificationFilter = TextureMinificationFilter.LINEAR_MIPMAP_LINEAR;
}
mipmapSampler = context.cache.imageryLayer_mipmapSampler = new Sampler({
wrapS : TextureWrap.CLAMP_TO_EDGE,
wrapT : TextureWrap.CLAMP_TO_EDGE,
minificationFilter : TextureMinificationFilter.LINEAR_MIPMAP_LINEAR,
magnificationFilter : TextureMagnificationFilter.LINEAR,
minificationFilter : mipmapMinificationFilter,
magnificationFilter : imageryLayer.magnificationFilter,
maximumAnisotropy : Math.min(maximumSupportedAnisotropy, defaultValue(imageryLayer._maximumAnisotropy, maximumSupportedAnisotropy))
});
}
Expand All @@ -815,8 +872,8 @@ define([
nonMipmapSampler = context.cache.imageryLayer_nonMipmapSampler = new Sampler({
wrapS : TextureWrap.CLAMP_TO_EDGE,
wrapT : TextureWrap.CLAMP_TO_EDGE,
minificationFilter : TextureMinificationFilter.LINEAR,
magnificationFilter : TextureMagnificationFilter.LINEAR
minificationFilter : imageryLayer.minificationFilter,
magnificationFilter : imageryLayer.magnificationFilter
});
}
texture.sampler = nonMipmapSampler;
Expand Down
23 changes: 23 additions & 0 deletions Specs/Scene/ImageryLayerSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ defineSuite([
'Core/Rectangle',
'Core/RequestScheduler',
'Renderer/ComputeEngine',
'Renderer/TextureMagnificationFilter',
'Renderer/TextureMinificationFilter',
'Scene/ArcGisMapServerImageryProvider',
'Scene/BingMapsImageryProvider',
'Scene/createTileMapServiceImageryProvider',
Expand All @@ -30,6 +32,8 @@ defineSuite([
Rectangle,
RequestScheduler,
ComputeEngine,
TextureMagnificationFilter,
TextureMinificationFilter,
ArcGisMapServerImageryProvider,
BingMapsImageryProvider,
createTileMapServiceImageryProvider,
Expand Down Expand Up @@ -367,6 +371,25 @@ defineSuite([
expect(layer.isDestroyed()).toEqual(true);
});

it('texture filter properties work as expected', function() {
var provider = new SingleTileImageryProvider({
url : 'Data/Images/Red16x16.png'
});

var layer = new ImageryLayer(provider, {});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just be new ImageryLayer(provider);

expect(layer.magnificationFilter).toEqual(TextureMagnificationFilter.LINEAR);
expect(layer.minificationFilter).toEqual(TextureMinificationFilter.LINEAR);
layer.destroy();

layer = new ImageryLayer(provider, {
magnificationFilter: TextureMagnificationFilter.NEAREST,
minificationFilter: TextureMinificationFilter.NEAREST
});
expect(layer.magnificationFilter).toEqual(TextureMagnificationFilter.NEAREST);
expect(layer.minificationFilter).toEqual(TextureMinificationFilter.NEAREST);
layer.destroy();
});

it('returns HTTP status code information in TileProviderError', function() {
// Web browsers unfortunately provide very little information about what went wrong when an Image fails
// to load. But when an imagery provider is configured to use a TileDiscardPolicy, Cesium downloads the image
Expand Down