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

Quad feature #528

Merged
merged 12 commits into from
Feb 10, 2016
Merged

Quad feature #528

merged 12 commits into from
Feb 10, 2016

Conversation

manthey
Copy link
Contributor

@manthey manthey commented Feb 4, 2016

Adding a quad feature which allows multiple quads each with a different image.

Moved getBuffer from various gl features to geo.util.getGeomBuffer so as to avoid duplicating code.

Updates to the latest vgl as it needs to be able to set the origin of the modelViewOrigin uniform.

If multiple quads use the same texture, that texture is only created once. This doesn't get leveraged with an osm layer using a single image, as the tile layer creates a separate Image element per tile, but does get the benefit with general quads.

Preview images are supported for tiles, but only if those preview images are loaded -- if they are still loading, they won't be used.

Optionally cache information about quads to reduce computation. Cache vgl textures inside images to reduce recomputing them.

Support solid-colored quads, including previewColor.

Switched the tile layer to use the quad feature.

If the tiles haven't changed, don't requeue them. Before, on all updates, we would calculate which tiles are needed and then make sure they were queued.

Added some support for altering tile projections. This is not really sufficient, as reprojecting tiles cause the tiles to have a non-uniform density, which means our calculation of the appropriate tiles to show at different zoom levels is incorrect. If a projection cannot wrap and wrapping is turned on, we may try to ask for a functionally infinite set of tiles.

Do a little less work while checking the tile queue. This greatly reduces the cpu load when no new tiles are needed or old tiles go away.

The mockVGLRenderer functions now count how often GL functions are invoked. This is used in some of the tests.

Note that with the change for the tile layer from gl.planeFeature to gl.quadFeature, the gl.planeFeature source module is no longer covered.

Fixed a bug that could occur if a tile layer didn't include tiles for zoom level 0.

…nt image.

Move getBuffer from various gl features to geo.util.getGeomBuffer so as to avoid duplicating code.

Requires a change to vgl to support adjusting the modelViewOriginUniform.

If multiple quads use the same texture, that texture is only created once.

Preview images are supported for tiles, but only if those preview images are loaded -- if they are still loading, they won't be used.  It would require quite a bit more logic to try to load both the preview and the desired image, and properly only show the desired image if it loaded first.

Optionally cache information about quads to reduce computation.  Cache vgl textures inside images to reduce recomputing them.

Support solid-colored quads, including previewColor.

Switch the tile layer to use the quad feature.

If the tiles haven't changed, don't requeue them.

Before, on all updates, we would calculate which tiles are needed and then make sure they were queued.
This is not really sufficient, as reprojecting tiles cause the tiles to have a non-uniform density, which means our calculation of the appropriate tiles to show at different zoom levels is incorrect.  If a projection cannot wrap and wrapping is turned on, we may try to ask for a functionally infinite set of tiles.

Do a little less work while checking the tile queue.
Add some protection against too many tiles.
The mockVGLRenderer functions now count how often GL functions are invoked.

Note that with the change for the tile layer from gl.planeFeature to gl.quadFeature, the gl.planeFeature source module is no longer covered.
Using other example's thumbnails seems too ugly.  I've swapped them out for flower pictures.
Add some more core to improve the behavior with reprojection.

If the tile layer had a minimum tile level other than zero and keepLower was true, zooming out would make all tiles vanish.
@manthey
Copy link
Contributor Author

manthey commented Feb 4, 2016

This addresses issues #517 (use a single shader program), issue #228 (update planeFeature -- though, strictly, planeFeature hasn't been altered), issue #374 (non-square tiles -- additional work will be needed to truly generalize this).

This helps issue #473 (optimize tileLayer._update -- it does much less work if the tile set isn't changing) and issue #474 (reduce memory allocations).

@jbeezley
Copy link
Contributor

jbeezley commented Feb 4, 2016

I added the qlQuad screenshot. Is the new parallelProjection failure expected?

@manthey
Copy link
Contributor Author

manthey commented Feb 4, 2016

No. I was expecting everything to pass.

@manthey
Copy link
Contributor Author

manthey commented Feb 4, 2016

When I load the parallelProjection test locally, it looks fine.

@aashish24
Copy link
Member

I like the new test images 😄 lily is my favorite. Will review it but it won't be quick since it has reasonable changes

The resize event was fired when the camera viewport had been resized but its bounds had not been updated.  This meant that during a resize event the displayToGcs function would return invalid results.
@manthey
Copy link
Contributor Author

manthey commented Feb 5, 2016

Here are some timing results, using the transitions example, starting with the browser having cached the tile images, but geojs not having cached the tiles except those visible in the default view, and running through the five transitions in the order shown in the example a total of three times. In all cases I had the developer console open on the browser, which probably impacted the results some. The frame times are the amount of time spent in requestAnimationFrame.

