iText cursor placement issue

[paulcanning]

I have noticed a possible bug with cursor placement in an iText object, when using Google Fonts.

I have linked to an imgur album showing two images to show the problem. You can see that where the cursor appears, is not exactly where it types.

http://imgur.com/a/x07Aa

(The cursor appears in front of the period, but types after it)

[jedsmithobd]

I have noticed this with other fonts as well. These fonts while not Google Fonts are being dynamically loaded. For us this also affects the box showing the selected object when we use a very wide font.

[ajck]

I've just opened another issue that may have the same root cause? Link: https://github.com/kangax/fabric.js/issues/2614

[ajck]

@asturur I think the easiest, quickest way to fix this (either temporarily or permanently) is to be able to tell the itext or canvas to recalculate it's font widths cache when the font is updated for a specific itext. e.g. in my app user is able to change font by loading a new web font, and I'm using Google's web font loader for this, but I mainly see the bug when I reload a stringified canvas with loadfromJSON. Being able to loop through the itexts in that and just tell them to recalculate the character width cache would be great :). Thanks.

[asturur]

My idea is to move the fontCache from "iText." level to "fabric." level. So one font one cache. Then make a method "fabric.clearFontCache(fontFamily)" Even when we will get a proper font loading mechanism this could be usefull.

[ajck]

@asturur Sounds good. Is there a quick workaround we can do even now - e.g. some small patch to force a font width recalculation for an itext, when we want it?

[asturur]

closed with #2995.

Call fabric.util.clearFabricFontCache(fontName) to clear the char widths cache of all objects. I moved the cache to fabric level not iText to avoid duplication in case of multiple text. Removed shadow from cache declariation, charwidths does not change with shadow. I have some doubts about stroke, but i see that svg has a similar behaviour. Also insterting strokewidth in cache would mean add spacing to letters with increasing strokeWidth.

[StephanBijzitter]

fabric.util.clearFabricFontCache is undefined in version 1.6.2, which version would be needed?

[asturur]

Well i do not remember. if in changelog is not specified ... https://github.com/kangax/fabric.js/wiki/Changelog-(draft) is it. is written there, open the page and do a search.

Anyway, depending on how much you want to keep your app updated, the text in 2.0 beta is way better. You may have to do some effort to update the app to 2.0 api but you get way better cursor placement in general.

I m also available to discuss, clear, help with migration since for me, working it alone, is easy to miss some developer problem during a code refactor.

[StephanBijzitter]

We have extended fabric to the point where updating is very time consuming and very fragile (last time we made a mistake that cost us a few hundred grand), so we're quite a bit behind right now because it's stable for us.

Reason I found this issue is because we currently have an issue with the latest update for the webkit (but not blink) engine that turns our text objects into question marks when the customer changes the font. It seems the new webkit update triggers our system to think the font is loaded, before it's actually loaded. Doing a re-render after a few seconds "fixes" it, but the calculations are still wrong which is causing incorrect wrapping.

We're currently also completely re-writing everything, but we'll maintain the old version until we're finished. So it'll be a while before we can make the switch to 2.0

[asturur]

I m also interested in pain point during big fabric extensions. I m currentlyd doing same, working with other people, and realizing some things needs extension out of the box and the dev experience should be as smooth as possible.

So feel free to report bothersome issues on extending.

[jay4499]

Can you please give me example to how to clear cache.

[kondurisai]

I am facing the same issue. Is this issue closed ?

[ipfedor]

same problem with cyrillic texts

[asturur]

On version 2, this is solved. You need to load fonts correctly before using then, or look the docs for how to clear font cache

[ipfedor]

it's not the cache

a few useless remarks:

1) Save the canvas with text to json, restore it - the bounds of size is incorrect. We set the text attribute bold - the size is restored to the desired. By manipulating style bold/italic we change the size of bounds. 2) The scheme can be changed on the contrary, set the bolt, save and load - there will be an erroneous size. After that, remove the bold, the size will be restored.

ctx.measureText - returned different size of bolded text from 1) or 2)

[asturur]

How are you loading the fonts?

