From 6aba9b22d6927aff7755a3d7e9acd67b6507e563 Mon Sep 17 00:00:00 2001 From: wimrijnders Date: Fri, 21 Jul 2017 19:00:28 +0200 Subject: [PATCH] Fix infinite loop on drawing of large labels (#3228) * Code cleanup, for better understanding * Further refactoring; text processing to blocks in separate method * Added unit test for labels - tests standard text and html tags * Labels added unit tests for markdown * Further refactoring; made multi and regular handling more congruent * Interim save, not there yet * Unit tests done, first working version * Added test case with two big words * Code cleanup * Break up huge words into lines. * Restore unrelated code change --- .../modules/components/shared/Label.js | 450 ++++++++++++------ test/Label.test.js | 403 ++++++++++++++++ 2 files changed, 718 insertions(+), 135 deletions(-) create mode 100644 test/Label.test.js diff --git a/lib/network/modules/components/shared/Label.js b/lib/network/modules/components/shared/Label.js index 09954ea8e..da037b9df 100644 --- a/lib/network/modules/components/shared/Label.js +++ b/lib/network/modules/components/shared/Label.js @@ -1,5 +1,130 @@ let util = require('../../../../util'); + +/** + * Internal helper class used for splitting a label text into lines. + * + * This has been moved away from the label processing code for better undestanding upon reading. + * + * @private + */ +class LabelAccumulator { + constructor(measureText) { + this.measureText = measureText; // callback to determine text dimensions, using the parent label settings. + this.current = 0; + this.width = 0; + this.height = 0; + this.lines = []; + } + + + /** + * Append given text to the given line. + * + * @param {number} l index of line to add to + * @param {string} text string to append to line + * @param {boolean} mod id of multi-font to use, default 'normal' + * @private + */ + _add(l, text, mod = 'normal') { + if (text === undefined || text === "") return; + + if (this.lines[l] === undefined) { + this.lines[l] = { + width : 0, + height: 0, + blocks: [] + }; + } + + // Determine width and get the font properties + let result = this.measureText(text, mod); + let block = Object.assign({}, result.values); + block.text = text; + block.width = result.width; + block.mod = mod; + + this.lines[l].blocks.push(block); + + // Update the line width. We need this for + // determining if a string goes over max width + this.lines[l].width += result.width; + } + + + /** + * Returns the width in pixels of the current line. + */ + curWidth() { + let line = this.lines[this.current]; + if (line === undefined) return 0; + + return line.width; + } + + + /** + * Add text in block to current line + */ + append(text, mod = 'normal') { + this._add(this.current, text, mod); + } + + + /** + * Add text in block to current line and start a new line + */ + newLine(text, mod = 'normal') { + this._add(this.current, text, mod); + this.current++; + } + + + /** + * Set the sizes for all lines and the whole thing. + */ + finalize() { + // console.log(JSON.stringify(this.lines, null, 2)); + + // Determine the heights of the lines + // Note that width has already been set + for (let k = 0; k < this.lines.length; k++) { + let line = this.lines[k]; + let height = 0; + + for (let l = 0; l < line.blocks.length; l++) { + let block = line.blocks[l]; + height += block.height; + } + + line.height = height; + } + + // Determine the full label size + let width = 0; + let height = 0; + for (let k = 0; k < this.lines.length; k++) { + let line = this.lines[k]; + + if (line.width > width) { + width = line.width; + } + height += line.height; + } + + this.width = width; + this.height = height; + + // Return a simple hash object for further processing. + return { + width : this.width, + height: this.height, + lines : this.lines + } + } +} + + class Label { constructor(body, options, edgelabel = false) { this.body = body; @@ -15,7 +140,7 @@ class Label { setOptions(options, allowDeletion = false) { this.elementOptions = options; - // We want to keep the font options seperated from the node options. + // We want to keep the font options separated from the node options. // The node options have to mirror the globals when they are not overruled. this.fontOptions = util.deepExtend({},options.font, true); @@ -796,155 +921,210 @@ class Label { differentState(selected, hover) { return ((selected !== this.fontOptions.selectedState) && (hover !== this.fontOptions.hoverState)); } + /** - * This explodes the label string into lines and sets the width, height and number of lines. + * This explodes the passed text into lines and determines the width, height and number of lines. + * * @param ctx * @param selected + * @param hover + * @param {String} text the text to explode * @private */ - _processLabel(ctx, selected, hover) { - let width = 0; - let height = 0; - let nlLines = []; - let lines = []; - let k = 0; - lines.add = function(l, text, font, color, width, height, vadjust, mod, strokeWidth, strokeColor) { - if (this.length == l) { - this[l] = { width: 0, height: 0, blocks: [] }; - } - this[l].blocks.push({ text, font, color, width, height, vadjust, mod, strokeWidth, strokeColor }); - } - lines.accumulate = function(l, width, height) { - this[l].width += width; - this[l].height = height > this[l].height ? height : this[l].height; - } - lines.addAndAccumulate = function(l, text, font, color, width, height, vadjust, mod, strokeWidth, strokeColor) { - this.add(l, text, font, color, width, height, vadjust, mod, strokeWidth, strokeColor); - this.accumulate(l, width, height); - } - if (this.elementOptions.label !== undefined) { - let nlLines = String(this.elementOptions.label).split('\n'); - let lineCount = nlLines.length; - if (this.elementOptions.font.multi) { - for (let i = 0; i < lineCount; i++) { - let blocks = this.splitBlocks(nlLines[i], this.elementOptions.font.multi); - let lineWidth = 0; - let lineHeight = 0; - if (blocks) { - if (blocks.length == 0) { - let values = this.getFormattingValues(ctx, selected, hover, "normal"); - lines.addAndAccumulate(k, "", values.font, values.color, 0, values.size, values.vadjust, "normal", values.strokeWidth, values.strokeColor); - height += lines[k].height; - k++; - continue; - } - for (let j = 0; j < blocks.length; j++) { - if (this.fontOptions.maxWdt > 0) { - let values = this.getFormattingValues(ctx, selected, hover, blocks[j].mod); - let words = blocks[j].text.split(" "); - let atStart = true - let text = ""; - let measure = { width: 0 }; - let lastMeasure; - let w = 0; - while (w < words.length) { - let pre = atStart ? "" : " "; - lastMeasure = measure; - measure = ctx.measureText(text + pre + words[w]); - if ((lineWidth + measure.width > this.fontOptions.maxWdt) && - (lastMeasure.width != 0)) { - lineHeight = (values.height > lineHeight) ? values.height : lineHeight; - lines.add(k, text, values.font, values.color, lastMeasure.width, values.height, values.vadjust, blocks[j].mod, values.strokeWidth, values.strokeColor); - lines.accumulate(k, lastMeasure.width, lineHeight); - text = ""; - atStart = true; - lineWidth = 0; - width = lines[k].width > width ? lines[k].width : width; - height += lines[k].height; - k++; - } else { - text = text + pre + words[w]; - if (w === words.length-1) { - lineHeight = (values.height > lineHeight) ? values.height : lineHeight; - lineWidth += measure.width; - lines.add(k, text, values.font, values.color, measure.width, values.height, values.vadjust, blocks[j].mod, values.strokeWidth, values.strokeColor); - lines.accumulate(k, measure.width, lineHeight); - if (j === blocks.length-1) { - width = lines[k].width > width ? lines[k].width : width; - height += lines[k].height; - k++; - } - } - w++; - atStart = false; - } - } - } else { - let values = this.getFormattingValues(ctx, selected, hover, blocks[j].mod); - let measure = ctx.measureText(blocks[j].text); - lines.addAndAccumulate(k, blocks[j].text, values.font, values.color, measure.width, values.height, values.vadjust, blocks[j].mod, values.strokeWidth, values.strokeColor); - width = lines[k].width > width ? lines[k].width : width; - if (blocks.length-1 === j) { - height += lines[k].height; - k++; - } - } - } + _processLabelText(ctx, selected, hover, text) { + let self = this; + + + /** + * Callback to determine text width; passed to LabelAccumulator instance + * + * @param {String} text string to determine width of + * @param {String} mod font type to use for this text + * @return {Object} { width, values} width in pixels and font attributes + */ + let textWidth = function(text, mod) { + if (text === undefined) return 0; + + // TODO: This can be done more efficiently with caching + let values = self.getFormattingValues(ctx, selected, hover, mod); + + let width = 0; + if (text !== '') { + // NOTE: The following may actually be *incorrect* for the mod fonts! + // This returns the size with a regular font, bold etc. may + // have different sizes. + let measure = ctx.measureText(text); + width = measure.width; + } + + return {width, values: values}; + }; + + + let lines = new LabelAccumulator(textWidth); + + if (text === undefined || text === "") { + return lines.finalize(); + } + + + let overMaxWidth = function(text) { + let width = ctx.measureText(text).width; + return (lines.curWidth() + width > self.fontOptions.maxWdt); + } + + + /** + * Determine the longest part of the sentence which still fits in the + * current max width. + * + * @param {Array} words Array of strings signifying a text lines + * @return index of first item in string making string go over max + */ + let getLongestFit = function(words) { + let text = ''; + let w = 0; + + while (w < words.length) { + let pre = (text === '') ? '' : ' '; + let newText = text + pre + words[w]; + + if (overMaxWidth(newText)) break; + text = newText; + w++; + } + + return w; + } + + /** + * Determine the longest part of th string which still fits in the + * current max width. + * + * @param {Array} words Array of strings signifying a text lines + * @return index of first item in string making string go over max + */ + let getLongestFitWord = function(word) { + let w = 0; + + while (w < word.length) { + if (overMaxWidth(word.slice(0,w))) break; + w++; + } + + return w; + } + + + let splitStringIntoLines = function(str, mod = 'normal', appendLast = false) { + let words = str.split(" "); + + while (words.length > 0) { + let w = getLongestFit(words); + + if (w === 0) { + // Special case: the first word may already + // be larger than the max width. + let word = words[0]; + + // Break the word to the largest part that fits the line + let x = getLongestFitWord(word); + lines.newLine(word.slice(0, x), mod); + + // Adjust the word, so that the rest will be done next iteration + words[0] = word.slice(x); + } else { + let text = words.slice(0, w).join(" "); + + if (w == words.length && appendLast) { + lines.append(text, mod); + } else { + lines.newLine(text, mod); + } + + words = words.slice(w); + } + } + } + + + let nlLines = String(text).split('\n'); + let lineCount = nlLines.length; + + if (this.elementOptions.font.multi) { + // Multi-font case: styling tags active + for (let i = 0; i < lineCount; i++) { + let blocks = this.splitBlocks(nlLines[i], this.elementOptions.font.multi); + if (blocks === undefined) continue; + + if (blocks.length === 0) { + lines.newLine(""); + continue; + } + + if (this.fontOptions.maxWdt > 0) { + // widthConstraint.maximum defined + //console.log('Running widthConstraint multi, max: ' + this.fontOptions.maxWdt); + for (let j = 0; j < blocks.length; j++) { + let mod = blocks[j].mod; + let text = blocks[j].text; + splitStringIntoLines(text, mod, true); + } + } else { + // widthConstraint.maximum NOT defined + for (let j = 0; j < blocks.length; j++) { + let mod = blocks[j].mod; + let text = blocks[j].text; + lines.append(text, mod); } } + + lines.newLine(); + } + } else { + // Single-font case + if (this.fontOptions.maxWdt > 0) { + // widthConstraint.maximum defined + // console.log('Running widthConstraint normal, max: ' + this.fontOptions.maxWdt); + for (let i = 0; i < lineCount; i++) { + splitStringIntoLines(nlLines[i]); + } } else { + // widthConstraint.maximum NOT defined for (let i = 0; i < lineCount; i++) { - let values = this.getFormattingValues(ctx, selected, hover, "normal"); - if (this.fontOptions.maxWdt > 0) { - let words = nlLines[i].split(" "); - let text = ""; - let measure = { width: 0 }; - let lastMeasure; - let w = 0; - while (w < words.length) { - let pre = (text === "") ? "" : " "; - lastMeasure = measure; - measure = ctx.measureText(text + pre + words[w]); - if ((measure.width > this.fontOptions.maxWdt) && (lastMeasure.width != 0)) { - lines.addAndAccumulate(k, text, values.font, values.color, lastMeasure.width, values.size, values.vadjust, "normal", values.strokeWidth, values.strokeColor) - width = lines[k].width > width ? lines[k].width : width; - height += lines[k].height; - text = ""; - k++; - } else { - text = text + pre + words[w]; - if (w === words.length-1) { - lines.addAndAccumulate(k, text, values.font, values.color, measure.width, values.size, values.vadjust, "normal", values.strokeWidth, values.strokeColor) - width = lines[k].width > width ? lines[k].width : width; - height += lines[k].height; - k++; - } - w++; - } - } - } else { - let text = nlLines[i]; - let measure = ctx.measureText(text); - lines.addAndAccumulate(k, text, values.font, values.color, measure.width, values.size, values.vadjust, "normal", values.strokeWidth, values.strokeColor); - width = lines[k].width > width ? lines[k].width : width; - height += lines[k].height; - k++; - } + lines.newLine(nlLines[i]); } } } - if ((this.fontOptions.minWdt > 0) && (width < this.fontOptions.minWdt)) { - width = this.fontOptions.minWdt; + + return lines.finalize(); + } + + + /** + * This explodes the label string into lines and sets the width, height and number of lines. + * @param ctx + * @param selected + * @param hover + * @private + */ + _processLabel(ctx, selected, hover) { + let state = this._processLabelText(ctx, selected, hover, this.elementOptions.label); + + if ((this.fontOptions.minWdt > 0) && (state.width < this.fontOptions.minWdt)) { + state.width = this.fontOptions.minWdt; } - this.size.labelHeight = height; - if ((this.fontOptions.minHgt > 0) && (height < this.fontOptions.minHgt)) { - height = this.fontOptions.minHgt; + + this.size.labelHeight =state.height; + if ((this.fontOptions.minHgt > 0) && (state.height < this.fontOptions.minHgt)) { + state.height = this.fontOptions.minHgt; } - this.lines = lines; - this.lineCount = lines.length; - this.size.width = width; - this.size.height = height; + + this.lines = state.lines; + this.lineCount = state.lines.length; + this.size.width = state.width; + this.size.height = state.height; this.selectedState = selected; this.hoverState = hover; } diff --git a/test/Label.test.js b/test/Label.test.js new file mode 100644 index 000000000..97b504af6 --- /dev/null +++ b/test/Label.test.js @@ -0,0 +1,403 @@ +/** + * TODO - add tests for: + * ==== + * + * - !!! good test case with the tags for max width + * - pathological cases of spaces (and other whitespace!) + * - html unclosed or unopened tags + * - html tag combinations with no font defined (e.g. bold within mono) + */ +var assert = require('assert') +var Label = require('../lib/network/modules/components/shared/Label').default; +var NodesHandler = require('../lib/network/modules/NodesHandler').default; + +/************************************************************** + * Dummy class definitions for minimal required functionality. + **************************************************************/ + +class DummyContext { + measureText(text) { + return { + width: 12*text.length, + height: 14 + }; + } +} + + +class DummyLayoutEngine { + positionInitially() {} +} + +/************************************************************** + * End Dummy class definitions + **************************************************************/ + + +describe('Network Label', function() { + + /** + * Retrieve options object from a NodesHandler instance + * + * NOTE: these are options at the node-level + */ + function getOptions(options = {}) { + var body = { + functions: {}, + emitter: { + on: function() {} + } + } + + var nodesHandler = new NodesHandler(body, {}, options, new DummyLayoutEngine() ); + //console.log(JSON.stringify(nodesHandler.options, null, 2)); + + return nodesHandler.options; + } + + + /** + * Check if the returned lines and blocks are as expected. + * + * All width/height fields and font info are ignored. + * Within blocks, only the text is compared + */ + function checkBlocks(returned, expected) { + let showBlocks = () => { + return '\nreturned: ' + JSON.stringify(returned, null, 2) + '\n' + + 'expected: ' + JSON.stringify(expected, null, 2); + } + + assert.equal(expected.lines.length, returned.lines.length, 'Number of lines does not match, ' + showBlocks()); + + for (let i = 0; i < returned.lines.length; ++i) { + let retLine = returned.lines[i]; + let expLine = expected.lines[i]; + + assert(retLine.blocks.length === expLine.blocks.length, 'Number of blocks does not match, ' + showBlocks()); + for (let j = 0; j < retLine.blocks.length; ++j) { + let retBlock = retLine.blocks[j]; + let expBlock = expLine.blocks[j]; + + assert(retBlock.text === expBlock.text, 'Text does not match, ' + showBlocks()); + + assert(retBlock.mod !== undefined); + if (retBlock.mod === 'normal' || retBlock.mod === '') { + assert(expBlock.mod === undefined || expBlock.mod === 'normal' || expBlock === '', + 'No mod field expected in returned, ' + showBlocks()); + } else { + assert(retBlock.mod === expBlock.mod, 'Mod fields do not match, line: ' + i + ', block: ' + j + + '; ret: ' + retBlock.mod + ', exp: ' + expBlock.mod + '\n' + showBlocks()); + } + } + } + } + + + function checkProcessedLabels(label, text, expected) { + var ctx = new DummyContext(); + + for (var i in text) { + var ret = label._processLabelText(ctx, false, false, text[i]); + //console.log(JSON.stringify(ret, null, 2)); + checkBlocks(ret, expected[i]); + } + } + + +/************************************************************** + * Test data + **************************************************************/ + + var normal_text = [ + "label text", + "label\nwith\nnewlines", + "OnereallylongwordthatshouldgooverwidthConstraint.maximumifdefined", + "One really long sentence that should go over widthConstraint.maximum if defined", + "Reallyoneenormouslylargelabel withtwobigwordsgoingoverwayovermax" + ] + + var html_text = [ + "label with some multi tags", + "label with some \n multi tags\n and newlines" // NB spaces around \n's + ]; + + var markdown_text = [ + "label *with* `some` _multi *tags*_", + "label *with* `some` \n _multi *tags*_\n and newlines" // NB spaces around \n's + ]; + + + +/************************************************************** + * Expected Results + **************************************************************/ + + + var normal_expected = [{ + // In first item, width/height kept in for reference + width: 120, + height: 14, + lines: [{ + width: 120, + height: 14, + blocks: [{ + text: "label text", + width: 120, + height: 14, + }] + }] + }, { + lines: [{ + blocks: [{text: "label"}] + }, { + blocks: [{text: "with"}] + }, { + blocks: [{text: "newlines"}] + }] + }, { + // From here onward, changes width max width set + lines: [{ + blocks: [{text: "OnereallylongwordthatshouldgooverwidthConstraint.maximumifdefined"}] + }] + }, { + lines: [{ + blocks: [{text: "One really long sentence that should go over widthConstraint.maximum if defined"}] + }] + }, { + lines: [{ + blocks: [{text: "Reallyoneenormouslylargelabel withtwobigwordsgoingoverwayovermax"}] + }] + }]; + + const indexWidthConstrained = 2; // index of first item that will be different with max width set + + var normal_widthConstraint_expected = normal_expected.slice(0, indexWidthConstrained); + Array.prototype.push.apply(normal_widthConstraint_expected, [{ + lines: [{ + blocks: [{text: "Onereallylongwordthatshoul"}] + }, { + blocks: [{text: "dgooverwidthConstraint.max"}] + }, { + blocks: [{text: "imumifdefined"}] + }] + }, { + lines: [{ + blocks: [{text: "One really long sentence"}] + }, { + blocks: [{text: "that should go over"}] + }, { + blocks: [{text: "widthConstraint.maximum"}] + }, { + blocks: [{text: "if defined"}] + }] + }, { + lines: [{ + blocks: [{text: "Reallyoneenormouslylargela"}] + }, { + blocks: [{text: "bel"}] + }, { + blocks: [{text: "withtwobigwordsgoingoverwa"}] + }, { + blocks: [{text: "yovermax"}] + }] + }]); + + + var html_unchanged_expected = [{ + lines: [{ + blocks: [{text: "label with some multi tags"}] + }] + }, { + lines: [{ + blocks: [{text: "label with some "}] + }, { + blocks: [{text: " multi tags"}] + }, { + blocks: [{text: " and newlines"}] + }] + }]; + + var html_widthConstraint_unchanged = [{ + lines: [{ + blocks: [{text: "label with"}] + }, { + blocks: [{text: "some"}] + }, { + blocks: [{text: "multi tags"}] + }] + }, { + lines: [{ + blocks: [{text: "label with"}] + }, { + blocks: [{text: "some "}] + }, { + blocks: [{text: " multi tags"}] + }, { + blocks: [{text: " and newlines"}] + }] + }]; + + + var markdown_unchanged_expected = [{ + lines: [{ + blocks: [{text: "label *with* `some` _multi *tags*_"}] + }] + }, { + lines: [{ + blocks: [{text: "label *with* `some` "}] + }, { + blocks: [{text: " _multi *tags*_"}] + }, { + blocks: [{text: " and newlines"}] + }] + }]; + + + var markdown_widthConstraint_expected = [{ + lines: [{ + blocks: [{text: "label *with* `some`"}] + }, { + blocks: [{text: "_multi *tags*_"}] + }] + }, { + lines: [{ + blocks: [{text: "label *with* `some` "}] + }, { + blocks: [{text: " _multi *tags*_"}] + }, { + blocks: [{text: " and newlines"}] + }] + }]; + + + var multi_expected = [{ + lines: [{ + blocks: [ + {text: "label "}, + {text: "with" , mod: 'bold'}, + {text: " "}, + {text: "some" , mod: 'mono'}, + {text: " "}, + {text: "multi ", mod: 'ital'}, + {text: "tags" , mod: 'boldital'} + ] + }] + }, { + lines: [{ + blocks: [ + {text: "label "}, + {text: "with" , mod: 'bold'}, + {text: " "}, + {text: "some" , mod: 'mono'}, + {text: " "} + ] + }, { + blocks: [ + {text: " "}, + {text: "multi ", mod: 'ital'}, + {text: "tags" , mod: 'boldital'} + ] + }, { + blocks: [{text: " and newlines"}] + }] + }]; + + + +/************************************************************** + * End Expected Results + **************************************************************/ + + it('parses normal text labels', function (done) { + var label = new Label({}, getOptions()); + + checkProcessedLabels(label, normal_text , normal_expected); + checkProcessedLabels(label, html_text , html_unchanged_expected); // html unchanged + checkProcessedLabels(label, markdown_text, markdown_unchanged_expected); // markdown unchanged + + done(); + }); + + + it('parses html labels', function (done) { + var options = getOptions(options); + options.font.multi = true; // TODO: also test 'html', also test illegal value here + + var label = new Label({}, options); + + checkProcessedLabels(label, normal_text , normal_expected); // normal as usual + checkProcessedLabels(label, html_text , multi_expected); + checkProcessedLabels(label, markdown_text, markdown_unchanged_expected); // markdown unchanged + + done(); + }); + + + it('parses markdown labels', function (done) { + var options = getOptions(options); + options.font.multi = 'markdown'; // TODO: also test 'md', also test illegal value here + + var label = new Label({}, options); + + checkProcessedLabels(label, normal_text , normal_expected); // normal as usual + checkProcessedLabels(label, html_text , html_unchanged_expected); // html unchanged + checkProcessedLabels(label, markdown_text, multi_expected); + + done(); + }); + + + it('handles normal text with widthConstraint.maximum', function (done) { + var options = getOptions(options); + + // + // What the user would set: + // + // options.widthConstraint = { minimum: 100, maximum: 200}; + // + // No sense in adding minWdt, not used when splitting labels into lines + // + // This comment also applies to the usage of maxWdt in the test cases below + // + options.font.maxWdt = 300; + + var label = new Label({}, options); + + checkProcessedLabels(label, normal_text , normal_widthConstraint_expected); + checkProcessedLabels(label, html_text , html_widthConstraint_unchanged); // html unchanged + checkProcessedLabels(label, markdown_text, markdown_widthConstraint_expected); // markdown unchanged + + done(); + }); + + + it('handles html tags with widthConstraint.maximum', function (done) { + var options = getOptions(options); + options.font.multi = true; + options.font.maxWdt = 300; + + var label = new Label({}, options); + + checkProcessedLabels(label, normal_text , normal_widthConstraint_expected); + checkProcessedLabels(label, html_text , multi_expected); + checkProcessedLabels(label, markdown_text, markdown_widthConstraint_expected); + + done(); + }); + + + it('handles markdown tags with widthConstraint.maximum', function (done) { + var options = getOptions(options); + options.font.multi = 'markdown'; + options.font.maxWdt = 300; + + var label = new Label({}, options); + + checkProcessedLabels(label, normal_text , normal_widthConstraint_expected); + checkProcessedLabels(label, html_text , html_widthConstraint_unchanged); + checkProcessedLabels(label, markdown_text, multi_expected); + + done(); + }); +});