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

Texture rotation incorrect #2737

Closed
maikuru opened this issue May 20, 2015 · 30 comments · Fixed by #4725
Closed

Texture rotation incorrect #2737

maikuru opened this issue May 20, 2015 · 30 comments · Fixed by #4725

Comments

@maikuru
Copy link

maikuru commented May 20, 2015

When rotating a rectangle the texture is zooming in and out, even when using the default cesium materials.

I've attached some sample html to quickly reproduce.

<div id="cesiumContainer"></div>
<div id="buttons">
    <button onclick="this.disabled=true; addBillboard();">Add Billboard</button>
</div>

<script>
    var viewer          = new Cesium.Viewer('cesiumContainer');
    var entity          = new Cesium.Entity();
    var image           = new Image(300, 300);
    var canvas          = document.createElement('canvas');

    canvas.width    = 300;
    canvas.height   = 300;

    var context         = canvas.getContext('2d');

    var rot             = 0.0;
    var doRotation      = false;

    function addBillboard() {
        image.src = 'red.png';

        //Need to wait for image to load before proceeding to draw
        image.onload = function () {
            canvas.getContext('2d').drawImage(image, 0, 0);

            entity.rectangle = new Cesium.RectangleGraphics(
                {
                    coordinates: Cesium.Rectangle.fromDegrees(-86.69777, 30.03883, -76.69777, 40.03883),
                    material: new Cesium.ImageMaterialProperty( {image: canvas} ),
//                    material: new Cesium.StripeMaterialProperty({repeat: 4}),
                    rotation: new Cesium.CallbackProperty(getRotationValue, false),
                    stRotation: new Cesium.CallbackProperty(getRotationValue, false)
                }
            );

            viewer.entities.add(entity);
            viewer.flyTo(viewer.entities)
                .then(function() {
                    setInterval(rotateRect, 50);
                });
        };
    }

    function getRotationValue() {
        return rot;
    }

    function rotateRect() {
        rot = (rot + 0.015) % Cesium.Math.TWO_PI;
    }

</script>
@pjcozzi
Copy link
Contributor

pjcozzi commented May 20, 2015

Was this posted on the forum? If so can you include a link to the post so we can follow up later?

@devguy22
Copy link

It was my thread on the forum: https://groups.google.com/forum/#!topic/cesium-dev/UF9wQugwRf0

@pjcozzi
Copy link
Contributor

pjcozzi commented May 21, 2015

Thanks @devguy22!

@devguy22
Copy link

Any updates on this?

@pjcozzi
Copy link
Contributor

pjcozzi commented Jun 18, 2015

No update. We've been focused on Cesium 1.11.

@zaichang
Copy link

zaichang commented Dec 4, 2015

I have also run into this issue when creating a rectangular entity and then rotating it along with it's texture. Here's an interactive example of the issue:
http://www.zaiadesign.com/play/entity-rotation-test.html

@dwhipps
Copy link
Contributor

dwhipps commented Sep 8, 2016

I'm also running into this. In the special case where the rotation is multiples of 90 degrees, it looks fine, but anything else and the texture is wrong.

See my SANDCASTLE here.

@dwhipps
Copy link
Contributor

dwhipps commented Sep 9, 2016

@pjcozzi One thing I noticed while trying to debug this, is the comment on line 787 of RectangleGeometry.js. It says:

// negate angle for a counter-clockwise rotation

... but stRotation is defined as ccw, and the function it's used in immediately below:

Matrix2.fromRotation(-stRotation, textureMatrix)

says that the "angle" parameter is:

@param {Number} angle The angle, in radians, of the rotation. Positive angles are counterclockwise.

That strikes me as an unnecessary negation. When I tried removing the minus sign it didn't completely solve this issue, but I think it might be one of perhaps several small issues?

@pjcozzi
Copy link
Contributor

pjcozzi commented Sep 10, 2016

@dwhipps we would definitely welcome pull requests with incremental fixes even if it doesn't fix this issue completely. Thanks for looking into this!

@hpinkos
Copy link
Contributor

hpinkos commented Oct 11, 2016

@lucasvw
Copy link

lucasvw commented Oct 12, 2016

See also:
#3172 and #3173.

@bagnell
Copy link
Contributor

bagnell commented Oct 18, 2016

Updated code example that can be pasted in the Sandcastle:

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

var width = 1189;
var height = 300;

var entity          = new Cesium.Entity();
var image           = new Image(width, height);
var canvas          = document.createElement('canvas');

canvas.width    = width;
canvas.height   = height;

var context         = canvas.getContext('2d');

var rot             = 0.0;
var doRotation      = false;

function addBillboard() {
    image.src = '../images/Cesium_Logo_Color.jpg';

    //Need to wait for image to load before proceeding to draw
    image.onload = function () {
        canvas.getContext('2d').drawImage(image, 0, 0);

        entity.rectangle = new Cesium.RectangleGraphics(
            {
                coordinates: Cesium.Rectangle.fromDegrees(-86.69777, 30.03883, -76.69777, 40.03883),
                material: new Cesium.ImageMaterialProperty( {image: canvas} ),
//                    material: new Cesium.StripeMaterialProperty({repeat: 4}),
                rotation: new Cesium.CallbackProperty(getRotationValue, false),
                stRotation: new Cesium.CallbackProperty(getRotationValue, false)
            }
        );

        viewer.entities.add(entity);
        viewer.flyTo(viewer.entities)
            .then(function() {
                setInterval(rotateRect, 50);
            });
    };
}

