Skip to content

Commit

Permalink
Make sure to trigger an immediate render when enabling a layer.
Browse files Browse the repository at this point in the history 
This was a common race condition on any layers that depend on a service.

Before, in the layer `enable` methods, we would call `startAsync()` but never
actually chain off that Promise to trigger a redraw.  Enabling the layer _would_
trigger `layerchange` and a redraw, but if the layer code was still waiting for
`startAsync()` to settle, the redraw wouldn't do anything.

The effect was that you could toggle on a layer like streetside or mapillary,
but you wouldn't see any markers appear until you did something else later to
trigger another redraw, like move the mouse to hover over something.

This change also exposed a few situations where `startAsync()` wasn't actually
returning a chainable `Promise` - these return `Promise.resolve()` now.
  • Loading branch information
@bhousel
bhousel committed Dec 11, 2024
1 parent efe022b commit 07cd5a5
Show file tree
Hide file tree
Showing 17 changed files with 183 additions and 122 deletions.
15 changes: 9 additions & 6 deletions modules/pixi/PixiLayerGeoScribble.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,12 @@ export class PixiLayerGeoScribble extends AbstractLayer {
if (val === this._enabled) return; // no change
this._enabled = val;

if (val) {
this.dirtyLayer();
this.context.services.geoScribble.startAsync();
const context = this.context;
const gfx = context.systems.gfx;
const geoScribble = context.services.geoScribble;
if (val && geoScribble) {
geoScribble.startAsync()
.then(() => gfx.immediateRedraw());
}
}

Expand All @@ -77,10 +80,10 @@ export class PixiLayerGeoScribble extends AbstractLayer {
render(frame, viewport, zoom) {
if (!this.enabled) return;

const service = this.context.services.geoScribble;
service.loadTiles();
const geoScribble = this.context.services.geoScribble;
geoScribble.loadTiles();

const geoData = service.getData();
const geoData = geoScribble.getData();

// No polygons will be returned by the service, so we don't need to consider those types.
const lines = geoData.filter(d => d.geometry.type === 'LineString' || d.geometry.type === 'MultiLineString');
Expand Down
23 changes: 13 additions & 10 deletions modules/pixi/PixiLayerKartaPhotos.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,12 @@ export class PixiLayerKartaPhotos extends AbstractLayer {
if (val === this._enabled) return; // no change
this._enabled = val;

if (val) {
this.dirtyLayer();
this.context.services.kartaview.startAsync();
const context = this.context;
const gfx = context.systems.gfx;
const kartaview = context.services.kartaview;
if (val && kartaview) {
kartaview.startAsync()
.then(() => gfx.immediateRedraw());
}
}

Expand Down Expand Up @@ -142,12 +145,12 @@ export class PixiLayerKartaPhotos extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
renderMarkers(frame, viewport, zoom) {
const service = this.context.services.kartaview;
if (!service?.started) return;
const kartaview = this.context.services.kartaview;
if (!kartaview?.started) return;

const parentContainer = this.scene.groups.get('streetview');
let images = service.getImages();
let sequences = service.getSequences();
let images = kartaview.getImages();
let sequences = kartaview.getSequences();

sequences = this.filterSequences(sequences);
images = this.filterImages(images);
Expand Down Expand Up @@ -239,10 +242,10 @@ export class PixiLayerKartaPhotos extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
render(frame, viewport, zoom) {
const service = this.context.services.kartaview;
if (!this.enabled || !service?.started || zoom < MINZOOM) return;
const kartaview = this.context.services.kartaview;
if (!this.enabled || !kartaview?.started || zoom < MINZOOM) return;

service.loadTiles();
kartaview.loadTiles();
this.renderMarkers(frame, viewport, zoom);
}

Expand Down
23 changes: 13 additions & 10 deletions modules/pixi/PixiLayerKeepRight.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ export class PixiLayerKeepRight extends AbstractLayer {
if (val === this._enabled) return; // no change
this._enabled = val;

if (val) {
this.dirtyLayer();
this.context.services.keepRight.startAsync();
const context = this.context;
const gfx = context.systems.gfx;
const keepRight = context.services.keepRight;
if (val && keepRight) {
keepRight.startAsync()
.then(() => gfx.immediateRedraw());
}
}

Expand All @@ -59,11 +62,11 @@ export class PixiLayerKeepRight extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
renderMarkers(frame, viewport, zoom) {
const service = this.context.services.keepRight;
if (!service?.started) return;
const keepRight = this.context.services.keepRight;
if (!keepRight?.started) return;

const parentContainer = this.scene.groups.get('qa');
const items = service.getData();
const items = keepRight.getData();

for (const d of items) {
const featureID = `${this.layerID}-${d.id}`;
Expand All @@ -72,7 +75,7 @@ export class PixiLayerKeepRight extends AbstractLayer {
if (!feature) {
const style = {
markerName: 'keepright',
markerTint: service.getColor(d.parentIssueType)
markerTint: keepRight.getColor(d.parentIssueType)
};

feature = new PixiFeaturePoint(this, featureID);
Expand All @@ -97,10 +100,10 @@ export class PixiLayerKeepRight extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
render(frame, viewport, zoom) {
const service = this.context.services.keepRight;
if (!this.enabled || !service?.started || zoom < MINZOOM) return;
const keepRight = this.context.services.keepRight;
if (!this.enabled || !keepRight?.started || zoom < MINZOOM) return;

service.loadTiles();
keepRight.loadTiles();
this.renderMarkers(frame, viewport, zoom);
}

Expand Down
21 changes: 12 additions & 9 deletions modules/pixi/PixiLayerMapRoulette.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ export class PixiLayerMapRoulette extends AbstractLayer {
if (val === this._enabled) return; // no change
this._enabled = val;

if (val) {
this.dirtyLayer();
this.context.services.maproulette.startAsync();
const context = this.context;
const gfx = context.systems.gfx;
const maproulette = context.services.maproulette;
if (val && maproulette) {
maproulette.startAsync()
.then(() => gfx.immediateRedraw());
}
}

Expand All @@ -59,11 +62,11 @@ export class PixiLayerMapRoulette extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
renderMarkers(frame, projection, zoom) {
const service = this.context.services.maproulette;
if (!service?.started) return;
const maproulette = this.context.services.maproulette;
if (!maproulette?.started) return;

const parentContainer = this.scene.groups.get('qa');
const items = service.getData();
const items = maproulette.getData();

for (const d of items) {
const featureID = `${this.layerID}-${d.id}`;
Expand Down Expand Up @@ -102,10 +105,10 @@ export class PixiLayerMapRoulette extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
render(frame, projection, zoom) {
const service = this.context.services.maproulette;
if (!this.enabled || !service?.started || zoom < MINZOOM) return;
const maproulette = this.context.services.maproulette;
if (!this.enabled || !maproulette?.started || zoom < MINZOOM) return;

service.loadTiles();
maproulette.loadTiles();
this.renderMarkers(frame, projection, zoom);
}

Expand Down
23 changes: 13 additions & 10 deletions modules/pixi/PixiLayerMapillaryDetections.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,12 @@ export class PixiLayerMapillaryDetections extends AbstractLayer {
if (val === this._enabled) return; // no change
this._enabled = val;

if (val) {
this.dirtyLayer();
this.context.services.mapillary.startAsync();
const context = this.context;
const gfx = context.systems.gfx;
const mapillary = context.services.mapillary;
if (val && mapillary) {
mapillary.startAsync()
.then(() => gfx.immediateRedraw());
}
}

Expand Down Expand Up @@ -86,12 +89,12 @@ export class PixiLayerMapillaryDetections extends AbstractLayer {
const context = this.context;
const presets = context.systems.presets;

const service = context.services.mapillary;
if (!service?.started) return;
const mapillary = context.services.mapillary;
if (!mapillary?.started) return;

const parentContainer = this.scene.groups.get('qa');

let items = service.getData('detections');
let items = mapillary.getData('detections');
items = this.filterDetections(items);

for (const d of items) {
Expand All @@ -109,7 +112,7 @@ export class PixiLayerMapillaryDetections extends AbstractLayer {

if (feature.dirty) {
const isSelected = feature.hasClass('selectdetection');
const presetID = service.getDetectionPresetID(d.value);
const presetID = mapillary.getDetectionPresetID(d.value);
const preset = presetID && presets.item(presetID);

const style = {
Expand Down Expand Up @@ -137,10 +140,10 @@ export class PixiLayerMapillaryDetections extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
render(frame, viewport, zoom) {
const service = this.context.services.mapillary;
if (!this.enabled || !service?.started || zoom < MINZOOM) return;
const mapillary = this.context.services.mapillary;
if (!this.enabled || !mapillary?.started || zoom < MINZOOM) return;

service.loadTiles('detections');
mapillary.loadTiles('detections');
this.renderMarkers(frame, viewport, zoom);
}

Expand Down
31 changes: 17 additions & 14 deletions modules/pixi/PixiLayerMapillaryPhotos.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ export class PixiLayerMapillaryPhotos extends AbstractLayer {
this._fovchanged = this._fovchanged.bind(this);

if (this.supported) {
const service = this.context.services.mapillary;
service.on('bearingChanged', this._bearingchanged);
service.on('fovChanged', this._fovchanged);
service.on('imageChanged', () => {
const mapillary = this.context.services.mapillary;
mapillary.on('bearingChanged', this._bearingchanged);
mapillary.on('fovChanged', this._fovchanged);
mapillary.on('imageChanged', () => {
this._viewerFov = 55;
});
}
Expand Down Expand Up @@ -133,9 +133,12 @@ export class PixiLayerMapillaryPhotos extends AbstractLayer {
if (val === this._enabled) return; // no change
this._enabled = val;

if (val) {
this.dirtyLayer();
this.context.services.mapillary.startAsync();
const context = this.context;
const gfx = context.systems.gfx;
const mapillary = context.services.mapillary;
if (val && mapillary) {
mapillary.startAsync()
.then(() => gfx.immediateRedraw());
}
}

Expand Down Expand Up @@ -214,15 +217,15 @@ export class PixiLayerMapillaryPhotos extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
renderMarkers(frame, viewport, zoom) {
const service = this.context.services.mapillary;
if (!service?.started) return;
const mapillary = this.context.services.mapillary;
if (!mapillary?.started) return;

// const showMarkers = (zoom >= MINMARKERZOOM);
// const showViewfields = (zoom >= MINVIEWFIELDZOOM);

const parentContainer = this.scene.groups.get('streetview');
let sequences = service.getSequences();
let images = service.getData('images');
let sequences = mapillary.getSequences();
let images = mapillary.getData('images');

sequences = this.filterSequences(sequences);
images = this.filterImages(images);
Expand Down Expand Up @@ -324,10 +327,10 @@ export class PixiLayerMapillaryPhotos extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
render(frame, viewport, zoom) {
const service = this.context.services.mapillary;
if (!this.enabled || !service?.started || zoom < MINZOOM) return;
const mapillary = this.context.services.mapillary;
if (!this.enabled || !mapillary?.started || zoom < MINZOOM) return;

service.loadTiles('images');
mapillary.loadTiles('images');
this.renderMarkers(frame, viewport, zoom);
}

Expand Down
21 changes: 12 additions & 9 deletions modules/pixi/PixiLayerMapillarySigns.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,12 @@ export class PixiLayerMapillarySigns extends AbstractLayer {
if (val === this._enabled) return; // no change
this._enabled = val;

if (val) {
this.dirtyLayer();
this.context.services.mapillary.startAsync();
const context = this.context;
const gfx = context.systems.gfx;
const mapillary = context.services.mapillary;
if (val && mapillary) {
mapillary.startAsync()
.then(() => gfx.immediateRedraw());
}
}

Expand Down Expand Up @@ -80,13 +83,13 @@ export class PixiLayerMapillarySigns extends AbstractLayer {
*/
renderMarkers(frame, viewport, zoom) {
const context = this.context;
const service = context.services.mapillary;
if (!service?.started) return;
const mapillary = context.services.mapillary;
if (!mapillary?.started) return;

const container = context.container();
const parentContainer = this.scene.groups.get('qa');

let items = service.getData('signs');
let items = mapillary.getData('signs');
items = this.filterDetections(items);

for (const d of items) {
Expand Down Expand Up @@ -136,10 +139,10 @@ export class PixiLayerMapillarySigns extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
render(frame, viewport, zoom) {
const service = this.context.services.mapillary;
if (!this.enabled || !service?.started || zoom < MINZOOM) return;
const mapillary = this.context.services.mapillary;
if (!this.enabled || !mapillary?.started || zoom < MINZOOM) return;

service.loadTiles('signs');
mapillary.loadTiles('signs');
this.renderMarkers(frame, viewport, zoom);
}

Expand Down
15 changes: 9 additions & 6 deletions modules/pixi/PixiLayerOsm.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,12 @@ export class PixiLayerOsm extends AbstractLayer {
if (val === this._enabled) return; // no change
this._enabled = val;

if (val) {
this.dirtyLayer();
this.context.services.osm.startAsync();
const context = this.context;
const gfx = context.systems.gfx;
const osm = context.services.osm;
if (val && osm) {
osm.startAsync()
.then(() => gfx.immediateRedraw());
}
}

Expand Down Expand Up @@ -123,10 +126,10 @@ export class PixiLayerOsm extends AbstractLayer {
* @param zoom Effective zoom to use for rendering
*/
render(frame, viewport, zoom) {
const service = this.context.services.osm;
if (!this.enabled || !service?.started || zoom < MINZOOM) return;

const context = this.context;
const osm = context.services.osm;
if (!this.enabled || !osm?.started || zoom < MINZOOM) return;

const editor = context.systems.editor;
const filters = context.systems.filters;
const graph = editor.staging.graph;
Expand Down
Loading

0 comments on commit 07cd5a5

Please sign in to comment.