fix(): isNotVisible + rename to isVisible

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [next]
 
+- fix(): `isNotVisible` + rename to `isVisible` [#9574](https://github.com/fabricjs/fabric.js/pull/9574)
 - fix() Addressing path cloning slowness ( partially ) [#9573](https://github.com/fabricjs/fabric.js/pull/9573)
 - feat(): Add save/restore ability to group LayoutManager [#9564](https://github.com/fabricjs/fabric.js/pull/9564)
 - fix() Remove unwanted set type warning [#9569](https://github.com/fabricjs/fabric.js/pull/9569)

src/Collection.ts

@@ -336,7 +336,7 @@ export function createCollectionMixin<TBase extends Constructor>(Base: TBase) {
         const object = this._objects[i] as unknown as InteractiveFabricObject;
         if (
           object.selectable &&
-          object.visible &&
+          object.isVisible() &&
           ((includeIntersecting && object.intersectsWithRect(tl, br)) ||
             object.isContainedWithinRect(tl, br) ||
             (includeIntersecting && object.containsPoint(tl)) ||

src/canvas/SelectableCanvas.ts

@@ -806,7 +806,7 @@ export class SelectableCanvas<EventSpec extends CanvasEvents = CanvasEvents>
   _checkTarget(obj: FabricObject, pointer: Point): boolean {
     if (
       obj &&
-      obj.visible &&
+      obj.isVisible() &&
       obj.evented &&
       this._pointIsInObjectSelectionArea(
         obj,

src/shapes/Object/Object.ts

@@ -739,11 +739,9 @@ export class FabricObject<
    * @memberOf FabricObject.prototype
    * @return {Boolean}
    */
-  isNotVisible() {
+  isVisible() {
     return (
-      this.opacity === 0 ||
-      (!this.width && !this.height && this.strokeWidth === 0) ||
-      !this.visible
+      this.visible && this.opacity > 0 && this.width > 0 && this.height > 0
     );
   }
 
@@ -753,7 +751,7 @@ export class FabricObject<
    */
   render(ctx: CanvasRenderingContext2D) {
     // do not render if width/height are zeros or object is not visible
-    if (this.isNotVisible()) {
+    if (!this.isVisible()) {
       return;
     }
     if (
@@ -978,7 +976,7 @@ export class FabricObject<
    * on parent canvas.
    */
   isCacheDirty(skipCanvas = false) {
-    if (this.isNotVisible()) {
+    if (!this.isVisible()) {
       return false;
     }
     if (

src/shapes/Text/Text.ts

@@ -599,7 +599,7 @@ export class FabricText<
    */
   _render(ctx: CanvasRenderingContext2D) {
     const path = this.path;
-    path && !path.isNotVisible() && path._render(ctx);
+    path && path.isVisible() && path._render(ctx);
     this._setTextStyles(ctx);
     this._renderTextLinesBackground(ctx);
     this._renderTextDecoration(ctx, 'underline');
@@ -1679,7 +1679,7 @@ export class FabricText<
    * @param {CanvasRenderingContext2D} ctx Context to render on
    */
   render(ctx: CanvasRenderingContext2D) {
-    if (!this.visible) {
+    if (!this.isVisible()) {
       return;
     }
     if (

test/unit/ellipse.js

@@ -95,16 +95,16 @@
     });
   });
 
-  QUnit.test('isNotVisible', function(assert) {
+  QUnit.test('isVisible', function(assert) {
     var ellipse = new fabric.Ellipse();
     ellipse.set('rx', 0);
     ellipse.set('ry', 0);
 
-    assert.equal(ellipse.isNotVisible(), false, 'isNotVisible false when rx/ry are 0 because strokeWidth is > 0');
+    assert.equal(ellipse.isVisible(), true, 'isVisible false when rx/ry are 0 because strokeWidth is > 0');
 
     ellipse.set('strokeWidth', 0);
 
-    assert.equal(ellipse.isNotVisible(), true, 'should not render anymore with also strokeWidth 0');
+    assert.equal(ellipse.isVisible(), false, 'should not render anymore with also strokeWidth 0');
 
   });
 

test/unit/object.js

@@ -1210,17 +1210,17 @@
     object._set('fill', 'blue');
     assert.equal(object.dirty, false, 'dirty is not raised');
   });
-  QUnit.test('isNotVisible', function(assert) {
+  QUnit.test('isVisible', function(assert) {
     var object = new fabric.Object({ fill: 'blue', width: 100, height: 100 });
-    assert.equal(object.isNotVisible(), false, 'object is default visible');
+    assert.equal(object.isVisible(), true, 'object is default visible');
     object = new fabric.Object({ fill: 'blue', width: 0, height: 0, strokeWidth: 1 });
-    assert.equal(object.isNotVisible(), false, 'object is visible with width and height equal 0, but strokeWidth 1');
+    assert.equal(object.isVisible(), false, 'object is visible with width and height equal 0, but strokeWidth 1');
     object = new fabric.Object({ opacity: 0, fill: 'blue' });
-    assert.equal(object.isNotVisible(), true, 'object is not visible with opacity 0');
+    assert.equal(object.isVisible(), false, 'object is not visible with opacity 0');
     object = new fabric.Object({ fill: 'blue', visible: false });
-    assert.equal(object.isNotVisible(), true, 'object is not visible with visible false');
+    assert.equal(object.isVisible(), false, 'object is not visible with visible false');
     object = new fabric.Object({ fill: 'blue', width: 0, height: 0, strokeWidth: 0 });
-    assert.equal(object.isNotVisible(), true, 'object is not visible with also strokeWidth equal 0');
+    assert.equal(object.isVisible(), false, 'object is not visible with also strokeWidth equal 0');
   });
   QUnit.test('shouldCache', function(assert) {
     var object = new fabric.Object();