Fix:issues#15716, compute the actual height of the text style background

[Frank-Chan]

Fix issue#15716

[ahocevar]

Suggested change

lineHeight = Math.max(lineHeight, currentHeight);

And the lineHeight variable defined in line 376 is no longer needed.

[Frank-Chan]

Yes，you are right.

[github-actions]

📦 Preview the website for this branch here: https://deploy-preview-15717--ol-site.netlify.app/.

[ahocevar]

Thanks @Frank-Chan, see my comments about a suggested change.

[Frank-Chan]

@ahocevar Modified and submitted, please review

[ahocevar]

@Frank-Chan Look at the failing rendering tests. Seems the test fixes your issue, but breaks things in cases where the font changes within a single line. The best thing would be to add a rendering test for your case first, and then work on a solution that fixes your case without breaking the existing ones.

[ahocevar]

Looks like from the code before your pull request, the only change needed is

--- a/src/ol/render/canvas.js
+++ b/src/ol/render/canvas.js
@@ -381,6 +381,7 @@ export function getTextDimensions(baseStyle, chunks) {
       lineWidths.push(lineWidth);
       lineWidth = 0;
       height += lineHeight;
+      lineHeight = 0;
       continue;
     }
     const font = chunks[i + 1] || baseStyle.font;

And for a rendering test, you could use the following as main.js:

import Feature from '../../../../src/ol/Feature.js';
import Fill from '../../../../src/ol/style/Fill.js';
import Map from '../../../../src/ol/Map.js';
import Point from '../../../../src/ol/geom/Point.js';
import Stroke from '../../../../src/ol/style/Stroke.js';
import Style from '../../../../src/ol/style/Style.js';
import Text from '../../../../src/ol/style/Text.js';
import VectorLayer from '../../../../src/ol/layer/Vector.js';
import VectorSource from '../../../../src/ol/source/Vector.js';
import View from '../../../../src/ol/View.js';

const vectorSource = new VectorSource({
  features: [new Feature(new Point([0, 0]))],
});

const labelStyle = new Style({
  text: new Text({
    font: '13px Ubuntu,sans-serif',
    fill: new Fill({
      color: '#000',
    }),
    stroke: new Stroke({
      color: '#fff',
      width: 4,
    }),
    padding: [0, 0, 0, 0],
    overflow: false,
  }),
});

new Map({
  pixelRatio: 1,
  layers: [
    new VectorLayer({
      source: vectorSource,
      style: function (feature) {
        const textStyle = labelStyle.getText();
        textStyle.setFont('13px Ubuntu,sans-serif');
        textStyle.setBackgroundFill(
          new Fill({
            color: 'blue',
          }),
        );
        textStyle.setBackgroundStroke(
          new Stroke({
            color: 'red',
            width: 2,
          }),
        );
        textStyle.setStroke(
          new Stroke({
            color: '#b4ebaf',
            width: 4,
          }),
        );
        textStyle.setText([
          '30',
          'bold 23px/23px serif',
          '\n',
          '',
          ' Montana',
          '900 11px/11px serif',
          '\n',
          '',
          '6.858 people/mi²',
          'italic 11px/11px Ubuntu,sans-serif',
        ]);
        return labelStyle;
      },
    }),
  ],
  target: 'map',
  view: new View({
    center: [0, 0],
    resolution: 1,
  }),
});

render({tolerance: 0.01});

You can generate the reference image with

node test/rendering/test.js --fix --match rich-text-multiline

when the new test directory for that main.js is test/rendering/cases/richt-text-multiline.

[Frank-Chan]

@Frank-Chan Look at the failing rendering tests. Seems the test fixes your issue, but breaks things in cases where the font changes within a single line. The best thing would be to add a rendering test for your case first, and then work on a solution that fixes your case without breaking the existing ones.

Yes，I hadn't thought about the single line before. I've been changed now.
Thanks for your suggestion!

[ahocevar]

I just added a rendering test, and updated an existing one that actually showed the issue which is now fixed.

[ahocevar]

Thanks, @Frank-Chan