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

Make maps better at exiting and reloading. #750

Merged
merged 3 commits into from
Dec 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/canvas/heatmapFeature.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,9 @@ var canvas_heatmapFeature = function (arg) {
* @protected
*/
this._setTransform = function () {
m_this.layer().canvas()[0].style.transform = m_heatMapTransform;
if (m_this.layer() && m_this.layer().canvas() && m_this.layer().canvas()[0]) {
m_this.layer().canvas()[0].style.transform = m_heatMapTransform;
}
};

/**
Expand Down
12 changes: 7 additions & 5 deletions src/gl/quadFeature.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ var gl_quadFeature = function (arg) {
* Build this feature.
*/
this._build = function () {
var mapper, mat, prog, srctex, unicrop, geom;
var mapper, mat, prog, srctex, unicrop, geom, context;

if (!m_this.position()) {
return;
Expand All @@ -174,10 +174,11 @@ var gl_quadFeature = function (arg) {
unicrop = new vgl.uniform(vgl.GL.FLOAT_VEC2, 'crop');
unicrop.set([1.0, 1.0]);
prog.addUniform(unicrop);
context = m_this.renderer()._glContext();
prog.addShader(vgl.getCachedShader(
vgl.GL.VERTEX_SHADER, vgl.GL, vertexShaderImageSource));
vgl.GL.VERTEX_SHADER, context, vertexShaderImageSource));
prog.addShader(vgl.getCachedShader(
vgl.GL.FRAGMENT_SHADER, vgl.GL, fragmentShaderImageSource));
vgl.GL.FRAGMENT_SHADER, context, fragmentShaderImageSource));
Copy link
Member

Choose a reason for hiding this comment

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

+1 how did we miss this 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Historically, this was really only using the context to access constants, so we thought the general class was sufficient.

