Fix TypeError: fill.render is not a function

[chr314]

check if fill.render is function

fixing #7171 (comment)

[asturur]

@ShaMan123 does this seems a legit fix for you?

[ShaMan123]

See this Originally posted by @ShaMan123 in #7220 (comment)

NO

Yes and no.
Yes it will fix the error but I am not sure why it is happening.
The logic assumes that the background/overlay color has been replaced by a Rect so it could be erased by the eraser.
That is where the true bug originates from.
I think I can reproduce now that I understand.

When I built the eraser mixin this specific logic was handled by the eraser but later on I moved it to Canvas.
This logic was correct when the eraser brush was in charge of replacing bg/overlay color to rects if they were erasable because it checked it every render.
Now it fails because it has no replacement mechanism.
I moved it because it seemed too expensive to run every mouse move and I stick to it.

If I am not mistaken this is where the bug comes from:

fabric.js/src/mixins/eraser_brush.mixin.js

Line 77 in e6d16d3

___setBgOverlay.call(this, property, value, loaded, callback);

fabric.js/src/mixins/eraser_brush.mixin.js

Line 95 in e6d16d3

__setSVGBgOverlayColor.call(this, markup, property, reviver);

So if my assumptions are correct the true fix would be to change the else blocks and add rects instead of filters/string values.
In addition, this proposed fix will only remove the error but it will not render the actual bg/overlay.

Therefore I think this is not a fix after all.

There is another bug regarding this block of code that I pushed into #7100 (which is waiting for your feedback):
The bg/overlay color should not be affected by canvas transform.

[ShaMan123]

@chr314 Could you help me reproduce?
Did you load the canvas from JSON/SVG before this error happened?

[ShaMan123]

I can't reproduce.
@chr314 are you setting bg/overlay color directly (assigning) or using the setBackgroundColor method as STATED in the instructions of the demo??

look at these 2 repos:
https://codesandbox.io/s/d16uq
https://codesandbox.io/s/dl79j

Read their comments to understand what they check

[ShaMan123]

Yes and no.
Yes it will fix the error but I am not sure why it is happening.
The logic assumes that the background/overlay color has been replaced by a Rect so it could be erased by the eraser.
That is where the true bug originates from.
I think I can reproduce now that I understand.

When I built the eraser mixin this specific logic was handled by the eraser but later on I moved it to Canvas.
This logic was correct when the eraser brush was in charge of replacing bg/overlay color to rects if they were erasable because it checked it every render.
Now it fails because it has no replacement mechanism.
I moved it because it seemed too expensive to run every mouse move and I stick to it.

If I am not mistaken this is where the bug comes from:

fabric.js/src/mixins/eraser_brush.mixin.js

Line 77 in e6d16d3

___setBgOverlay.call(this, property, value, loaded, callback);

fabric.js/src/mixins/eraser_brush.mixin.js

Line 95 in e6d16d3

__setSVGBgOverlayColor.call(this, markup, property, reviver);

So if my assumptions are correct the true fix would be to change the else blocks and add rects instead of filters/string values.
In addition, this proposed fix will only remove the error but it will not render the actual bg/overlay.

Therefore I think this is not a fix after all.

There is another bug regarding this block of code that I pushed into #7100 (which is waiting for your feedback):
The bg/overlay color should not be affected by canvas transform.

After looking closely at serialization I can say confindently that the eraser mixin works as expected with no problems since serialization calls setBackgroundColor or setOverlayColor that the eraser mixin extends.

fabric.js/src/mixins/canvas_serialization.mixin.js

Line 142 in 2eabc92

this['set' + fabric.util.string.capitalize(property, true)](value, function() {

[ShaMan123]

What I would do is add a warning here (or somewhere in the eraser, say at the beginning of the render cycle) instead explaining how to set bg/overlay color when using eraser and referencing the docs if bg/overlay color are not rects

[chr314]

@ShaMan123 thanks
yeah I think I was using the bg color the wrong way

    //  this is WRONG!!
    canvas.backgroundColor = 'yellow';
    //  this is CORRECT!!
    canvas.setBackgroundColor('yellow');