Test Total frames Average frame time Frame time stddev Slow frames Percent slow frames Worst frame < 0.5ms < 1ms < 2ms < 4ms < 8ms <16ms <32ms <64ms <128ms <256ms <512ms <1024ms <2048ms
desktop-chrome-plane 1712 12.69 9.98 421 24.59 84.9 0 0 0 3 733 594 289 85 8 0 0 0 0
desktop-chrome-quad 1823 3.31 2.52 8 0.44 20.74 3 58 269 1140 234 113 6 0 0 0 0 0 0
desktop-ie-quad 825 4.78 2.74 7 0.85 22.77 0 0 16 465 240 97 7 0 0 0 0 0 0
destop-firefox-plane 571 55.25 120.35 317 55.52 1316.73 0 0 0 1 45 237 155 54 12 33 25 8 1
destop-firefox-quad 1804 7.5 3.81 69 3.82 41.04 0 2 7 237 841 658 58 1 0 0 0 0 0
laptop-chrome-plane 1068 25.55 25.58 643 60.21 157.15 0 0 2 4 123 350 380 119 79 11 0 0 0
laptop-chrome-quad 1631 3.89 5.65 54 3.31 62.62 6 226 488 460 311 88 35 17 0 0 0 0 0
laptop-firefox-plane 233 130.04 211.72 223 95.71 1126.31 0 0 0 1 2 9 83 57 29 17 18 15 2
laptop-firefox-quad 1067 21.92 7.02 972 91.1 67.21 3 7 1 0 1 115 892 46 2 0 0 0 0

The tests were run on my desktop and my laptop. The desktop has an AMD FirePro W5000.

Short summary: the quads are vastly faster than the planes. Except for Firefox on my laptop, the speed is very good, with very few dropped frames.

IE reported very few slow frames, but only animated half as many frames as expected. This means that it spent a lot of time outside of requestAnimationFrame and failed to serve frames as often as expected (it may also have limited the frame rate to 30fps rather than 60fps used in Chrome and Firefox).

…h low resolution z-buffers.

Specifically, on Chrome on Android (current version), using too small a z-value results in the layers flickering.
@aashish24
Copy link
Member

Short summary: the quads are vastly faster than the planes. Except for Firefox on my laptop, the speed is very good, with very few dropped frames.

This is awesome @manthey How did you capture these times?

reset_minimum_zoom();
var newZoom = fix_zoom(m_zoom);
if (newZoom !== m_zoom) {
m_this.zoom(newZoom);
}
m_this.camera().viewport = {width: w, height: h};
Copy link
Member

Choose a reason for hiding this comment

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

@manthey why was this necessary?

@aashish24
Copy link
Member

@manthey looks great! I only have few questions. I didn't review the test code line by line but did look into the API and GL implementation and it looks great !

@aashish24
Copy link
Member

@manthey I locally merged your branch into master did npm install and grunt and got other examples running but the quad one is failing:

http://localhost:8082/data/grid.jpg Failed to load resource: the server responded with a status of 404 (Not Found)

@aashish24
Copy link
Member

Also, in wms example, I don't see wms layer unless I zoom in or pan. Not sure if this branch caused this may be this was the case before. (http://localhost:8082/examples/wms/)

@manthey
Copy link
Contributor Author

manthey commented Feb 8, 2016

I captured the times by instrumenting the requestAnimationFrame function. When we run transitions, we typically have two requestAnimationFrame calls per frame, one for rendering and one for computing where the next frame will be located. The times are the sum of those two calls (strictly, requestAnimationFrame is passed the starting time of the frame callback, so the time is actually the delta between window.performance.now() and the callback time value.

@manthey
Copy link
Contributor Author

manthey commented Feb 8, 2016

Regarding http://localhost:8082/data/grid.jpg -- I think this file is only pulled down from Girder if you also build the selenium tests.

@manthey
Copy link
Contributor Author

manthey commented Feb 8, 2016

I see the issue with the WMS example, too. I'll look into it.

@manthey
Copy link
Contributor Author

manthey commented Feb 8, 2016

@aashish24 The issue with the WMS example is that we change the url after the wms tile layer is created, and I was failing to clear a cache. It is good that it showed up this error, but from an efficiency standpoint, the WMS example could just start with the correct url, so it doesn't bother to load a handful of different tiles before loading the tiles we want.

@jbeezley
Copy link
Contributor

I don't know if @aashish24 wants to chime in again, but this LGTM.

@aashish24
Copy link
Member

LGTM Ship it!

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.

3 participants