docs() perf(): Reorder caching conditions for most common scenario an…

…d docs fixes. (#10366)

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [next]
 
+- docs() perf(): Reorder caching conditions for most common scenario and docs fixes. [#10366](https://github.com/fabricjs/fabric.js/pull/10366)
+
 ## [6.5.3]
 
 - fix(ColorMatrix): Restore correct alpha for JS colorMatrix filter [#10313](https://github.com/fabricjs/fabric.js/pull/10313)

src/shapes/ActiveSelection.ts

@@ -209,11 +209,7 @@ export class ActiveSelection extends Group {
   }
 
   /**
-   * Decide if the object should cache or not. Create its own cache level
-   * objectCaching is a global flag, wins over everything
-   * needsItsOwnCache should be used when the object drawing method requires
-   * a cache step. None of the fabric classes requires it.
-   * Generally you do not cache objects in groups because the group outside is cached.
+   * Decide if the object should cache or not. The Active selection never caches
    * @return {Boolean}
    */
   shouldCache() {

src/shapes/Group.ts

@@ -446,9 +446,9 @@ export class Group
   }
 
   /**
-   * Decide if the object should cache or not. Create its own cache level
+   * Decide if the group should cache or not. Create its own cache level
    * needsItsOwnCache should be used when the object drawing method requires
-   * a cache step. None of the fabric classes requires it.
+   * a cache step.
    * Generally you do not cache objects in groups because the group is already cached.
    * @return {Boolean}
    */

src/shapes/Image.ts

@@ -609,11 +609,11 @@ export class FabricImage<
   }
 
   /**
-   * Decide if the object should cache or not. Create its own cache level
+   * Decide if the FabricImage should cache or not. Create its own cache level
    * needsItsOwnCache should be used when the object drawing method requires
-   * a cache step. None of the fabric classes requires it.
+   * a cache step.
    * Generally you do not cache objects in groups because the group outside is cached.
-   * This is the special image version where we would like to avoid caching where possible.
+   * This is the special Image version where we would like to avoid caching where possible.
    * Essentially images do not benefit from caching. They may require caching, and in that
    * case we do it. Also caching an image usually ends in a loss of details.
    * A full performance audit should be done.

src/shapes/Object/Object.ts

@@ -752,14 +752,15 @@ export class FabricObject<
   }
 
   /**
-   * When set to `true`, force the object to have its own cache, even if it is inside a group
+   * When returns `true`, force the object to have its own cache, even if it is inside a group
    * it may be needed when your object behave in a particular way on the cache and always needs
    * its own isolated canvas to render correctly.
    * Created to be overridden
    * since 1.7.12
    * @returns Boolean
    */
   needsItsOwnCache() {
+    // TODO re-evaluate this shadow condition
     if (
       this.paintFirst === STROKE &&
       this.hasFill() &&
@@ -778,15 +779,15 @@ export class FabricObject<
    * Decide if the object should cache or not. Create its own cache level
    * objectCaching is a global flag, wins over everything
    * needsItsOwnCache should be used when the object drawing method requires
-   * a cache step. None of the fabric classes requires it.
+   * a cache step.
    * Generally you do not cache objects in groups because the group outside is cached.
    * Read as: cache if is needed, or if the feature is enabled but we are not already caching.
    * @return {Boolean}
    */
   shouldCache() {
     this.ownCaching =
-      this.needsItsOwnCache() ||
-      (this.objectCaching && (!this.parent || !this.parent.isOnACache()));
+      (this.objectCaching && (!this.parent || !this.parent.isOnACache())) ||
+      this.needsItsOwnCache();
     return this.ownCaching;
   }
 
@@ -912,7 +913,10 @@ export class FabricObject<
   }
 
   /**
-   * Check if cache is dirty
+   * Check if cache is dirty and if is dirty clear the context.
+   * This check has a big side effect, it changes the underlying cache canvas if necessary.
+   * Do not call this method on your own to check if the cache is dirty, because if it is,
+   * it is also going to wipe the cache. This is badly designed and needs to be fixed.
    * @param {Boolean} skipCanvas skip canvas checks because this object is painted
    * on parent canvas.
    */

src/shapes/Object/ObjectGeometry.ts

@@ -611,6 +611,7 @@ export class ObjectGeometry<EventSpec extends ObjectEvents = ObjectEvents>
       width: this.width,
       height: this.height,
       strokeWidth: this.strokeWidth,
+      // TODO remove this spread. is visible in the performance inspection
       ...options,
     };
     // stroke is applied before/after transformations are applied according to `strokeUniform`