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

Performance: hide ol layers when 2D not visible #66

Merged
merged 2 commits into from
Sep 22, 2014
Merged

Performance: hide ol layers when 2D not visible #66

merged 2 commits into from
Sep 22, 2014

Conversation

petrsloup
Copy link
Member

Related to #19 (comment).

This optimization hides the root LayerGroup (ol.Map#getLayerGroup) via setVisible(false) to prevent the underlying layers from being rendered.
The Chrome Dev Tools report that the tiles are not even being requested, so the performance gain should be noticeable (please report).

If the developer changes the root LayerGroup (via setLayerGroup()) everything works OK, but the original LayerGroup will still have visible set to false -- until the ol3cesium is re-enabled.
If the developer tries to hide the root LayerGroup when already hidden by ol3cesium, this setting will be overwritten when re-enabling ol3cesium.

These rare issues could be solved by adding some event listeners, but I don't think it's worth the extra code that would be required. Opinions?

@gberaudo
Copy link
Member

I think there will be an impact on vector synchronization if the visibility of the group is modified.
To be analysed.

@petrsloup
Copy link
Member Author

Oh, you are probably right -- It might affect visibility of the "streamed" vector content, but I think static data should work ok...

In the case, we could probably add some @define or even olcs.OLCesium constructor option to optionally enable this optimization.

@petrsloup
Copy link
Member Author

@gberaudo I tested the existing vectors example (modified to run in the "stacked view") and it does not seem to be affected. Is there any reason why it should be?

@gberaudo
Copy link
Member

@petrsloup, the vector synchronizer is listening for layer group
visibility change and showing/hidding the Cesium primitives accordingly.

@petrsloup
Copy link
Member Author

As far as I can tell, the VectorSynchronizer does not interact with the root LayerGroup of the ol.Map in any way -- it directly accesses its children via ol.Map#getLayers. So modifying visibility of the root group should have no impact on the cesium layer visibility.

@gberaudo
Copy link
Member

@petrsloup, thank you for spotting this bug.
It must be fixed so we can not rely on this for the performance trick.

@klokan
Copy link
Member

klokan commented Sep 22, 2014

@gberaudo I am not sure this is a bug which should be fixed right now.

The fact that the root LayerGroup hiding is not synced is used now for #19 - for performance optimization - as OL3 does not have any other way for stopping rendering in the stacked view with globe above.

@klokan
Copy link
Member

klokan commented Sep 22, 2014

@ahocevar please comment - whether there is a better way how to stop rendering in ol3 properly or if this PR should be merged now as a temporary solution for improving the performance.

@ahocevar
Copy link
Member

The only way I could think of for handling this in ol3 would be to give the map a setRenderer method, and create a dummy renderer that does nothing but dispatch render events. The former would be implemented in ol3, the latter could be done on the app (i.e. ol3-cesium) level if we allow applications to subclass ol.renderer.Map and implement renderFrame. Same for ol.renderer.Layer and prepareFrame.

@ahocevar
Copy link
Member

If the above sounds like too much effort for now, I think we can merge this pull request as a temporary solution.

klokan added a commit that referenced this pull request Sep 22, 2014
Performance: hide ol layers when 2D not visible
@klokan klokan merged commit 101e113 into openlayers:master Sep 22, 2014
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.

None yet

4 participants