Merge pull request #7460 from penpot/ladybenko-12205-cap-fills-text

✨ Cap the amount of text fills

frontend/playwright/data/design/get-file-text-fills-limit.json

@@ -0,0 +1,132 @@
+{
+    "~:features": {
+        "~#set": [
+            "fdata/path-data",
+            "plugins/runtime",
+            "design-tokens/v1",
+            "variants/v1",
+            "layout/grid",
+            "styles/v2",
+            "fdata/objects-map",
+            "render-wasm/v1",
+            "components/v2",
+            "fdata/shape-data-type"
+        ]
+    },
+    "~:team-id": "~u3e5ffd68-2819-8084-8006-eb1c616a5afd",
+    "~:permissions": {
+        "~:type": "~:membership",
+        "~:is-owner": true,
+        "~:is-admin": true,
+        "~:can-edit": true,
+        "~:can-read": true,
+        "~:is-logged": true
+    },
+    "~:has-media-trimmed": false,
+    "~:comment-thread-seqn": 0,
+    "~:name": "Text fills",
+    "~:revn": 26,
+    "~:modified-at": "~m1760450987132",
+    "~:vern": 0,
+    "~:id": "~ub1ff3fdf-b491-812b-8006-f2ce3d29333a",
+    "~:is-shared": false,
+    "~:migrations": {
+        "~#ordered-set": [
+            "legacy-2",
+            "legacy-3",
+            "legacy-5",
+            "legacy-6",
+            "legacy-7",
+            "legacy-8",
+            "legacy-9",
+            "legacy-10",
+            "legacy-11",
+            "legacy-12",
+            "legacy-13",
+            "legacy-14",
+            "legacy-16",
+            "legacy-17",
+            "legacy-18",
+            "legacy-19",
+            "legacy-25",
+            "legacy-26",
+            "legacy-27",
+            "legacy-28",
+            "legacy-29",
+            "legacy-31",
+            "legacy-32",
+            "legacy-33",
+            "legacy-34",
+            "legacy-36",
+            "legacy-37",
+            "legacy-38",
+            "legacy-39",
+            "legacy-40",
+            "legacy-41",
+            "legacy-42",
+            "legacy-43",
+            "legacy-44",
+            "legacy-45",
+            "legacy-46",
+            "legacy-47",
+            "legacy-48",
+            "legacy-49",
+            "legacy-50",
+            "legacy-51",
+            "legacy-52",
+            "legacy-53",
+            "legacy-54",
+            "legacy-55",
+            "legacy-56",
+            "legacy-57",
+            "legacy-59",
+            "legacy-62",
+            "legacy-65",
+            "legacy-66",
+            "legacy-67",
+            "0001-remove-tokens-from-groups",
+            "0002-normalize-bool-content-v2",
+            "0002-clean-shape-interactions",
+            "0003-fix-root-shape",
+            "0003-convert-path-content-v2",
+            "0004-clean-shadow-color",
+            "0005-deprecate-image-type",
+            "0006-fix-old-texts-fills",
+            "0007-clear-invalid-strokes-and-fills-v2",
+            "0008-fix-library-colors-v4",
+            "0009-clean-library-colors",
+            "0009-add-partial-text-touched-flags",
+            "0010-fix-swap-slots-pointing-non-existent-shapes",
+            "0011-fix-invalid-text-touched-flags",
+            "0012-fix-position-data",
+            "0013-fix-component-path",
+            "0014-fix-tokens-lib-duplicate-ids"
+        ]
+    },
+    "~:version": 67,
+    "~:project-id": "~u3e5ffd68-2819-8084-8006-eb1c616e69bf",
+    "~:created-at": "~m1760368824484",
+    "~:backend": "legacy-db",
+    "~:data": {
+        "~:pages": [
+            "~ub1ff3fdf-b491-812b-8006-f2ce3d29333b"
+        ],
+        "~:pages-index": {
+            "~ub1ff3fdf-b491-812b-8006-f2ce3d29333b": {
+                "~:objects": {
+                    "~#penpot/objects-map/v2": {
+                        "~u00000000-0000-0000-0000-000000000000": "[\"~#shape\",[\"^ \",\"~:y\",0,\"~:hide-fill-on-export\",false,\"~:transform\",[\"~#matrix\",[\"^ \",\"~:a\",1.0,\"~:b\",0.0,\"~:c\",0.0,\"~:d\",1.0,\"~:e\",0.0,\"~:f\",0.0]],\"~:rotation\",0,\"~:name\",\"Root Frame\",\"~:width\",0.01,\"~:type\",\"~:frame\",\"~:points\",[[\"~#point\",[\"^ \",\"~:x\",0.0,\"~:y\",0.0]],[\"^:\",[\"^ \",\"~:x\",0.01,\"~:y\",0.0]],[\"^:\",[\"^ \",\"~:x\",0.01,\"~:y\",0.01]],[\"^:\",[\"^ \",\"~:x\",0.0,\"~:y\",0.01]]],\"~:r2\",0,\"~:proportion-lock\",false,\"~:transform-inverse\",[\"^3\",[\"^ \",\"~:a\",1.0,\"~:b\",0.0,\"~:c\",0.0,\"~:d\",1.0,\"~:e\",0.0,\"~:f\",0.0]],\"~:r3\",0,\"~:r1\",0,\"~:id\",\"~u00000000-0000-0000-0000-000000000000\",\"~:parent-id\",\"~u00000000-0000-0000-0000-000000000000\",\"~:frame-id\",\"~u00000000-0000-0000-0000-000000000000\",\"~:strokes\",[],\"~:x\",0,\"~:proportion\",1.0,\"~:r4\",0,\"~:selrect\",[\"~#rect\",[\"^ \",\"~:x\",0,\"~:y\",0,\"^6\",0.01,\"~:height\",0.01,\"~:x1\",0,\"~:y1\",0,\"~:x2\",0.01,\"~:y2\",0.01]],\"~:fills\",[[\"^ \",\"~:fill-color\",\"#FFFFFF\",\"~:fill-opacity\",1]],\"~:flip-x\",null,\"^H\",0.01,\"~:flip-y\",null,\"~:shapes\",[\"~u23b2a9cc-5d10-800a-8006-f2ce6354e75d\"]]]",
+                        "~u23b2a9cc-5d10-800a-8006-f2ce6354e75d": "[\"~#shape\",[\"^ \",\"~:y\",540.9999842227844,\"~:transform\",[\"~#matrix\",[\"^ \",\"~:a\",1.0,\"~:b\",0.0,\"~:c\",0.0,\"~:d\",1.0,\"~:e\",0.0,\"~:f\",0.0]],\"~:rotation\",0,\"~:grow-type\",\"~:auto-width\",\"~:content\",[\"^ \",\"~:type\",\"root\",\"~:key\",\"1tx250q8puw\",\"~:children\",[[\"^ \",\"^7\",\"paragraph-set\",\"^9\",[[\"^ \",\"~:line-height\",\"1.2\",\"~:font-style\",\"normal\",\"^9\",[[\"^ \",\"^:\",\"\",\"^;\",\"normal\",\"~:typography-ref-id\",null,\"~:text-transform\",\"none\",\"~:font-id\",\"sourcesanspro\",\"^8\",\"1c1skqx1y2p\",\"~:font-size\",\"36\",\"~:font-weight\",\"700\",\"~:typography-ref-file\",null,\"~:font-variant-id\",\"bold\",\"~:text-decoration\",\"none\",\"~:letter-spacing\",\"0\",\"~:fills\",[[\"^ \",\"~:fill-color\",\"#000000\",\"~:fill-opacity\",1],[\"^ \",\"^F\",\"#00ffc3\",\"^G\",1],[\"^ \",\"^F\",\"#d9ff00\",\"^G\",1],[\"^ \",\"^F\",\"#192b60\",\"^G\",1],[\"^ \",\"^F\",\"#7f9fff\",\"^G\",1],[\"^ \",\"^F\",\"#ff00ca\",\"^G\",1],[\"^ \",\"^F\",\"#003fff\",\"^G\",1]],\"~:font-family\",\"sourcesanspro\",\"~:text\",\"Lorem ipsum\"]],\"^<\",null,\"^=\",\"none\",\"~:text-align\",\"left\",\"^>\",\"sourcesanspro\",\"^8\",\"12a6vwimj9j\",\"^?\",\"0\",\"^@\",\"700\",\"^A\",null,\"~:text-direction\",\"ltr\",\"^7\",\"paragraph\",\"^B\",\"bold\",\"^C\",\"none\",\"^D\",\"0\",\"^E\",[[\"^ \",\"^F\",\"#000000\",\"^G\",1],[\"^ \",\"^F\",\"#00ffc3\",\"^G\",1],[\"^ \",\"^F\",\"#d9ff00\",\"^G\",1],[\"^ \",\"^F\",\"#192b60\",\"^G\",1],[\"^ \",\"^F\",\"#7f9fff\",\"^G\",1],[\"^ \",\"^F\",\"#ff00ca\",\"^G\",1],[\"^ \",\"^F\",\"#003fff\",\"^G\",1]],\"^H\",\"sourcesanspro\"]]]],\"~:vertical-align\",\"top\"],\"~:hide-in-viewer\",false,\"~:name\",\"Lorem ipsum\",\"~:width\",207.000004835394,\"^7\",\"^I\",\"~:points\",[[\"~#point\",[\"^ \",\"~:x\",321.9999624852649,\"~:y\",540.9999842227845]],[\"^Q\",[\"^ \",\"~:x\",528.9999673206589,\"~:y\",540.9999842227845]],[\"^Q\",[\"^ \",\"~:x\",528.9999673206589,\"~:y\",583.9999821366218]],[\"^Q\",[\"^ \",\"~:x\",321.9999624852649,\"~:y\",583.9999821366218]]],\"~:transform-inverse\",[\"^2\",[\"^ \",\"~:a\",1.0,\"~:b\",0.0,\"~:c\",0.0,\"~:d\",1.0,\"~:e\",0.0,\"~:f\",0.0]],\"~:id\",\"~u23b2a9cc-5d10-800a-8006-f2ce6354e75d\",\"~:parent-id\",\"~u00000000-0000-0000-0000-000000000000\",\"~:frame-id\",\"~u00000000-0000-0000-0000-000000000000\",\"~:x\",321.99996248526486,\"~:selrect\",[\"~#rect\",[\"^ \",\"~:x\",321.99996248526486,\"~:y\",540.9999842227844,\"^O\",207.000004835394,\"~:height\",42.99999791383732,\"~:x1\",321.99996248526486,\"~:y1\",540.9999842227844,\"~:x2\",528.9999673206589,\"~:y2\",583.9999821366217]],\"~:flip-x\",null,\"^X\",42.99999791383732,\"~:flip-y\",null]]"
+                    }
+                },
+                "~:id": "~ub1ff3fdf-b491-812b-8006-f2ce3d29333b",
+                "~:name": "Page 1"
+            }
+        },
+        "~:id": "~ub1ff3fdf-b491-812b-8006-f2ce3d29333a",
+        "~:options": {
+            "~:components-v2": true,
+            "~:base-font-size": "16px"
+        }
+    }
+}