mat.addAttribute(prog);
mat.addAttribute(new vgl.blend());
/* This is similar to vgl.planeSource */
Expand Down Expand Up @@ -214,9 +215,10 @@ var gl_quadFeature = function (arg) {
prog.addUniform(new vgl.projectionUniform('projectionMatrix'));
prog.addUniform(new vgl.floatUniform('opacity', 1.0));
prog.addUniform(new vgl.uniform(vgl.GL.FLOAT_VEC3, 'vertexColor'));
context = m_this.renderer()._glContext();
prog.addShader(vgl.getCachedShader(
vgl.GL.VERTEX_SHADER, vgl.GL, vertexShaderColorSource));
prog.addShader(vgl.utils.createFragmentShader(vgl.GL));
vgl.GL.VERTEX_SHADER, context, vertexShaderColorSource));
prog.addShader(vgl.utils.createFragmentShader(context));
mat.addAttribute(prog);
mat.addAttribute(new vgl.blend());
/* This is similar to vgl.planeSource */
Expand Down
37 changes: 32 additions & 5 deletions src/gl/vglRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,19 +169,46 @@ var vglRenderer = function (arg) {
* This clears the render timer and actually renders.
*/
this._renderFrame = function () {
if (m_updateCamera) {
m_updateCamera = false;
m_this._updateRendererCamera();
if (m_viewer) {
if (m_updateCamera) {
m_updateCamera = false;
m_this._updateRendererCamera();
}
m_viewer.render();
}
m_viewer.render();
};

/**
* Get the GL context for this renderer.
*
* @returns {WebGLRenderingContext} The current context. If unavailable,
* falls back to the vgl generic context.
*/
this._glContext = function () {
if (m_viewer && m_viewer.renderWindow()) {
return m_viewer.renderWindow().context();
}
return vgl.GL;
};

/**
* Exit.
*/
this._exit = function () {
m_this.layer().map().scheduleAnimationFrame(this._renderFrame, 'remove');
m_this.canvas().remove();
m_viewer.exit();
if (m_viewer) {
var renderState = new vgl.renderState();
renderState.m_renderer = m_viewer;
renderState.m_context = this._glContext();
m_viewer.exit(renderState);
if (this._glContext() !== vgl.GL && this._glContext().getExtension('WEBGL_lose_context') && this._glContext().getExtension('WEBGL_lose_context').loseContext) {
this._glContext().getExtension('WEBGL_lose_context').loseContext();
}
}
// make sure we clear shaders associated with the generate context, too
vgl.clearCachedShaders(vgl.GL);
m_viewer = null;
s_exit();
};

Expand Down
4 changes: 3 additions & 1 deletion src/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -1482,7 +1482,9 @@ var map = function (arg) {
if (m_discreteZoom) {
m_this.zoom(Math.round(m_this.zoom()));
}
m_this.interactor().options({discreteZoom: m_discreteZoom});
if (m_this.interactor()) {
Copy link
Member

Choose a reason for hiding this comment

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

can we have a situation where we do not have interactor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if you want to display a static map. We have a test without an interactor.

Copy link
Member

Choose a reason for hiding this comment

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

thanks!

m_this.interactor().options({discreteZoom: m_discreteZoom});
}
}
return m_this;
};
Expand Down
2 changes: 1 addition & 1 deletion src/sceneObject.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ var sceneObject = function (arg) {
* Free all resources and destroy the object.
*/
this._exit = function () {
m_this.children = [];
m_children = [];
Copy link
Member

Choose a reason for hiding this comment

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

was this a bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

delete m_this.parent;
s_exit();
};
Expand Down
6 changes: 2 additions & 4 deletions src/ui/sliderWidget.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,7 @@ var sliderWidget = function (arg) {
.on('mouseout', mouseOut);

// Update the nub position on zoom
m_this.layer().geoOn(geo_event.zoom, function () {
m_this._update();
});
m_this.geoOn(geo_event.zoom, m_this._update);

mouseOut();
m_this._update();
Expand All @@ -302,8 +300,8 @@ var sliderWidget = function (arg) {
* @private
*/
this._exit = function () {
m_this.geoOff(geo_event.zoom, m_this._update);
m_group.remove();
m_this.layer().geoOff(geo_event.zoom);
s_exit();
};

Expand Down
121 changes: 81 additions & 40 deletions src/ui/widget.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@ var sceneObject = require('../sceneObject');
*/

/**
* Create a new instance of class widget
* Create a new instance of class widget.
*
* @class geo.gui.widget
* @class
* @alias geo.gui.widget
* @param {object} [arg] Options for the widget.
* @param {geo.layer} [arg.layer] Layer associated with the widget.
* @param {geo.gui.widget.position} [arg.position] Location of the widget.
* @param {geo.gui.widget} [arg.parent] Optional parent widget.
* @extends {geo.sceneObject}
* @returns {geo.gui.widget}
*/
Expand All @@ -35,18 +40,27 @@ var widget = function (arg) {
var m_this = this,
s_exit = this._exit,
m_layer = arg.layer,
m_canvas = null;

arg.position = arg.position === undefined ? { left: 0, top: 0 } : arg.position;
m_canvas = null,
m_position = arg.position === undefined ? { left: 0, top: 0 } : arg.position;

if (arg.parent !== undefined && !(arg.parent instanceof widget)) {
throw new Error('Parent must be of type geo.gui.widget');
}

/**
* Initialize the widget.
*
* @returns {this}
*/
this._init = function () {
m_this.modified();
return m_this;
};

/**
* Clean up the widget.
*
*/
this._exit = function () {
m_this.children().forEach(function (child) {
m_this._deleteFeature(child);
Expand All @@ -58,9 +72,11 @@ var widget = function (arg) {
};

/**
* Create feature give a name
* Create a new feature.
*
* @returns {geo.Feature} Will return a new feature
* @param {string} featureName Name of the feature to create.
* @param {object} arg Options for the new feature.
* @returns {geo.feature} The new feature.
*/
this._createFeature = function (featureName, arg) {

Expand All @@ -73,7 +89,10 @@ var widget = function (arg) {
};

/**
* Delete feature
* Delete feature.
*
* @param {geo.feature} feature The feature to delete.
* @returns {this}
*/
this._deleteFeature = function (feature) {
m_this.removeChild(feature);
Expand All @@ -83,6 +102,8 @@ var widget = function (arg) {

/**
* Return the layer associated with this widget.
*
* @returns {geo.layer}
*/
this.layer = function () {
return m_layer;
Expand All @@ -96,54 +117,67 @@ var widget = function (arg) {
};

/**
* Get/Set the canvas for the widget
* Get/Set the canvas for the widget.
*
* @param {HTMLElement} [val] If specified, set the canvas, otherwise get
* the canvas.
* @returns {HTMLElement|this} If getting the canvas, return the current
* value; otherwise, return this widget.
*/
this.canvas = function (val) {
if (val === undefined) {
return m_canvas;
} else {
m_canvas = val;
}
m_canvas = val;
return m_this;
};

/**
* Appends a child to the widget
* The widget determines how to append itself to a parent, the parent can either
* be another widget, or the UI Layer.
* Appends a child to the widget.
* The widget determines how to append itself to a parent, the parent can
* either be another widget, or the UI Layer.
*/
this._appendChild = function () {
m_this.parentCanvas().appendChild(m_this.canvas());
};

/**
* Get the parent canvas (top level widgets define their layer as their parent canvas)
* Get the parent canvas (top level widgets define their layer as their
* parent canvas).
*
* @returns {HTMLElement} The canvas of the widget's parent.
*/
this.parentCanvas = function () {
if (m_this.parent === undefined) {
return m_this.layer().canvas();
} else {
return m_this.parent().canvas();
}
return m_this.parent().canvas();
};

/**
* Gets the CSS positioning that a widget should be placed at.
* { top: 0, left: 0 } by default.
* Get or set the CSS positioning that a widget should be placed at.
*
* @param {geo.gui.widget.position} [pos] If unspecified, return the current
* position. Otherwise, set the current position.
* @param {boolean} [actualValue] If getting the position, if this is truthy,
* always return the stored value, not a value adjusted for display.
* @returns {geo.gui.widget.position|this} Either the position or the widget
* instance. If this is the position and `actualValue` is falsy,
* positions that specify an explicit `x` and `y` parameter will be
* converted to a value that can be used by the display css.
*/
this.position = function (pos) {
this.position = function (pos, actualValue) {
if (pos !== undefined) {
arg.position = pos;
this.layer().geoOff(geo_event.pan, m_this.repositionEvent);
m_position = pos;
if (m_position.hasOwnProperty('x') && m_position.hasOwnProperty('y')) {
this.layer().geoOn(geo_event.pan, m_this.repositionEvent);
}
this.reposition();
return this;
}
var position;

if (arg &&
arg.hasOwnProperty('position') &&
arg.position.hasOwnProperty('x') &&
arg.position.hasOwnProperty('y')) {

position = m_this.layer().map().gcsToDisplay(arg.position);
if (m_position.hasOwnProperty('x') && m_position.hasOwnProperty('y') && !actualValue) {
var position = m_this.layer().map().gcsToDisplay(m_position);

return {
left: position.x,
Expand All @@ -153,14 +187,15 @@ var widget = function (arg) {
};
}

return arg.position;
return m_position;
};

/**
* Repositions a widget based on the argument passed, or calling position on
* the widget itself.
* @param {object} position A position with the form:
* { top: m, left: n }
* Repositions a widget.
*
* @param {geo.gui.widget.position} [position] The new position for the
* widget. `undefined` uses the stored position value.
* @returns {this}
*/
this.reposition = function (position) {
position = position || m_this.position();
Expand All @@ -171,21 +206,30 @@ var widget = function (arg) {
// if the property is a number, add px to it, otherwise set it to the
// specified value. Setting a property to null clears it. Setting to
// undefined doesn't alter it.
if (/^\s*(\-|\+)?(\d+(\.\d*)?|\d*\.\d+)([eE](\-|\+)?\d+)?\s*$/.test(position[cssAttr])) {
if (/^\s*(-|\+)?(\d+(\.\d*)?|\d*\.\d+)([eE](-|\+)?\d+)?\s*$/.test(position[cssAttr])) {
Copy link
Member

@aashish24 aashish24 Dec 12, 2017

Choose a reason for hiding this comment

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

whats change here? sorry it is hard to debug for review 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed some needless escaping; that is all.

m_this.canvas().style[cssAttr] = ('' + position[cssAttr]).trim() + 'px';
} else {
m_this.canvas().style[cssAttr] = position[cssAttr];
}
}
}
return m_this;
};

/**
* If the position is based on map coordinates, this gets called when the
* map is panned to resposition the widget.
*
* @returns {this}
*/
this.repositionEvent = function () {
return m_this.reposition();
};

/**
* Determines whether or not the widget is completely within the viewport.
* Report if the widget is completely within the viewport.
*
* @returns {boolean} True if the widget is completely within the viewport.
*/
this.isInViewport = function () {
var position = m_this.position();
Expand All @@ -195,10 +239,7 @@ var widget = function (arg) {
(position.left <= layer.width() && position.top <= layer.height()));
};

if (arg &&
arg.hasOwnProperty('position') &&
arg.position.hasOwnProperty('x') &&
arg.position.hasOwnProperty('y')) {
if (m_position.hasOwnProperty('x') && m_position.hasOwnProperty('y')) {
this.layer().geoOn(geo_event.pan, m_this.repositionEvent);
}
};
Expand Down
1 change: 0 additions & 1 deletion src/util/mockVGL.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ module.exports.restoreVGLRenderer = function () {
vgl.renderWindow = _renderWindow;
vglRenderer.supported = _supported;
delete vgl._mocked;
// delete vgl._mockedRenderWindow;
delete vgl.mockCounts;
}
};
Loading