[adimachmali]

This issue shouldn't be close, even though I am loading using FontFaceObserver and even setting canvas.getActiveObject().set("fontFamily", 'FONT_NAME'); The issue still exits. Try typing "hello hello hello hello jjjj" (or copy-paste that) and the text indication location on the j will be wrong.

[asturur]

can you leave a reproducible fiddle where we i can replicate the bug?

[bmartinson]

For those experiencing this bug, what platform/browser are you experiencing the issue on and what are the definitions you are using for your font? I found this issue was reproducible even after waiting for FontFaceObserver to load on Safari 12 and Firefox on various Linux distributions (CentOS7, Ubuntu 18.04). In my case, I noted that the cases where I was rendering my font size had a very large precision from scaling from various storage DPIs. I found that the general size seemed accurate of the highlighted region, but the text characters themselves were a bit off, so I modified the _renderChars function to render characters one at a time rather than in bulk between style definitions. This completely solved my issue and I have a very accurate rendering now in the platforms where I didn't before.

[ajck]

Sounds interesting Brian, could you possibly share your modifications to _renderChars? Thanks

On Monday, 8 April 2019, Brian Martinson notifications@github.com wrote:

For those experiencing this bug, what platform/browser are you experiencing the issue on and what are the definitions you are using for your font? I found this issue was reproducible even after waiting for FontFaceObserver to load on Safari 12 and Firefox on various Linux distributions (CentOS7, Ubuntu 18.04). In my case, I noted that the cases where I was rendering my font size had a very large precision from scaling from various storage DPIs. I found that the general size seemed accurate of the highlighted region, but the text characters themselves were a bit off, so I modified the _renderChars function to render characters one at a time rather than in bulk between style definitions. This completely solved my issue and I have a very accurate rendering now in the platforms where I didn't before.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fabricjs/fabric.js/issues/2612#issuecomment-481039459, or mute the thread https://github.com/notifications/unsubscribe-auth/AKzwevDjJvmlEqV4gErK02RhNL0gotbZks5ve8tBgaJpZM4Genqa .

[bmartinson]

Sounds interesting Brian, could you possibly share your modifications to _renderChars? Thanks

Absolutely. I just overrode the default implementation via the prototype chain and tried to do as minimal amount of change as possible in order to have it render character by character. Where we determine when it is time to render, I just updated to confirm that we have analyzed as least one character:

timeToRender = this._hasStyleChanged(actualStyle, nextStyle) || charsToRender.length === 1;

The full implementation in Typescript:

/**
 * Internal implementation of _renderChars to improve cross platform/browser rendering for high precision fonts and certain font faces. Here, we render each
 * character individually relying on the start position of Fabric.js to be more accurate than that of the canvas implementation that can vary across platforms.
 * Fabric.js measures font faces at a large scale to avoid precision loss that is seen when measuring at standard scale in the canvas.
 *
 * @param method The rendering method to use.
 * @param ctx The canvas context to render with.
 * @param line The line that needs to be rendered.
 * @param left The starting position.
 * @param top The top position of the line.
 * @param lineIndex The line index in the text element.
 * @private
 */
