Vector layer conversion #33

gberaudo · 2014-08-22T14:46:21Z

Core converters to be used by the vector synchronizer. #4 / #11

It handles:

all shapes;
fill and stroke colours;
outline width.

Dashes are not handled in general though some experimental code exists for lines.

It is now ready for merging though some issues remain:

for each geometry class, exporting the position where the text is placed
See drawText in src/ol/render/vector.js.
only the first style of an array is used for Cesium
Blending styles to a unique plain style would be an idea, see http://en.wikipedia.org/wiki/Alpha_compositing#Alpha_blending

Updated to reflect new state.
Old demo at http://dev.camptocamp.com/files/gberaudo/vectors/examples/vectors.html

elemoine · 2014-08-25T11:45:29Z

src/core.js

+
+/**
+ * Convert a 2D or 3D OpenLayers coordinate to Cesium.
+ * @param {!ol.coordinate} coordinate Ol3 coordinate


ol.coordinate -> ol.Coordinate

We usually end @param descriptions with a dot.

"Convert a 2D or 3D OpenLayers coordinate to Cesium." -> "Convert a 2D or 3D EPSG:4326 OpenLayers coordinate to a Cesium Cartesian3."

elemoine · 2014-08-26T09:39:04Z

src/core.js

+ * @param {!ol.geom.geometry} olGeometry Ol3 circle geometry
+ * @param {!ol.projectionLike} projection
+ * @param {!ol.style} olStyle
+ * @return {Cesium.Geometry|Array.<Cesium.Geometry>} geometries


Functions are easier to use when they always return values of the same type. In that case it should probably be Array.<Cesium.Geometry>.

elemoine · 2014-09-05T09:38:54Z

src/core.js

+ * @api
+ */
+olcs.core.GL_ALIASED_LINE_WIDTH_RANGE;
+//goog.exportProperty();


You can remove this.

elemoine · 2014-09-05T09:54:29Z

This is ton of work. Thanks. I still have some comments. But it's good to go to me when they are addressed.

gberaudo · 2014-09-10T12:53:53Z

@ahocevar , examples including vectors.html can now be executed with compiled ol3cesium.js.
Just do a make dist and launch a server in dist directory.

This PR depends on the merge of two ol3 PRs: openlayers/openlayers#2703 and openlayers/openlayers#2696 . Please have a look to these PRs.
It would also benefit from a proper way of handling events and non-API base classes exports. openlayers/openlayers#2615 (comment)

It would be really great if everyone could test now and report issues so that we can discuss proper solutions and have time to address them.

As a side note, KML/geojson drag&drop on the ol3 map is enabled but not working.
It would also be great if someone knowledgeable about it could have a look.

ahocevar · 2014-09-10T16:21:30Z

Thanks for the work on this. I did a bit more testing, and I found 2 issues:

When calling olcs.OLCesium#setEnabled(true) before a vector source has finished loading, no vector features will be displayed on the Cesium globe.
Performance for many features (I tested with the countries.json file from the ol3 examples) is surprisingly bad. Shouldn't Cesium be passed the simplified geometries that OpenLayers uses for rendering, instead of the original geometries?

It would also be good if @klokan could give this a review.

gberaudo · 2014-09-11T12:18:51Z

Hi Andreas, thank you for testing.

It seems the lags comes from the text style of polygons, please comment
the 'text' in vector.js and you will see it is much faster.
You may also need to close the debugger window as it severely slows down
Cesium.

I am also able to load the countries geojson but not the 110m version.
The profiler seems to blame ol3. Could you confirm?

gberaudo · 2014-09-12T16:26:34Z

Rebased on master following the merge of #42.

This PR is blocked by the still unmerged ol3 PRs.

They are quiet simple and necessary to get working:

compiled mode (no more crash on circle reprojection);
loading of non-cached icons.

I will have little time next week; we need to move on.

petrsloup · 2014-09-16T07:06:25Z

src/core.js

+      goog.events.unlisten(image, 'load', listener, this);
+      reallyCreateBillboard();
+    };
+    goog.events.listen(image, 'load', listener, this);


The function parameters are actually: goog.events.listen(src, type, listener, opt_capt, opt_handler) -- you are passing this as a boolean determining the capture phase. It would also be better to use goog.events.listenOnce in this case instead of unlistening the event after the first occurrence.

Thank you for spotting the error.