function getRotationValue() {
    return rot;
}

function rotateRect() {
    rot = (rot + 0.015) % Cesium.Math.TWO_PI;
}

addBillboard();

@bagnell
Copy link
Contributor

bagnell commented Oct 18, 2016

This was fixed in #4430.

@bagnell bagnell closed this as completed Oct 18, 2016
@hpinkos
Copy link
Contributor

hpinkos commented Oct 18, 2016

Great work @jorpiell, thanks for looking into this! This was a frequently reported issue, lots of people will be happy to know it's been fixed.

@dwhipps
Copy link
Contributor

dwhipps commented Oct 19, 2016

I'm not sure this is fixed. In the above example, why is the texture "changing size" inside the rectangle?

@hpinkos
Copy link
Contributor

hpinkos commented Oct 19, 2016

@dwhipps did you pull down master? This is what I see after the change was merged:

rect-spin

The texture was changing size before this fix was made.

@dwhipps
Copy link
Contributor

dwhipps commented Oct 19, 2016

Sorry, I misunderstood. I thought the example could be pasted into the current (web) Sandbox. I didn’t realize that it wasn’t up to date with master.

Looks good now.

Thanks,

Dave

On Oct 19, 2016, at 8:38 AM, Hannah notifications@github.com wrote:

@dwhipps https://github.com/dwhipps did you pull down master? This is what I see after the change was merged:

https://cloud.githubusercontent.com/assets/3451886/19518857/44fb4ade-95d7-11e6-9cec-7130859864bb.gif
The texture was changing size before this fix was made.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #2737 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/ACN_wNqkGZffihchapHtdR_nKKsD_G8-ks5q1g8wgaJpZM4EiVjZ.

@hpinkos
Copy link
Contributor

hpinkos commented Oct 19, 2016

No problem =) Yeah, Sandcastle on the website is always using the latest Cesium release

@dwhipps
Copy link
Contributor

dwhipps commented Oct 19, 2016

@hpinkos Could you try the above example with these coordinates, and tell me if it looks right?

coordinates: Cesium.Rectangle.fromDegrees(-66.69777, 50.03883, -56.69777, 52.03883),

Not sure if I'm going crazy or what, but it still seems broken to me.

@hpinkos hpinkos reopened this Oct 19, 2016
@hpinkos
Copy link
Contributor

hpinkos commented Oct 19, 2016

Thanks @dwhipps, you're right. I think we'll have to take a closer look at how we're computing texture coordinates.

rect-spin2

@jorpiell
Copy link

jorpiell commented Oct 21, 2016

It seems that the texture is correctly rotated when the angle is multiple of Math.PI/4 and it fails in the
other cases

@dwhipps
Copy link
Contributor

dwhipps commented Oct 24, 2016

@jorpiell Agreed. But where do we look next? Is someone on this issue? It's a big one for me.

@dwhipps
Copy link
Contributor

dwhipps commented Oct 24, 2016

A bit more on this...

You can easily see the effect by changing just the value of the "north" boundary in the rectangle in the @bagnell example above. (That is, my counter-example used a different centre. That has no effect, only the aspect ratio seem to matter.) Simply lower it to "32" and you'll see the bug. I.e.

coordinates: Cesium.Rectangle.fromDegrees(-86.69777, 30.03883, -76.69777, 32.03883),

@hpinkos
Copy link
Contributor

hpinkos commented Oct 24, 2016

@dwhipps this is a little overly complicated because we allow the texture and the geometry to rotate separately. I think creating a specific trapezoid geometry (like the KML LatLonQuad) is probably what users are looking for. We have an issue written up to create this here: #4164
I was planning to try to take a stab at that during our next code sprint (which will probably be early December) but let me know if you think that's something you'd like to take a look at.

@dwhipps
Copy link
Contributor

dwhipps commented Oct 24, 2016

December is way to far away for me. (I thought this one was part of this "bug bash?") I'm having a look right now, but it's pretty dense stuff, and almost no comments in code.

@dwhipps
Copy link
Contributor

dwhipps commented Oct 24, 2016

Ok. After all that complaining, I think I got it. See my PR.

@jorpiell
Copy link

@dwhipps, you fixed the bug. I've tested it just removing the "* options.lonScalar" and the "* options.latScalar" in RectangleGeometryLibrary.js . Well done!

@dwhipps
Copy link
Contributor

dwhipps commented Oct 25, 2016

@hpinkos This should be good to go now.

@hpinkos
Copy link
Contributor

hpinkos commented Oct 25, 2016

Simplified example

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

var rectangle = Cesium.Rectangle.fromDegrees(-86.69777, 30.03883, -76.69777, 40.03883);
var stRotation = Cesium.Math.toRadians(0.0);
var rotation = Cesium.Math.toRadians(30);
viewer.entities.add({
    rectangle: {
        coordinates: rectangle,
        material: '../images/Cesium_Logo_Color.jpg',
        rotation: rotation,
        stRotation: stRotation
    }
});

@dwhipps
Copy link
Contributor

dwhipps commented Oct 26, 2016

So, it looks like this bug isn't yet fixed. @jorpiell, can you have a look at this PR? (#4515) I'm at a loss. The code now only works when the rectangle rotation and the texture rotation angles are the same.

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

Successfully merging a pull request may close this issue.

9 participants