fabric.Text.prototype._renderChars = function(
    method: string,
    ctx: CanvasRenderingContext2D,
    line: string,
    left: number,
    top: number,
    lineIndex: number,
): void {
    // set proper line offset
    const lineHeight: number = this.getHeightOfLine(lineIndex);
    const isJustify: boolean = this.textAlign.indexOf("justify") !== -1;
    let actualStyle;
    let nextStyle;
    let charsToRender = "";
    let charBox;
    let boxWidth = 0;
    let timeToRender;
    const shortCut: boolean = !isJustify && this.charSpacing === 0 && this.isEmptyStyles(lineIndex);

    ctx.save();
    top -= lineHeight * this._fontSizeFraction / this.lineHeight;
    if (shortCut) {
        // render all the line in one pass without checking
        this._renderChar(method, ctx, lineIndex, 0, this.textLines[lineIndex], left, top, lineHeight);
        ctx.restore();
        return;
    }
    for (let i = 0, len = line.length - 1; i <= len; i++) {
        timeToRender = i === len || this.charSpacing;
        charsToRender += line[i];
        charBox = this.__charBounds[lineIndex][i];
        if (boxWidth === 0) {
            left += charBox.kernedWidth - charBox.width;
            boxWidth += charBox.width;
        } else {
            boxWidth += charBox.kernedWidth;
        }
        if (isJustify && !timeToRender) {
            if (this._reSpaceAndTab.test(line[i])) {
                timeToRender = true;
            }
        }
        if (!timeToRender) {
            // if we have charSpacing, we render char by char
            if (!actualStyle) {
                actualStyle = this.getCompleteStyleDeclaration(lineIndex, i);
            }
            nextStyle = this.getCompleteStyleDeclaration(lineIndex, i + 1);
            timeToRender = this._hasStyleChanged(actualStyle, nextStyle) || charsToRender.length === 1;
        }
        if (timeToRender) {
            this._renderChar(method, ctx, lineIndex, i, charsToRender, left, top, lineHeight);
            charsToRender = "";
            actualStyle = nextStyle;
            left += boxWidth;
            boxWidth = 0;
        }
    }
    ctx.restore();
};

[ajck]

That's great, thanks, appreciated

------ Original Message ------ From: "Brian Martinson" notifications@github.com To: "fabricjs/fabric.js" fabric.js@noreply.github.com Cc: "Alex Kerr" ajckerr@gmail.com; "Comment" comment@noreply.github.com Sent: 08/04/2019 21:19:50 Subject: Re: [fabricjs/fabric.js] iText cursor placement issue (#2612)

Sounds interesting Brian, could you possibly share your modifications to _renderChars? Thanks

Absolutely. I just overrode the default implementation via the prototype chain and tried to do as minimal amount of change as possible in order to have it render character by character. Where we determine when it is time to render, I just updated to confirm that we have analyzed as least one character:

timeToRender=this._hasStyleChanged(actualStyle, nextStyle) ||charsToRender.length===1; The full implementation in Typescript:

/* Internal implementation of _renderChars to improve cross platform/browser rendering for high precision fonts and certain font faces. Here, we render each character individually relying on the start position of Fabric.js to be more accurate than that of the canvas implementation that can vary across platforms. Fabric.js measures font faces at a large scale to avoid precision loss that is seen when measuring at standard scale in the canvas. @parammethod The rendering method to use. @paramctx The canvas context to render with. @paramline The line that needs to be rendered. @paramleft The starting position. @paramtop The top position of the line. @paramlineIndex The line index in the text element. @private*/fabric.Text.prototype._renderChars=function( method:string, ctx:CanvasRenderingContext2D, line:string, left:number, top:number, lineIndex:number, ):void { // set proper line offsetconst lineHeight:number=this.getHeightOfLine(lineIndex); const isJustify:boolean=this.textAlign.indexOf("justify") !==-1; let actualStyle; let nextStyle; let charsToRender =""; let charBox; let boxWidth =0; let timeToRender; const shortCut:boolean=!isJustify&&this.charSpacing===0&&this.isEmptyStyles(lineIndex);

ctx.save();
top-=lineHeight*this._fontSizeFraction/this.lineHeight;
if (shortCut) {
    // render all the line in one pass without checkingthis._renderChar(method, ctx, lineIndex, 0, this.textLines[lineIndex], left, top, lineHeight);
    ctx.restore();
    return;
}
for (let i =0, len =line.length-1; i<=len; i++) {
    timeToRender=i===len||this.charSpacing;
    charsToRender+=line[i];
    charBox=this.__charBounds[lineIndex][i];
    if (boxWidth===0) {
        left+=charBox.kernedWidth-charBox.width;
        boxWidth+=charBox.width;
    } else {
        boxWidth+=charBox.kernedWidth;
    }
    if (isJustify&&!timeToRender) {
        if (this._reSpaceAndTab.test(line[i])) {
            timeToRender=true;
        }
    }
    if (!timeToRender) {
        // if we have charSpacing, we render char by charif (!actualStyle) {
            actualStyle=this.getCompleteStyleDeclaration(lineIndex, i);
        }
        nextStyle=this.getCompleteStyleDeclaration(lineIndex, i+1);
        timeToRender=this._hasStyleChanged(actualStyle, nextStyle) ||charsToRender.length===1;
    }
    if (timeToRender) {
        this._renderChar(method, ctx, lineIndex, i, charsToRender, left, top, lineHeight);
        charsToRender="";
        actualStyle=nextStyle;
        left+=boxWidth;
        boxWidth=0;
    }
}
ctx.restore();

}; — You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/fabricjs/fabric.js/issues/2612#issuecomment-481095845, or mute the thread https://github.com/notifications/unsubscribe-auth/AKzweoTCxVkI8EOtxQag3ppzw-9uMuQ-ks5vfBTmgaJpZM4Genqa.

[kimgyucheol]

Sounds interesting Brian, could you possibly share your modifications to _renderChars? Thanks

Absolutely. I just overrode the default implementation via the prototype chain and tried to do as minimal amount of change as possible in order to have it render character by character. Where we determine when it is time to render, I just updated to confirm that we have analyzed as least one character:

timeToRender = this._hasStyleChanged(actualStyle, nextStyle) || charsToRender.length === 1;

The full implementation in Typescript:

/**
 * Internal implementation of _renderChars to improve cross platform/browser rendering for high precision fonts and certain font faces. Here, we render each
 * character individually relying on the start position of Fabric.js to be more accurate than that of the canvas implementation that can vary across platforms.
 * Fabric.js measures font faces at a large scale to avoid precision loss that is seen when measuring at standard scale in the canvas.
 *
 * @param method The rendering method to use.
 * @param ctx The canvas context to render with.
 * @param line The line that needs to be rendered.
 * @param left The starting position.
 * @param top The top position of the line.
 * @param lineIndex The line index in the text element.
 * @private
 */
fabric.Text.prototype._renderChars = function(
    method: string,
    ctx: CanvasRenderingContext2D,
    line: string,
    left: number,
    top: number,
    lineIndex: number,
): void {
    // set proper line offset
    const lineHeight: number = this.getHeightOfLine(lineIndex);
    const isJustify: boolean = this.textAlign.indexOf("justify") !== -1;
    let actualStyle;
    let nextStyle;
    let charsToRender = "";
    let charBox;
    let boxWidth = 0;
    let timeToRender;
    const shortCut: boolean = !isJustify && this.charSpacing === 0 && this.isEmptyStyles(lineIndex);

    ctx.save();
    top -= lineHeight * this._fontSizeFraction / this.lineHeight;
    if (shortCut) {
        // render all the line in one pass without checking
        this._renderChar(method, ctx, lineIndex, 0, this.textLines[lineIndex], left, top, lineHeight);
        ctx.restore();
        return;
    }
    for (let i = 0, len = line.length - 1; i <= len; i++) {
        timeToRender = i === len || this.charSpacing;
        charsToRender += line[i];
        charBox = this.__charBounds[lineIndex][i];
        if (boxWidth === 0) {
            left += charBox.kernedWidth - charBox.width;
            boxWidth += charBox.width;
        } else {
            boxWidth += charBox.kernedWidth;
        }
        if (isJustify && !timeToRender) {
            if (this._reSpaceAndTab.test(line[i])) {
                timeToRender = true;
            }
        }
        if (!timeToRender) {
            // if we have charSpacing, we render char by char
            if (!actualStyle) {
                actualStyle = this.getCompleteStyleDeclaration(lineIndex, i);
            }
            nextStyle = this.getCompleteStyleDeclaration(lineIndex, i + 1);
            timeToRender = this._hasStyleChanged(actualStyle, nextStyle) || charsToRender.length === 1;
        }
        if (timeToRender) {
            this._renderChar(method, ctx, lineIndex, i, charsToRender, left, top, lineHeight);
            charsToRender = "";
            actualStyle = nextStyle;
            left += boxWidth;
            boxWidth = 0;
        }
    }
    ctx.restore();
};

You are my hero!!