frontend/playwright/ui/specs/design-tab.spec.js

@@ -89,6 +89,38 @@ test.describe("Shape attributes", () => {
 
     await expect(workspace.page.getByTestId("add-fill")).toBeDisabled();
   });
+
+  test("Cannot add a new text fill when the limit has been reached", async ({
+    page,
+  }) => {
+    const workspace = new WorkspacePage(page);
+    await workspace.mockConfigFlags(["enable-feature-render-wasm"]);
+    await workspace.setupEmptyFile();
+    await workspace.mockRPC(
+      /get\-file\?/,
+      "design/get-file-text-fills-limit.json",
+    );
+
+    await workspace.goToWorkspace({
+      fileId: "b1ff3fdf-b491-812b-8006-f2ce3d29333a",
+      pageId: "b1ff3fdf-b491-812b-8006-f2ce3d29333b",
+    });
+
+    await workspace.clickLeafLayer("Lorem ipsum");
+
+    await expect(
+      workspace.page.getByRole("button", { name: "Remove color" }),
+    ).toHaveCount(7);
+
+    await workspace.page.getByRole("button", { name: "Add fill" }).click();
+    await expect(
+      workspace.page.getByRole("button", { name: "Remove color" }),
+    ).toHaveCount(8);
+
+    await expect(
+      workspace.page.getByRole("button", { name: "Add fill" }),
+    ).toBeDisabled();
+  });
 });
 
 test.describe("Multiple shapes attributes", () => {

frontend/src/app/render_wasm/api/texts.cljs

@@ -17,6 +17,7 @@
 
 (def ^:const PARAGRAPH-ATTR-U8-SIZE 44)
 (def ^:const LEAF-ATTR-U8-SIZE 60)
+(def ^:const MAX-TEXT-FILLS types.fills.impl/MAX-FILLS)
 
 (defn- encode-text
   "Into an UTF8 buffer. Returns an ArrayBuffer instance"
@@ -26,28 +27,27 @@
 
 (defn- write-leaf-fills
   [offset dview fills]
-  (reduce (fn [offset fill]
-            (let [opacity  (get fill :fill-opacity 1.0)
-                  color    (get fill :fill-color)
-                  gradient (get fill :fill-color-gradient)
-                  image    (get fill :fill-image)]
+  (let [new-ofset (reduce (fn [offset fill]
+                            (let [opacity  (get fill :fill-opacity 1.0)
+                                  color    (get fill :fill-color)
+                                  gradient (get fill :fill-color-gradient)
+                                  image    (get fill :fill-image)]
 
-              (cond
-                (some? color)
-                (types.fills.impl/write-solid-fill offset dview opacity color)
+                              (cond
+                                (some? color)
+                                (types.fills.impl/write-solid-fill offset dview opacity color)
 
-                (some? gradient)
-                (types.fills.impl/write-gradient-fill offset dview opacity gradient)
+                                (some? gradient)
+                                (types.fills.impl/write-gradient-fill offset dview opacity gradient)
 
-                (some? image)
-                (types.fills.impl/write-image-fill offset dview opacity image))))
+                                (some? image)
+                                (types.fills.impl/write-image-fill offset dview opacity image))))
 
-          offset
-          fills))
+                          offset
+                          fills)
+        padding-fills (max 0 (- MAX-TEXT-FILLS (count fills)))]
+    (+ new-ofset (* padding-fills types.fills.impl/FILL-U8-SIZE))))
 
-(defn- get-total-fills
-  [leaves]
-  (reduce #(+ %1 (count (:fills %2))) 0 leaves))
 
 (defn- write-paragraph
   [offset dview paragraph]
@@ -86,8 +86,7 @@
 
                   text-buffer (encode-text (get leaf :text))
                   text-length (mem/size text-buffer)
-                  fills       (get leaf :fills)
-                  total-fills (count fills)
+                  fills       (take MAX-TEXT-FILLS (get leaf :fills))
 
                   font-variant-id
                   (get leaf :font-variant-id)
@@ -127,7 +126,7 @@
                   (mem/write-uuid dview (d/nilv font-variant-id uuid/zero))
 
                   (mem/write-i32 dview text-length)
-                  (mem/write-i32 dview total-fills)
+                  (mem/write-i32 dview (count fills))
                   (mem/assert-written offset LEAF-ATTR-U8-SIZE)
 
                   (write-leaf-fills dview fills))))
@@ -139,11 +138,9 @@
   ;; [<num-leaves> <paragraph_attributes> <leaves_attributes> <text>]
   [leaves paragraph text]
   (let [num-leaves    (count leaves)
-        fills-size    (* types.fills.impl/FILL-U8-SIZE
-                         (get-total-fills leaves))
+        fills-size    (* types.fills.impl/FILL-U8-SIZE MAX-TEXT-FILLS)
         metadata-size (+ PARAGRAPH-ATTR-U8-SIZE
-                         (* num-leaves LEAF-ATTR-U8-SIZE)
-                         fills-size)
+                         (* num-leaves (+ LEAF-ATTR-U8-SIZE fills-size)))
 
         text-buffer   (encode-text text)
         text-size     (mem/size text-buffer)

render-wasm/src/wasm/text.rs

@@ -1,6 +1,6 @@
 use macros::ToJs;
 
-use super::fonts::RawFontStyle;
+use super::{fills::RawFillData, fonts::RawFontStyle};
 use crate::math::{Matrix, Point};
 use crate::mem;
 use crate::shapes::{
@@ -9,10 +9,11 @@ use crate::shapes::{
 use crate::utils::{uuid_from_u32, uuid_from_u32_quartet};
 use crate::{with_current_shape, with_current_shape_mut, with_state_mut, STATE};
 
-const RAW_LEAF_DATA_SIZE: usize = std::mem::size_of::<RawTextLeafAttrs>();
-pub const RAW_LEAF_FILLS_SIZE: usize = 160;
+const RAW_LEAF_DATA_SIZE: usize = std::mem::size_of::<RawTextLeaf>();
 const RAW_PARAGRAPH_DATA_SIZE: usize = std::mem::size_of::<RawParagraphData>();
 
+const MAX_TEXT_FILLS: usize = 8;
+
 #[derive(Debug, PartialEq, Clone, Copy, ToJs)]
 #[repr(u8)]
 pub enum RawTextAlign {
@@ -121,10 +122,9 @@ impl TryFrom<&[u8]> for RawParagraphData {
     }
 }
 
-// FIXME: Merge this struct with RawTextLeaf once we cap the amount of fills a text shape has
 #[repr(C)]
-#[derive(Debug, Clone, Copy)]
-pub struct RawTextLeafAttrs {
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub struct RawTextLeaf {
     font_style: RawFontStyle,
     text_decoration: RawTextDecoration,
     text_transform: RawTextTransform,
@@ -136,55 +136,24 @@ pub struct RawTextLeafAttrs {
     font_family: [u8; 4],
     font_variant_id: [u32; 4], // TODO: maybe add RawUUID type
     text_length: u32,
-    fill_count: u32, // FIXME: we should cap the amount of fills a text shape has
+    fill_count: u32,
+    fills: [RawFillData; MAX_TEXT_FILLS],
 }
 
-impl From<[u8; RAW_LEAF_DATA_SIZE]> for RawTextLeafAttrs {
+impl From<[u8; RAW_LEAF_DATA_SIZE]> for RawTextLeaf {
     fn from(bytes: [u8; RAW_LEAF_DATA_SIZE]) -> Self {
         unsafe { std::mem::transmute(bytes) }
     }
 }
 
-impl TryFrom<&[u8]> for RawTextLeafAttrs {
+impl TryFrom<&[u8]> for RawTextLeaf {
     type Error = String;
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         let data: [u8; RAW_LEAF_DATA_SIZE] = bytes
             .get(0..RAW_LEAF_DATA_SIZE)
             .and_then(|slice| slice.try_into().ok())
             .ok_or("Invalid text leaf data".to_string())?;
-        Ok(RawTextLeafAttrs::from(data))
-    }
-}
-
-#[allow(dead_code)]
-#[repr(C)]
-#[derive(Debug, Clone)]
-pub struct RawTextLeaf {
-    attrs: RawTextLeafAttrs,
-    raw_fills: Vec<u8>, // FIXME: remove this once we cap the amount of fills a text shape has
-}
-
-impl TryFrom<&[u8]> for RawTextLeaf {
-    // TODO: use a proper error type
-    type Error = String;
-
-    fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
-        let raw_attrs: RawTextLeafAttrs = RawTextLeafAttrs::try_from(bytes)?;
-        let total_fills = raw_attrs.fill_count as usize;
-
-        // Use checked_mul to prevent overflow
-        let fills_size = total_fills
-            .checked_mul(RAW_LEAF_FILLS_SIZE)
-            .ok_or("Overflow occurred while calculating fills size")?;
-
-        let fills_start = RAW_LEAF_DATA_SIZE;
-        let fills_end = fills_start + fills_size;
-        let raw_fills = &bytes[fills_start..fills_end];
-
-        Ok(Self {
-            attrs: raw_attrs,
-            raw_fills: raw_fills.to_vec(),
-        })
+        Ok(RawTextLeaf::from(data))
     }
 }
 
@@ -193,23 +162,28 @@ impl From<RawTextLeaf> for shapes::TextLeaf {
         let text = String::default();
 
         let font_family = shapes::FontFamily::new(
-            uuid_from_u32(value.attrs.font_id),
-            value.attrs.font_weight as u32,
-            value.attrs.font_style.into(),
+            uuid_from_u32(value.font_id),
+            value.font_weight as u32,
+            value.font_style.into(),
         );
-        let fills =
-            super::fills::parse_fills_from_bytes(&value.raw_fills, value.attrs.fill_count as usize);
+
+        let fills = value
+            .fills
+            .into_iter()
+            .take(value.fill_count as usize)
+            .map(|fill| fill.into())
+            .collect();
 
         Self::new(
             text,
             font_family,
-            value.attrs.font_size,
-            value.attrs.letter_spacing,
-            value.attrs.text_decoration.into(),
-            value.attrs.text_transform.into(),
-            value.attrs.text_direction.into(),
-            value.attrs.font_weight,
-            uuid_from_u32(value.attrs.font_variant_id),
+            value.font_size,
+            value.letter_spacing,
+            value.text_decoration.into(),
+            value.text_transform.into(),
+            value.text_direction.into(),
+            value.font_weight,
+            uuid_from_u32(value.font_variant_id),
             fills,
         )
     }
@@ -233,11 +207,8 @@ impl TryFrom<&Vec<u8>> for RawParagraph {
         let mut raw_text_leaves: Vec<RawTextLeaf> = Vec::new();
 
         for _ in 0..attrs.leaf_count {
-            let text_leaf = RawTextLeaf::try_from(&bytes[offset..])?;
-            let leaf_size =
-                RAW_LEAF_DATA_SIZE + (text_leaf.attrs.fill_count as usize * RAW_LEAF_FILLS_SIZE);
-
-            offset += leaf_size;
+            let text_leaf = RawTextLeaf::try_from(&bytes[offset..(offset + RAW_LEAF_DATA_SIZE)])?;
+            offset += RAW_LEAF_DATA_SIZE;
             raw_text_leaves.push(text_leaf);
         }
 
@@ -260,7 +231,7 @@ impl From<RawParagraph> for shapes::Paragraph {
 
         let mut offset = 0;
         for raw_leaf in value.leaves.into_iter() {
-            let delta = raw_leaf.attrs.text_length as usize;
+            let delta = raw_leaf.text_length as usize;
             let text_buffer = &value.text_buffer[offset..offset + delta];
 
             let mut leaf = shapes::TextLeaf::from(raw_leaf);