From b4306da7bb2461fda24e6a54bc8194993bea1e86 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Thu, 4 Jul 2019 16:06:13 +0200 Subject: [PATCH 1/4] Fix font comparison, less context.font operations --- package.json | 2 +- src/ol/render/canvas.js | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 724298fb0c..e96e4fa199 100644 --- a/package.json +++ b/package.json @@ -80,7 +80,7 @@ "loglevelnext": "^3.0.1", "marked": "0.6.3", "mocha": "6.1.4", - "ol-mapbox-style": "^5.0.0-beta.2", + "ol-mapbox-style": "^5.0.0-beta.3", "pixelmatch": "^5.0.0", "pngjs": "^3.4.0", "proj4": "2.5.0", diff --git a/src/ol/render/canvas.js b/src/ol/render/canvas.js index 0dc981a605..515cbab8db 100644 --- a/src/ol/render/canvas.js +++ b/src/ol/render/canvas.js @@ -180,6 +180,10 @@ export const checkedFonts = {}; */ let measureContext = null; +/** + * @type {string} + */ +let measureFont; /** * @type {!Object} @@ -238,6 +242,7 @@ export const checkFont = (function() { clear(textHeights); // Make sure that loaded fonts are picked up by Safari measureContext = null; + measureFont = undefined; labelCache.clear(); } else { ++checked[font]; @@ -317,8 +322,8 @@ export const measureTextHeight = (function() { */ export function measureTextWidth(font, text) { const measureContext = getMeasureContext(); - if (font != measureContext.font) { - measureContext.font = font; + if (font != measureFont) { + measureContext.font = measureFont = font; } return measureContext.measureText(text).width; } From 5616c535b0f17c5773e6426c322693887c6cc698 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Thu, 4 Jul 2019 16:10:28 +0200 Subject: [PATCH 2/4] Clean up properly when clearing label cache --- src/ol/render/canvas.js | 2 ++ src/ol/render/canvas/LabelCache.js | 2 +- src/ol/renderer/canvas/VectorTileLayer.js | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ol/render/canvas.js b/src/ol/render/canvas.js index 515cbab8db..c0ce02f7e5 100644 --- a/src/ol/render/canvas.js +++ b/src/ol/render/canvas.js @@ -243,7 +243,9 @@ export const checkFont = (function() { // Make sure that loaded fonts are picked up by Safari measureContext = null; measureFont = undefined; + if (labelCache.getCount()) { labelCache.clear(); + } } else { ++checked[font]; done = false; diff --git a/src/ol/render/canvas/LabelCache.js b/src/ol/render/canvas/LabelCache.js index b2ea1493e0..578ff22f0b 100644 --- a/src/ol/render/canvas/LabelCache.js +++ b/src/ol/render/canvas/LabelCache.js @@ -21,8 +21,8 @@ class LabelCache extends LRUCache { } clear() { - super.clear(); this.consumers = {}; + super.clear(); } /** diff --git a/src/ol/renderer/canvas/VectorTileLayer.js b/src/ol/renderer/canvas/VectorTileLayer.js index 8bd84485e0..9a6ce54b54 100644 --- a/src/ol/renderer/canvas/VectorTileLayer.js +++ b/src/ol/renderer/canvas/VectorTileLayer.js @@ -24,6 +24,7 @@ import { makeInverse } from '../../transform.js'; import CanvasExecutorGroup, {replayDeclutter} from '../../render/canvas/ExecutorGroup.js'; +import {clear} from '../../obj.js'; /** @@ -378,6 +379,7 @@ class CanvasVectorTileLayerRenderer extends CanvasTileLayerRenderer { * @inheritDoc */ handleFontsChanged() { + clear(this.renderTileImageQueue_); const layer = this.getLayer(); if (layer.getVisible() && this.renderedLayerRevision_ !== undefined) { layer.changed(); From ab2d97d49b2f89590af665fde146dfe635eb0b2e Mon Sep 17 00:00:00 2001 From: ahocevar Date: Thu, 4 Jul 2019 16:11:02 +0200 Subject: [PATCH 3/4] Don't give up too early when waiting for fonts --- src/ol/render/canvas.js | 2 +- test/spec/ol/render/canvas/index.test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ol/render/canvas.js b/src/ol/render/canvas.js index c0ce02f7e5..ed5994c0ce 100644 --- a/src/ol/render/canvas.js +++ b/src/ol/render/canvas.js @@ -196,7 +196,7 @@ export const textHeights = {}; * @param {string} fontSpec CSS font spec. */ export const checkFont = (function() { - const retries = 60; + const retries = 100; const checked = checkedFonts; const size = '32px '; const referenceFonts = ['monospace', 'serif']; diff --git a/test/spec/ol/render/canvas/index.test.js b/test/spec/ol/render/canvas/index.test.js index 560cf4698b..05c439c8f7 100644 --- a/test/spec/ol/render/canvas/index.test.js +++ b/test/spec/ol/render/canvas/index.test.js @@ -17,10 +17,10 @@ describe('ol.render.canvas', function() { render.measureTextHeight('12px sans-serif'); }); - const retries = 60; + const retries = 100; it('does not clear label cache and measurements for unavailable fonts', function(done) { - this.timeout(3000); + this.timeout(4000); const spy = sinon.spy(); listen(render.labelCache, 'clear', spy); const interval = setInterval(function() { From 4b48997a0bb122ddf03634f44832d40b389a5567 Mon Sep 17 00:00:00 2001 From: ahocevar Date: Thu, 4 Jul 2019 16:12:35 +0200 Subject: [PATCH 4/4] Check font style and weight in addition to family --- src/ol/css.js | 23 ++++++-- src/ol/render/canvas.js | 68 ++++++++++++------------ test/spec/ol/css.test.js | 26 ++++++--- test/spec/ol/render/canvas/index.test.js | 7 +-- 4 files changed, 77 insertions(+), 47 deletions(-) diff --git a/src/ol/css.js b/src/ol/css.js index a7eab92100..74455ec41a 100644 --- a/src/ol/css.js +++ b/src/ol/css.js @@ -2,6 +2,13 @@ * @module ol/css */ +/** + * @typedef {Object} FontParameters + * @property {Array} families + * @property {string} style + * @property {string} weight + */ + /** * The CSS class for hidden feature. @@ -62,10 +69,13 @@ export const CLASS_COLLAPSED = 'ol-collapsed'; * Get the list of font families from a font spec. Note that this doesn't work * for font families that have commas in them. * @param {string} The CSS font property. - * @return {Object} The font families (or null if the input spec is invalid). + * @return {FontParameters} The font families (or null if the input spec is invalid). */ -export const getFontFamilies = (function() { +export const getFontParameters = (function() { let style; + /** + * @type {Object} + */ const cache = {}; return function(font) { if (!style) { @@ -74,11 +84,18 @@ export const getFontFamilies = (function() { if (!(font in cache)) { style.font = font; const family = style.fontFamily; + const fontWeight = style.fontWeight; + const fontStyle = style.fontStyle; style.font = ''; if (!family) { return null; } - cache[font] = family.split(/,\s?/); + const families = family.split(/,\s?/); + cache[font] = { + families: families, + weight: fontWeight, + style: fontStyle + }; } return cache[font]; }; diff --git a/src/ol/render/canvas.js b/src/ol/render/canvas.js index ed5994c0ce..a5430f5fae 100644 --- a/src/ol/render/canvas.js +++ b/src/ol/render/canvas.js @@ -1,7 +1,7 @@ /** * @module ol/render/canvas */ -import {getFontFamilies} from '../css.js'; +import {getFontParameters} from '../css.js'; import {createCanvasContext2D} from '../dom.js'; import {clear} from '../obj.js'; import {create as createTransform} from '../transform.js'; @@ -204,32 +204,30 @@ export const checkFont = (function() { const text = 'wmytzilWMYTZIL@#/&?$%10\uF013'; let interval, referenceWidth; - function isAvailable(font) { + /** + * @param {string} fontStyle Css font-style + * @param {string} fontWeight Css font-weight + * @param {*} fontFamily Css font-family + * @return {boolean} Font with style and weight is available + */ + function isAvailable(fontStyle, fontWeight, fontFamily) { const context = getMeasureContext(); - // Check weight ranges according to - // https://developer.mozilla.org/en-US/docs/Web/CSS/font-weight#Fallback_weights - for (let weight = 100; weight <= 700; weight += 300) { - const fontWeight = weight + ' '; - let available = true; - for (let i = 0; i < len; ++i) { - const referenceFont = referenceFonts[i]; - context.font = fontWeight + size + referenceFont; - referenceWidth = context.measureText(text).width; - if (font != referenceFont) { - context.font = fontWeight + size + font + ',' + referenceFont; - const width = context.measureText(text).width; - // If width and referenceWidth are the same, then the fallback was used - // instead of the font we wanted, so the font is not available. - available = available && width != referenceWidth; - } - } - if (available) { - // Consider font available when it is available in one weight range. - //FIXME With this we miss rare corner cases, so we should consider - //FIXME checking availability for each requested weight range. - return true; + let available = true; + for (let i = 0; i < len; ++i) { + const referenceFont = referenceFonts[i]; + context.font = fontStyle + ' ' + fontWeight + ' ' + size + referenceFont; + referenceWidth = context.measureText(text).width; + if (fontFamily != referenceFont) { + context.font = fontStyle + ' ' + fontWeight + ' ' + size + fontFamily + ',' + referenceFont; + const width = context.measureText(text).width; + // If width and referenceWidth are the same, then the fallback was used + // instead of the font we wanted, so the font is not available. + available = available && width != referenceWidth; } } + if (available) { + return true; + } return false; } @@ -237,14 +235,14 @@ export const checkFont = (function() { let done = true; for (const font in checked) { if (checked[font] < retries) { - if (isAvailable(font)) { + if (isAvailable.apply(this, font.split('\n'))) { checked[font] = retries; clear(textHeights); // Make sure that loaded fonts are picked up by Safari measureContext = null; measureFont = undefined; if (labelCache.getCount()) { - labelCache.clear(); + labelCache.clear(); } } else { ++checked[font]; @@ -259,16 +257,18 @@ export const checkFont = (function() { } return function(fontSpec) { - const fontFamilies = getFontFamilies(fontSpec); - if (!fontFamilies) { + const font = getFontParameters(fontSpec); + if (!font) { return; } - for (let i = 0, ii = fontFamilies.length; i < ii; ++i) { - const fontFamily = fontFamilies[i]; - if (!(fontFamily in checked)) { - checked[fontFamily] = retries; - if (!isAvailable(fontFamily)) { - checked[fontFamily] = 0; + const families = font.families; + for (let i = 0, ii = families.length; i < ii; ++i) { + const family = families[i]; + const key = font.style + '\n' + font.weight + '\n' + family; + if (!(key in checked)) { + checked[key] = retries; + if (!isAvailable(font.style, font.weight, family)) { + checked[key] = 0; if (interval === undefined) { interval = setInterval(check, 32); } diff --git a/test/spec/ol/css.test.js b/test/spec/ol/css.test.js index 5a422d4bb8..47c95d1e92 100644 --- a/test/spec/ol/css.test.js +++ b/test/spec/ol/css.test.js @@ -1,44 +1,56 @@ -import {getFontFamilies} from '../../../src/ol/css.js'; +import {getFontParameters} from '../../../src/ol/css.js'; describe('ol.css', function() { - describe('getFontFamilies()', function() { + describe('getFontParameters()', function() { const cases = [{ font: '2em "Open Sans"', + style: 'normal', + weight: 'normal', families: ['"Open Sans"'] }, { font: '2em \'Open Sans\'', + style: 'normal', + weight: 'normal', families: ['"Open Sans"'] }, { font: '2em "Open Sans", sans-serif', + style: 'normal', + weight: 'normal', families: ['"Open Sans"', 'sans-serif'] }, { font: 'italic small-caps bolder 16px/3 cursive', + style: 'italic', + weight: 'bolder', families: ['cursive'] }, { font: 'garbage 2px input', families: null }, { font: '100% fantasy', + style: 'normal', + weight: 'normal', families: ['fantasy'] }]; cases.forEach(function(c, i) { it('works for ' + c.font, function() { - const families = getFontFamilies(c.font); + const font = getFontParameters(c.font); if (c.families === null) { - expect(families).to.be(null); + expect(font).to.be(null); return; } - families.forEach(function(family, j) { + font.families.forEach(function(family, j) { // Safari uses single quotes for font families, so we have to do extra work if (family.charAt(0) === '\'') { // we wouldn't want to do this in the lib since it doesn't properly escape quotes // but we know that our test cases don't include quotes in font names - families[j] = '"' + family.slice(1, -1) + '"'; + font.families[j] = '"' + family.slice(1, -1) + '"'; } }); - expect(families).to.eql(c.families); + expect(font.style).to.eql(c.style); + expect(font.weight).to.eql(c.weight); + expect(font.families).to.eql(c.families); }); }); diff --git a/test/spec/ol/render/canvas/index.test.js b/test/spec/ol/render/canvas/index.test.js index 05c439c8f7..c89828ef11 100644 --- a/test/spec/ol/render/canvas/index.test.js +++ b/test/spec/ol/render/canvas/index.test.js @@ -24,7 +24,7 @@ describe('ol.render.canvas', function() { const spy = sinon.spy(); listen(render.labelCache, 'clear', spy); const interval = setInterval(function() { - if (render.checkedFonts['foo'] == retries && render.checkedFonts['sans-serif'] == retries) { + if (render.checkedFonts['normal\nnormal\nfoo'] == retries && render.checkedFonts['normal\nnormal\nsans-serif'] == retries) { clearInterval(interval); unlisten(render.labelCache, 'clear', spy); expect(spy.callCount).to.be(0); @@ -39,7 +39,7 @@ describe('ol.render.canvas', function() { const spy = sinon.spy(); listen(render.labelCache, 'clear', spy); const interval = setInterval(function() { - if (render.checkedFonts['sans-serif'] == retries) { + if (render.checkedFonts['normal\nnormal\nsans-serif'] == retries) { clearInterval(interval); unlisten(render.labelCache, 'clear', spy); expect(spy.callCount).to.be(0); @@ -54,7 +54,7 @@ describe('ol.render.canvas', function() { const spy = sinon.spy(); listen(render.labelCache, 'clear', spy); const interval = setInterval(function() { - if (render.checkedFonts['monospace'] == retries) { + if (render.checkedFonts['normal\nnormal\nmonospace'] == retries) { clearInterval(interval); unlisten(render.labelCache, 'clear', spy); expect(spy.callCount).to.be(0); @@ -67,6 +67,7 @@ describe('ol.render.canvas', function() { it('clears label cache and measurements for fonts that become available', function(done) { head.appendChild(font); + render.labelCache.set('dummy', {}); listen(render.labelCache, 'clear', function() { expect(render.textHeights).to.eql({}); done();