Skip to content

Conversation

@kring
Copy link
Member

@kring kring commented Oct 12, 2012

Adds the ability to layer imagery from multiple sources and adjust the alpha of each independently. It also has a new architecture for rendering the surface of the globe that is a big step toward rendering terrain. For starters, check out the two new Sandcastle apps: "Imagery Layers" and "Map Projections".

Most of the action happens (or at least starts) in CentralBodySurface.js. If you want to understand how it works, the (four line) update function is the place to start.

There is no shortage of additional things that could be done, but I think this is a significant improvement from master and is worthy of a review.

kring and others added 30 commits August 27, 2012 09:15
Conflicts:
	Examples/Skeleton/Skeleton.js
Conflicts:
	Source/Core/Extent.js
	Source/Scene/CentralBody.js
	Source/Scene/Tile.js
This terrain provider uses 256x256=65536 vertices.  So, if we add skirts,
we go over the vertex limit for 16-bit indices.  And WebGL doesn't
support 32-bit indices.  What to do?  Just disable the skirts for now.
This is still sketchy.  I'm currently just rescaling each heightmap from
256x256 down to 129x129 (which I hoped would drop every other pixel, but
it probably isn't actually happening that way).  A better solution is to
treat each heightmap image as 4 tiles rather than rescaling, but that's
more complicated.
Don't require the user to explicitly install a discard policy.  Instead,
just install the default one (which ought to work pretty well in most
cases) if one is not specified in the description.
Also subtract an in-flight terrain request when it fails, not just when it
succeeds.
This is preparation for eliminating the texture sampling loop via shader
code generation.
Change the texture coordinate translation and scale uniforms so that the
fragment shader only needs to do a MAD instruction.
There's no need to sort TileImagery instances before removing layers,
because the instances will always be grouped together even if they aren't
in the right order relative to other layers.
@kring
Copy link
Member Author

kring commented Oct 25, 2012

Upon further consideration, my hypothetical situation can't happen, so I reverted part of my previous change.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

@kring the roadmap looks up-to-date. Is there anything else we need to add?

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

Now is a good time to address #291.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

Why do functions that start with an underscore, like ImageryLayer.prototype._calculateTextureTranslationAndScale have public documentation?

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

Otherwise, doc looks good.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

When transitioning between Columbus View and 3D, I would not enable the sky atmosphere during the morph. Just show it when it is 3D. Longer term, I want objects like this to fade in and out.

Run Cesium Viewer, and transition between Columbus View and 3D several times - I think there is a bug with the state of showing the sky atmosphere.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

We should probably rename the "Stamen Maps" button in the Imagery Sandcastle example to "WMS."

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

We should probably rename the "Stamen Maps" button in the Imagery Sandcastle example to "WMS."

Nevermind, it uses OpenStreetMap, not WMS. No rename needed.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

I thought you added a WMS button to the Imagery Sandcastle example, and replaced the Hello image in the Imagery Layers example. I clean built, cleared cache, and forced refresh, and I don't see either change.

@kring
Copy link
Member Author

kring commented Oct 25, 2012

I thought you added a WMS button to the Imagery Sandcastle example

No, I wrote this:

I didn't add a WMS layer to the Imagery example, but 2 of the 3 layers in the Imagery Layers example use WMS. Is that sufficient?

You said yes. :)

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

WMS is a commonly required feature. Perhaps it is the most requested imagery provide. So we should make it easy to find. It is kinda buried in the imagery layers example, right? It's not the primary purpose, so I think we should have a WMS button in the Imagery example so people don't have to hunt for it - or hunt less. I am tempted to suggest combing the Imagery and Imagery Layers examples because it would have all the imagery features in one spot, but it would also make the code borderline intimidatingly long. Ugh. Just solve this problem: "WMS is important so make it easy for users to discover our support, and copy and past the relevant code."

@kring
Copy link
Member Author

kring commented Oct 25, 2012

Why do functions that start with an underscore, like ImageryLayer.prototype._calculateTextureTranslationAndScale have public documentation?

I wanted to document what they do for future maintainers of the code (including me), even though they're not meant to be called externally. Using the standard format for that seemed reasonable. I didn't intend for that doc to show up in the actual documentation, though. I thought anything with an underscore was private (you told me that), but maybe I need to mark them private explicitly. I'll do that.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

Things starting with an underscore are supposed to be private from the doc tools perspective, but they changed it, there is a bug, or if any doc is added to the function, it is no longer considered private. I suspect the later.

@kring
Copy link
Member Author

kring commented Oct 25, 2012

Hello image in the Imagery Layers example

We discussed possibly generating a raster from vector data, but I didn't do it. Honestly I don't think it would look very good. Rasterized vector data without LOD is going to be too thin when zoomed out, and too pixelated when zoomed in. The Hello thing is a decent example because it shows you can drop whatever you want on the globe, and it also illustrates that the image itself can have alpha. I guess I'm just not sure what problem we're trying to solve by getting rid of it.

@kring
Copy link
Member Author

kring commented Oct 25, 2012

I wouldn't call WMS buried in the Imagery Layers example, but I see your point that they'd be better combined as long as that doesn't make the overall example too intimidating. I'll see what I can do.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

We discussed possibly generating a raster from vector data, but I didn't do it. Honestly I don't think it would look very good. Rasterized vector data without LOD is going to be too thin when zoomed out, and too pixelated when zoomed in. The Hello thing is a decent example because it shows you can drop whatever you want on the globe, and it also illustrates that the image itself can have alpha. I guess I'm just not sure what problem we're trying to solve by getting rid of it.

It demonstrates capability, but it is too far from a real problem domain. As of now, this is the best geospatial demo we have, and people are going to take screenshots, pass it around, etc. I drew a happy face for some work I did in 2007, and if I knew it was going to get recognized, I would have used better art.

A few other ideas: heat map, coverage grid, or a Cesium logo (like we use in the materials).

kring added 2 commits October 25, 2012 14:36
Still need to remove the Imagery example.
Also switch to the Cesium logo for the SingleImageTileProvider in the Imagery
Layers example instead of the "Hello" image.  And finally, skip mipmap
generation for textures that are not a power-of-two size.
@kring
Copy link
Member Author

kring commented Oct 25, 2012

@pjcozzi the two Sandcastle apps are now combined. I don't think it's unreasonably complicated, and it will get simpler in the future when we have a layer management widget. I also swapped in the Cesium logo in place of the Hello image.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

In the Sandcastle example, the Bing logo and other credits are quite a bit to the right of the Cesium logo. Is that intentional?

@kring
Copy link
Member Author

kring commented Oct 25, 2012

The logo position was left over from when this example used the viewer widget. I just changed it to be consistent with the other non-viewer apps.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

@kring @mramato what else needs to happen before merging this?

#291 should happen soon, but this can be merged first.

@kring
Copy link
Member Author

kring commented Oct 25, 2012

It's ready to go as far as I know.

@mramato
Copy link
Contributor

mramato commented Oct 25, 2012

I ran release build and all of the tests one last time. I even checked it out on an Android tablet and everything seems to work well. I think we're ready.

pjcozzi added a commit that referenced this pull request Oct 25, 2012
@pjcozzi pjcozzi merged commit 8982277 into master Oct 25, 2012
@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2012

Merged.

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.

8 participants