I was not using listenOnce because I wanted to ensure the image has been
loaded correctly. However, getting a 'load' event may perhaps be enough.

petrsloup · 2014-09-16T12:18:30Z

Please make sure that make lint does not return any errors before merging this PR (many of these errors were merged as part of #42).

elemoine · 2014-09-17T14:54:50Z

What happens if you pass to Cesium an icon that is not already loaded?

gberaudo · 2014-09-17T14:58:49Z

The icon will stay invisible for ever.
It is currently what you get when you force the browser to empty is
cache with ctrl+maj+r.

gberaudo · 2014-09-18T09:57:40Z

Rebased on master.
Added a 3D polyline.

Please merge.

petrsloup · 2014-09-18T10:01:41Z

src/core.js

+        reallyCreateBillboard();
+      };
+
+      goog.events.listenOnce(image, 'load', listener, false, this);


The last two parameters are actually not needed here.

petrsloup · 2014-09-18T10:08:32Z

Minor comments, otherwise LGTM.

Fixes invisible icon in Cesium when image in not in the browser cache.

It is inside the green square-like geometry. Note that: - 3D polygons or circles are not handled by Cesium; - enabling depth test is necessary to prevent seeing a primitive through terrain.

gberaudo · 2014-09-18T12:11:57Z

Thanks @petrsloup for the good review.

Vector layer conversion

elemoine reviewed Aug 25, 2014
View reviewed changes

ahocevar modified the milestone: FEATURES Aug 25, 2014

elemoine reviewed Aug 26, 2014
View reviewed changes

elemoine reviewed Sep 5, 2014
View reviewed changes

src/core.js

* @api

*/

olcs.core.GL_ALIASED_LINE_WIDTH_RANGE;

//goog.exportProperty();

Copy link

Member

elemoine Sep 5, 2014

Choose a reason for hiding this comment

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

You can remove this.

This was referenced Sep 12, 2014

Cesium externs changes necessary for vector support. #41

Closed

Vectors without hard ol3 pr dependency #42

Merged

petrsloup reviewed Sep 16, 2014
View reviewed changes

elemoine added this to the FEATURES milestone Sep 16, 2014

petrsloup reviewed Sep 18, 2014
View reviewed changes

gberaudo added 2 commits September 18, 2014 14:00

On image loaded billboard creation.

d675a6b

Fixes invisible icon in Cesium when image in not in the browser cache.

Add a 3D polyline feature in vectors example

d0f1b3b

It is inside the green square-like geometry. Note that: - 3D polygons or circles are not handled by Cesium; - enabling depth test is necessary to prevent seeing a primitive through terrain.

gberaudo added a commit that referenced this pull request Sep 18, 2014

Merge pull request #33 from camptocamp/vectors

b7cceb7

Vector layer conversion

gberaudo merged commit b7cceb7 into openlayers:master Sep 18, 2014

gberaudo deleted the vectors branch September 24, 2014 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vector layer conversion #33

Vector layer conversion #33

gberaudo commented Aug 22, 2014

elemoine Aug 25, 2014

elemoine Aug 25, 2014

elemoine Aug 25, 2014

elemoine Aug 26, 2014

elemoine Sep 5, 2014

elemoine commented Sep 5, 2014

gberaudo commented Sep 10, 2014

ahocevar commented Sep 10, 2014

gberaudo commented Sep 11, 2014

gberaudo commented Sep 12, 2014

petrsloup Sep 16, 2014

gberaudo Sep 16, 2014

petrsloup commented Sep 16, 2014

elemoine commented Sep 17, 2014

gberaudo commented Sep 17, 2014

gberaudo commented Sep 18, 2014

petrsloup Sep 18, 2014

petrsloup commented Sep 18, 2014

gberaudo commented Sep 18, 2014

Vector layer conversion #33

Vector layer conversion #33

Conversation

gberaudo commented Aug 22, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elemoine commented Sep 5, 2014

gberaudo commented Sep 10, 2014

ahocevar commented Sep 10, 2014

gberaudo commented Sep 11, 2014

gberaudo commented Sep 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrsloup commented Sep 16, 2014

elemoine commented Sep 17, 2014

gberaudo commented Sep 17, 2014

gberaudo commented Sep 18, 2014

Choose a reason for hiding this comment

petrsloup commented Sep 18, 2014

gberaudo commented Sep 18, 2014