🐛 Fix problem with stroke and filter ordering in frames

CHANGES.md

@@ -30,6 +30,7 @@
 - Fix problem with go back button on error page [Taiga #8887](https://tree.taiga.io/project/penpot/issue/8887)
 - Fix problem with shadows in text for Safari [Taiga #8770](https://tree.taiga.io/project/penpot/issue/8770)
 - Fix a regression with feedback form subject and content limits [Taiga #8908](https://tree.taiga.io/project/penpot/issue/8908)
+- Fix problem with stroke and filter ordering in frames [Github #5058](https://github.com/penpot/penpot/issues/5058)
 
 ## 2.2.1
 

frontend/src/app/main/ui/shapes/frame.cljs

@@ -15,6 +15,7 @@
    [app.main.ui.context :as muc]
    [app.main.ui.shapes.attrs :as attrs]
    [app.main.ui.shapes.custom-stroke :refer [shape-fills shape-strokes]]
+   [app.main.ui.shapes.filters :as filters]
    [app.util.debug :as dbg]
    [app.util.object :as obj]
    [rumext.v2 :as mf]))
@@ -65,6 +66,11 @@
 
         render-id     (mf/use-ctx muc/render-id)
 
+        filter-id-blur     (dm/fmt "filter-blur-%" render-id)
+        filter-id-shadows  (dm/fmt "filter-shadow-%" render-id)
+        filter-str-blur    (filters/filter-str filter-id-blur shape)
+        filter-str-shadows (filters/filter-str filter-id-shadows shape)
+
         x             (dm/get-prop shape :x)
         y             (dm/get-prop shape :y)
         w             (dm/get-prop shape :width)
@@ -86,29 +92,37 @@
                                   :className "frame-background"})))
         path?         (some? (.-d props))]
 
-    [:*
-     [:g {:clip-path (when-not ^boolean show-content?
-                       (frame-clip-url shape render-id))
-          ;; A frame sets back normal fill behavior (default
-          ;; transparent). It may have been changed to default black
-          ;; if a shape coming from an imported SVG file is
-          ;; rendered. See main.ui.shapes.attrs/add-style-attrs.
-          :fill "none"
-          :opacity opacity}
+    ;; We need to separate blur from shadows because the blur is applied to the strokes
+    ;; while the shadows have to be placed *under* the stroke (for example, the inner shadows)
+    ;; and the shadows needs to be applied only to the content (without the stroke)
+    [:g {:filter filter-str-blur}
+     [:defs
+      [:& filters/filters {:shape (dissoc shape :blur) :filter-id filter-id-shadows}]
+      [:& filters/filters {:shape (assoc shape :shadow []) :filter-id filter-id-blur}]]
 
-      [:& shape-fills {:shape shape}
-       (if ^boolean path?
-         [:> :path props]
-         [:> :rect props])]
+     ;; This need to be separated in two layers so the clip doesn't affect the shadow filters
+     ;; otherwise the shadow will be clipped and not visible
+     [:g {:filter filter-str-shadows}
+      [:g {:clip-path (when-not ^boolean show-content? (frame-clip-url shape render-id))
+           ;; A frame sets back normal fill behavior (default
+           ;; transparent). It may have been changed to default black
+           ;; if a shape coming from an imported SVG file is
+           ;; rendered. See main.ui.shapes.attrs/add-style-attrs.
+           :fill "none"
+           :opacity opacity}
 
-      children]
+       [:& shape-fills {:shape shape}
+        (if ^boolean path?
+          [:> :path props]
+          [:> :rect props])]
+
+       children]]
 
      [:& shape-strokes {:shape shape}
       (if ^boolean path?
         [:> :path props]
         [:> :rect props])]]))
 
-
 (mf/defc frame-thumbnail-image
   {::mf/wrap-props false}
   [props]

frontend/src/app/main/ui/shapes/shape.cljs

@@ -56,7 +56,6 @@
   (let [shape            (unchecked-get props "shape")
         children         (unchecked-get props "children")
         pointer-events   (unchecked-get props "pointer-events")
-        disable-shadows? (unchecked-get props "disable-shadows?")
         shape-id         (dm/get-prop shape :id)
 
         preview-blend-mode-ref
@@ -67,7 +66,6 @@
 
         type             (dm/get-prop shape :type)
         render-id        (h/use-render-id)
-        filter-id        (dm/str "filter-" render-id)
         styles           (-> (obj/create)
                              (obj/set! "pointerEvents" pointer-events)
                              (cond-> (not (cfh/frame-shape? shape))
@@ -82,18 +80,16 @@
         shape-without-blur (dissoc shape :blur)
         shape-without-shadows (assoc shape :shadow [])
 
+        filter-id        (dm/str "filter-" render-id)
         filter-str
-        (when (and (or (cfh/group-shape? shape)
-                       (cfh/frame-shape? shape)
-                       (cfh/svg-raw-shape? shape))
-                   (not disable-shadows?))
+        (when (or (cfh/group-shape? shape)
+                  (cfh/svg-raw-shape? shape))
           (filters/filter-str filter-id shape))
 
         wrapper-props
         (-> (obj/clone props)
             (obj/unset! "shape")
             (obj/unset! "children")
-            (obj/unset! "disable-shadows?")
             (obj/set! "ref" ref)
             (obj/set! "id" (dm/fmt "shape-%" shape-id))
             (obj/set! "style" styles))
@@ -130,9 +126,14 @@
 
       [:defs
        [:& defs/svg-defs          {:shape shape :render-id render-id}]
-       [:& filters/filters        {:shape shape :filter-id filter-id}]
-       [:& filters/filters        {:shape shape-without-blur :filter-id (dm/fmt "filter-shadow-%" render-id)}]
-       [:& filters/filters        {:shape shape-without-shadows :filter-id (dm/fmt "filter-blur-%" render-id)}]
+
+       ;; The filters for frames should be setup inside the container.
+       (when-not (cfh/frame-shape? shape)
+         [:*
+          [:& filters/filters        {:shape shape :filter-id filter-id}]
+          [:& filters/filters        {:shape shape-without-blur :filter-id (dm/fmt "filter-shadow-%" render-id)}]
+          [:& filters/filters        {:shape shape-without-shadows :filter-id (dm/fmt "filter-blur-%" render-id)}]])
+
        [:& frame/frame-clip-def   {:shape shape :render-id render-id}]
 
        ;; Text fills need to be defined afterwards because they are specified per text-block

frontend/src/app/main/ui/workspace/shapes/frame.cljs

@@ -8,7 +8,6 @@
   (:require
    [app.common.data :as d]
    [app.common.data.macros :as dm]
-   [app.common.files.helpers :as cfh]
    [app.common.geom.shapes.bounds :as gsb]
    [app.common.math :as mth]
    [app.common.thumbnails :as thc]
@@ -45,7 +44,7 @@
                          (refs/children-objects shape-id))
             childs     (mf/deref childs-ref)]
 
-        [:& shape-container {:shape shape :ref ref :disable-shadows? (cfh/is-direct-child-of-root? shape)}
+        [:& shape-container {:shape shape :ref ref}
          [:& frame-shape {:shape shape :childs childs}]
          (when *assert*
            [:& wsd/shape-debug {:shape shape}])]))))
@@ -187,7 +186,7 @@
 
         (fdm/use-dynamic-modifiers objects (mf/ref-val content-ref) modifiers)
 
-        [:& shape-container {:shape shape :disable-shadows? thumbnail?}
+        [:& shape-container {:shape shape}
          [:g.frame-container
           {:id (dm/str "frame-container-" frame-id)
            :key "frame-container"