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

Billboard.prototype.computeScreenSpacePosition returning wrong y position #3920

Closed
e-andersson opened this issue May 16, 2016 · 8 comments · Fixed by #3976
Closed

Billboard.prototype.computeScreenSpacePosition returning wrong y position #3920

e-andersson opened this issue May 16, 2016 · 8 comments · Fixed by #3976

Comments

@e-andersson
Copy link
Contributor

The behavior of computeScreenSpacePosition seems to have changed recently (there seem to be a few new commits changing Billboard.js). It looks like the y screen coordinate is the inverse of what it used to be. According to the API docs, the y coordinate increases from top to bottom.

The attached Billboards sandcastle example prints the screen positions and the computed screen positions of the clicked billboard.

Am I possibly missing something here?
sandcastle_compute_screenposition.txt

@pjcozzi
Copy link
Contributor

pjcozzi commented May 18, 2016

@emackey
Copy link
Contributor

emackey commented May 18, 2016

Git bisect says this went bad in 651a8b2.

@mramato
Copy link
Contributor

mramato commented May 18, 2016

Given this has been brought up several times this month, we should probably try to get to this before the next release.

@emackey
Copy link
Contributor

emackey commented May 18, 2016

Here's some test code. A small div should float in the middle of the Cesium logo, and not go to the top of the screen when the logo is at the bottom of the screen and vice-versa.

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

var billboards = scene.primitives.add(new Cesium.BillboardCollection());
var b = billboards.add({
    image : '../images/Cesium_Logo_overlay.png',
    position : Cesium.Cartesian3.fromDegrees(-75.59777, 40.03883)
});

var floater = document.createElement('div');
floater.innerHTML = 'Test';
floater.style.cssText = 'position:absolute;border: 1px solid #aaa;background:black';
document.body.appendChild(floater);

var scratchPos = new Cesium.Cartesian2();
viewer.clock.onTick.addEventListener(function(clock) {
    var pos = b.computeScreenSpacePosition(scene, scratchPos);
    if (pos) {
        floater.style.display = 'block';
        floater.style.top = pos.y + 'px';
        floater.style.left = pos.x + 'px';
    } else {
        floater.style.display = 'none';
    }
});

@mramato
Copy link
Contributor

mramato commented May 18, 2016

So is this just a one-character change to negate y?

@emackey
Copy link
Contributor

emackey commented May 18, 2016

It's basically height - y, yes. We should probably check that the solution works when the canvas has a different number of CSS pixels from canvas pixels, unless we're explicitly giving the answer in canvas pixels with CSS-oriented coordinates. Also, several other classes besides Billboard offer a computeScreenSpacePosition, they should be tested too.

@hpinkos
Copy link
Contributor

hpinkos commented May 23, 2016

Possibly related: https://groups.google.com/forum/?hl=en#!topic/cesium-dev/2LkqgI7LemA

He says Label.computeScreenSpacePosition works in 3D but not 2D

@pjcozzi
Copy link
Contributor

pjcozzi commented May 26, 2016

@bagnell can you look at this in time for the release next week?

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.

5 participants