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

lineHeight does not affect height as expected #336

Closed
Marine-Dos-Wiztivi opened this issue Sep 15, 2021 · 10 comments
Closed

lineHeight does not affect height as expected #336

Marine-Dos-Wiztivi opened this issue Sep 15, 2021 · 10 comments

Comments

@Marine-Dos-Wiztivi
Copy link
Contributor

In textTextureRenderer, height is calculated by the following line :
`// calculate text height
lineHeight = lineHeight || fontSize;

    let height;
    if (h) {
        height = h;
    } else {
        height = lineHeight * (lines.length - 1) + 0.5 * fontSize + Math.max(lineHeight, fontSize) + offsetY;
    }

When lineHeight is defined, I think that it will be more accurate to just have :height = lineHeight * (lines.length - 1);`

For example, in case that lineHeight is smaller than fontHeight, text should be cropped.
On the following image, you can see the display with CSS on the left, and with lightning on the right, with a lineHeight of 10.
Screenshot from 2021-09-15 08-55-31

Does it makes sense for you ?

@g-zachar
Copy link
Contributor

Hi @Marine-Dos-Wiztivi,

the code fragment you're referring is out of date, this problem you're mentioning has been updated in version 2.3.0. In short, if you change textBaseline property to bottom you should expect more consistent behaviour. However, if your intention is to clip the texture, you should instead use techniques described here.

Let me know if you have any further questions.

Best regards

@Marine-Dos-Wiztivi
Copy link
Contributor Author

Marine-Dos-Wiztivi commented Sep 15, 2021

Hi @g-zachar,

You are right, the code shared is not up to date. Here code from 2.5.0 :
` lineHeight = lineHeight || fontSize;

    let height;
    if (h) {
        height = h;
    } else {
        const baselineOffset = (this._settings.textBaseline != 'bottom') ? 0.5 * fontSize : 0;
        height = lineHeight * (lines.length - 1) + baselineOffset + Math.max(lineHeight, fontSize) + offsetY;
    }

`

If I set textBaseline to bottom, here is the result :
Screenshot from 2021-09-15 12-30-30

If I also change height to height = lineHeight * lines.length, here is the result :
bottom+lineHeight

In first case, text is cropped on the top but not on the bottom. Text texture is bigger than expectation, so you can see the bottom of the "p"
Same result, event if there is not direct visual impact, if I set a two lines text with a smaller line height and draw a rectangle base on finalW and finalH, here what is the result :
Screenshot from 2021-09-15 12-42-07

I won't be able to use clip in that scenario because I do not know the number of lines.

In case that lineHeight is not defined, I understand that we should guess / round lineHeight. But in result, I think that height of the texture should be number of lines * height of a line.

Regards

@g-zachar
Copy link
Contributor

Hi @Marine-Dos-Wiztivi,

I don't think I understand your use case. Can you perhaps provide a code example?

Best regards

@Marine-Dos-Wiztivi
Copy link
Contributor Author

Hi @g-zachar,

Of course !
Here a complete HTML page :
`

<script src="lightning.js"></script>
Text with big margin
Text displayed under
<script type="module"> window.onload = function () { const options = { stage: { w: 1920, h: 1080/*, clearColor: 0xFF000000*/ } }, app = new lng.Application(options); document.body.appendChild(app.stage.getCanvas());
    const initialX = 50,
        initialY = 50;
    let txt = app.stage.createElement({
        text: {
            fontSize: 36,
            text: 'Text with big margin',
            wordWrapWidth: 200,
            lineHeight: 20,
            textColor: 0xffff0000,
            textBaseline: "bottom",
            verticalAlign: "middle"
        },
        width: 200,
        x: initialX,
        y: initialY,
    });
    txt.on("txLoaded", () => {
        console.log(window.txt.finalH);
        let txt2 = app.stage.createElement({
            text: {
                fontSize: 36,
                text: 'Text displayed under',
                wordWrapWidth: 200,
                textColor: 0xffff0000,
                textBaseline: "bottom",
                verticalAlign: "middle"
            },
            width: 200,
            x: initialX,
            y: initialY + window.txt.finalH,
        });
        app.stage.root.add(txt2);
    });
    app.stage.root.add(txt);
    window.app = app;
    window.txt = txt;

};
</script> `

And here what I get. In red, text with lightning, in black text with HTML / CSS.
Screenshot from 2021-09-24 13-27-20

I want to put a text below an other one, but I a get unexpected margin between them. Sans-serif is not the best font to do the demo, because there is text overlapping. But some font have big margin above and under letter, and I want to specify line-height to reduce that space. That is working. But if I want to reuse the height of the text with line-height, I would expect 40px in my scenario (2 lines of 20px) but get 56.

Regards

@g-zachar
Copy link
Contributor

@Marine-Dos-Wiztivi,

If I understood correctly you just want to align two texts with each other. Please see this example and let me know if it helped: https://jsfiddle.net/qg0bxn42/1/

Best regards

@Marine-Dos-Wiztivi
Copy link
Contributor Author

Hi @g-zachar,

I still have the same issue with your example. I made the jsfiddle with flex and compare with a CSS renderer : https://jsfiddle.net/tLyfkz9p/1/. As you can see, with lightning, you have a "margin" below the first text that does not appear in CSS.

Regards

@g-zachar
Copy link
Contributor

@Marine-Dos-Wiztivi

I still don't quite understand your use case, but please bear in mind that while lightning's text rendering has similarities with CSS engine, it will not behave the same way in many scenarios. Is this what you're after? https://jsfiddle.net/z8fhwv9u/2/

Best regards

@Marine-Dos-Wiztivi
Copy link
Contributor Author

@g-zachar

I am using multiple renderer engine, including CSS and lightning, and my requirement is to have similar behavior between those renderers. I understand that rendering can't be exactly the same but I think it could be closer between lightning and CSS regarding the line height.
Current calcul in lightning seems to be based on the fact that we can't know the exact value of line height and so is calculated based on font size. But in reality, if line height is defined, it seems to make sens that height of the texture is number of line multiplied by the height of one line. That is how CSS renderer works when you set a line-height in pixels.

Here is what I suggest to not impact use case where lineHeight is not set, but to allow to have texture height equal to number of lines multiplied by line height when it is set : 640f3d3.

Regards

@g-zachar
Copy link
Contributor

g-zachar commented Oct 4, 2021

@Marine-Dos-Wiztivi

I see. Current line height calculation is most likely in place to specifically avoid undesired clipping behaviour and probably also include additional space for possible shadow. But more importantly, the change you propose would not be backwards compatible.

Coming back to your original question, you can read texture data after it loads. My suggestion is to create text wrapper component that will aim to fix the discrepancies between CSS and lightning's rendering, e.g.: https://jsfiddle.net/xapd5nsq/

Best regards

@Marine-Dos-Wiztivi
Copy link
Contributor Author

Hi @g-zachar,

Thanks for your reply. I didn't get that we could retrieve number of line in the return of txLoaded. That's perfectly solve my issue.

Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants