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

Exposed minificationFilter and magnificationFilter for Material #8473

Merged
merged 9 commits into from
Jan 7, 2020

Conversation

santilland
Copy link
Contributor

Ok, so here goes first try at making a pull request, quite some text to read to get to know the workflow.
I signed the CLA.
This pull request is related to discussion here:
https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/cesium-dev/CacSIMIhqeo/jdpgL-v9AQAJ
and to ticket #8328
I basically tried to mimic/copy the approach taken for the imagerylayer.
A sandcastle example:

var viewer = new Cesium.Viewer('cesiumContainer');

var checker2x2 = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAIAAAACCAIAAAD91JpzAAAACXBIWXMAAC4jAAAuIwF4pT92AAAAB3RJTUUH4wwSDS0atnSBXwAAABl0RVh0Q29tbWVudABDcmVhdGVkIHdpdGggR0lNUFeBDhcAAAAVSURBVAjXBcEBAQAAAIAQ/08XUKkwKeQF+2OdNgAAAAAASUVORK5CYII=';

var rectanglelinear = viewer.scene.primitives.add(new Cesium.Primitive({
    geometryInstances : new Cesium.GeometryInstance({
        geometry : new Cesium.RectangleGeometry({
            rectangle : Cesium.Rectangle.fromDegrees(-140.0, 10.0, -80.0, 40.0),
            vertexFormat : Cesium.EllipsoidSurfaceAppearance.VERTEX_FORMAT
        })
    }),
    appearance : new Cesium.EllipsoidSurfaceAppearance({
        aboveGround : false
    })
}));

rectanglelinear.appearance.material = new Cesium.Material({
    fabric : {
        type : 'DiffuseMap',
        uniforms : {
            image : checker2x2
        }
    }
});

var rectanglenearest = viewer.scene.primitives.add(new Cesium.Primitive({
    geometryInstances : new Cesium.GeometryInstance({
        geometry : new Cesium.RectangleGeometry({
            rectangle : Cesium.Rectangle.fromDegrees(-80.0, 10.0, -20.0, 40.0),
            vertexFormat : Cesium.EllipsoidSurfaceAppearance.VERTEX_FORMAT
        })
    }),
    appearance : new Cesium.EllipsoidSurfaceAppearance({
        aboveGround : false
    })
}));

rectanglenearest.appearance.material = new Cesium.Material({
    fabric : {
        type : 'DiffuseMap',
        uniforms : {
            image : checker2x2
        }
    },
    minificationFilter: Cesium.TextureMinificationFilter.NEAREST,
    magnificationFilter: Cesium.TextureMagnificationFilter.NEAREST
});

Should look like this:
minmagexample

On the left the current default linear approach on the right using nearest neighbor, really useful for low resolution images/materials.

Hope i did not forget anything, let me know if you need something more,
kind regards,
Daniel

@cesium-concierge
Copy link

Thanks for the pull request @santilland!

  • ❌ Missing CLA.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

@OmarShehata
Copy link
Contributor

Thanks @santilland! I can confirm we just received your signed corporate CLA.

@IanLilleyT IanLilleyT requested a review from lilleyse January 7, 2020 00:57
@IanLilleyT
Copy link
Contributor

Thanks for the PR @santilland! I made some changes and tests, and this should be ready after another quick review.

@lilleyse
Copy link
Contributor

lilleyse commented Jan 7, 2020

I pushed a fix for video materials.

sampling

Local sandcastle - you need to disable CORS for the video to work locally (I use the allow-cors-access-control Chrome extension but only for dev purposes like this, use at your own risk).

* @type {TextureMinificationFilter}
* @default {@link TextureMinificationFilter.LINEAR}
*/
this.minificationFilter = defaultValue(options.minificationFilter, 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.

These options should be documented in the Material constructor.

If these options only take effect before textures are created maybe they should only be documented in the constructor and not here.

@lilleyse
Copy link
Contributor

lilleyse commented Jan 7, 2020

Thanks @santilland and @IanLilleyT!

@lilleyse lilleyse merged commit 684e9b7 into CesiumGS:master Jan 7, 2020
@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/cesium-dev/CacSIMIhqeo/jdpgL-v9AQAJ

If this issue affects any of these threads, please post a comment like the following:

The issue at #8473 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

@santilland
Copy link
Contributor Author

Great! Thanks for the merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants