[Bug]: blurred text in zoom if use paintFirst: "stroke" about fabric.js

[asturur]

Hi @Jonnas123 what do you mean by zoom the text?

[Jonnas123]

I mean use mouse wheel to zoom in.

[asturur]

Yes there is a simple bug.

    if (
      this.paintFirst === 'stroke' &&
      this.hasFill() &&
      this.hasStroke() &&
      typeof this.shadow === 'object'
    ) {
      return true;
    }

we are running this condition to determine if the object needs the cache forced on.
Somehow since some change, this.shadow is an object even if we are not setting it.

This is probably an easy fix ( i didn't look at it ).

If you look at it is also possible you can open a PR to fix it.

[asturur]

https://github.com/fabricjs/fabric.js/blob/master/src/shapes/Object/defaultValues.ts#L79

Here the shadow is set as 'null' as a default.

I sincerly do not remember why we have the shadow property typed as Shadow | null, it could just be ?: Shadow and then fix the code to actually work with this change.

A simpler and more direct fix is to update that condition to say:

this.paintFirst === 'stroke' &&
      this.hasFill() &&
      this.hasStroke() &&
      this.shadow !== null 

Probably less involved

Would require adding a jest test anyway in both cases

[Jonnas123]

Could you give a hint on where to place the codes? I 've tied both codes inside and outside the zoom function but not working.

[ShaMan123]

start using codesandbox for repro
https://codesandbox.io/p/sandbox/fabric-vanillajs-sandbox-forked-y3kryw?file=%2Fsrc%2Findex.ts%3A14%2C24

[asturur]

Could you give a hint on where to place the codes? I 've tied both codes inside and outside the zoom function but not working.

if you do a full search for this.paintFirst === 'stroke' you shouldn't get more than one or two results.
One of those results is 'needsItsOwnCache' that is the one you need to fix.

Even better, if you just look for typeof this.shadow === 'object' you find the exact bug and code.

[gloriousjob]

@asturur Do you have a recommendation for testing? This looks like a visual test (i.e., pixelmatch to see if the blur occurs) but I'm not sure that there's an example for jest. Is something like this needed?
https://github.com/americanexpress/jest-image-snapshot

Or did you have a simpler idea?

[ShaMan123]

We use playwright for that.
Look at https://github.com/fabricjs/fabric.js/tree/master/e2e/tests/template-with-node

[asturur]

I don't think a visual test is a good idea.
For a visual test is hard to determine if something is blurry or not.
We just need to test the output of 'needsItsOwnCache' function for a text object with different combination of properties.
The bug is not being blurry, that is not a bug itself, the bug is that it force cache on, against your request without a valid reason.