diff --git a/docs/animation-model.md b/docs/animation-model.md index 77b3478..647f798 100644 --- a/docs/animation-model.md +++ b/docs/animation-model.md @@ -268,6 +268,22 @@ This is what `docs/design.md` means by an override layer, and it is why re-freezing is safe: the base is regenerated, the layers are untouched. It is Blender's NLA blending and AE's effect stack at one property. +WHEN THE BASE OUTGROWS A CORRECTION it is a CONFLICT, which is neither a dropped +layer nor an applied one. Turning `:verts` gives the mouth a different number of +points, and an `:offset` is a row of components that has to match: so the +regeneration records `:conflict` on the layer, the layer stays in the document, +the picture is the base meanwhile, and `clip/conflicts` is the list a view +offers to resolve. Deliberately not `problems` — the document loads and saves +fine, it just contains a decision nobody has made yet. A later regeneration +that restores the shape clears the mark. Only `:offset` can conflict; `:replace` +states a whole value and has nothing to agree with. + +A correction is NOT a hand placement. `regenerate-head` leaves the head's +authored channels alone once somebody has placed it by hand, and it compares the +channels WITHOUT their layers to decide: otherwise the first correction anyone +made would stop the head following re-measurement forever, which is the opposite +of what a layer is for. + Layers are what "set it by hand" means for anything measured, and the measured channel does not need to know. A hand-set gaze is an `:over` on `[:xform :pos]` of the iris; a hand-set mouth shape is an `:over` on diff --git a/docs/lane-model.md b/docs/lane-model.md index 2ae0054..00da7b8 100644 --- a/docs/lane-model.md +++ b/docs/lane-model.md @@ -488,13 +488,27 @@ on the girl's lane reaches across the drawing boundary beneath it and leaves every frame outside its support identical, and a correction owned by one exposure travels with that exposure when a hold before it grows. +Regeneration keeps them, which is the obligation the layer design exists to +meet: `rebased` replaces a base and carries its corrections across, and a +correction the new base no longer fits is MARKED rather than dropped or +misapplied — `clip/conflicts` lists those for a view to offer, separately from +`problems`, because a conflict is a decision nobody has made yet and not a +document that will not load. Turning the mouth's `:verts` knob is a real +topology change and is what the test uses. Two latent faults turned up there and +are fixed: `regenerate-head` compared authored channels to measured ones +directly, so the first correction on the head would have stopped it following +re-measurement for good; and an incompatible offset threw in the read path, +which would have taken the stage down on exactly the case the model says to +report. + What is NOT implemented is a command that produces a layer — the doc's Constant adjustment, Ramp and Return motion — and with it the question of how a view -offers those three over a selected range. Overwrite, the range and retiming +offers those three over a selected range, and how it offers a conflict for +resolution. Overwrite, the range and retiming commands (blank, trim, move, slip source, retime) and the exposure-sheet view are also not implemented; a refusal is the current behavior where the model -demands an explicit choice nobody has made yet. The suite stands at 408 tests -and 5,655 assertions, with `frontend/test/browser/sequence.mjs` driving the +demands an explicit choice nobody has made yet. The suite stands at 414 tests +and 5,696 assertions, with `frontend/test/browser/sequence.mjs` driving the editor through create, hold, overflow, undo, reuse, make unique, duplicate, split and insert. Rewrite tests that encode superseded behavior rather than preserving behavior to keep them green. diff --git a/frontend/src/arthur/domain/channel.cljs b/frontend/src/arthur/domain/channel.cljs index 9f9bd11..277c7a1 100644 --- a/frontend/src/arthur/domain/channel.cljs +++ b/frontend/src/arthur/domain/channel.cljs @@ -132,6 +132,63 @@ [v] (cond (number? v) nil (vector? v) (count v) :else (.-length v))) +(defn- shape + "What kind of value this is, for asking whether one can be added to another: + `:scalar`, a component count, or `:opaque` for a value that is neither — a + `[:vis]` boolean is opaque, and can be replaced but not offset." + [v] + (cond + (number? v) :scalar + (vector? v) (count v) + (and (some? v) (number? (.-length v))) (.-length v) + :else :opaque)) + +(defn value-shape + "The shape of the values a channel yields, without sampling it, or nil where + there is nothing to read it off — an empty key map says nothing about what its + values would have been, and nil must not be taken for a scalar." + [ch] + (cond + (not (:animated? ch)) (when (some? (:value ch)) (shape (:value ch))) + (:dense ch) (if (= 1 (:stride (:dense ch))) :scalar (:stride (:dense ch))) + (seq (:keys ch)) (shape (val (first (:keys ch)))) + :else nil)) + +(defn conflict-with + "Why correction `l` cannot apply to base channel `base`, or nil. + + ONLY `:offset` can conflict. It adds component by component, so it needs the + base to have the components it has — which is what a topology change takes + away when a re-freeze gives a mouth a different number of points. `:replace` + states a whole value and so has nothing to agree with. + + Shapes that cannot be read yet do not conflict: an empty key map is not a + disagreement, it is a channel with nothing in it." + [base l] + (when (= :offset (:op l)) + (let [b (value-shape base) v (value-shape (:values l))] + (cond + (or (nil? b) (nil? v)) nil + (= :opaque b) "the base is not a number or a row of components" + (= :opaque v) "the correction is not a number or a row of components" + (not= b v) (str "the base has " (pr-str b) " and the correction " + (pr-str v) " — a correction cannot offset a value of" + " a different shape"))))) + +(defn conflicts + "Corrections on `ch` that cannot apply to its base, as `[{:id :why}]`. + + NOT `problems`. A conflict is a legitimate state for a document to be in: a + regeneration changed the topology under a correction that was right when it was + made, and resolving it is a person's decision, not a reason the document will + not load. `flow/regenerate` records one on the layer, a conflicted layer is not + applied, and this is how a view finds them to offer." + [ch] + (vec (for [l (:over ch) + :let [why (or (:conflict l) (conflict-with ch l))] + :when why] + {:id (:id l) :why why}))) + (defn- offset-onto "`base` plus `v`, component-wise. A vector, never a write into `base`, which for a dense channel is a view onto the block itself." @@ -150,8 +207,11 @@ and is the only thing that differs between the specification and the cursor." [ch f base read] (reduce-kv - (fn [v i {:keys [support op values]}] - (if-not (covers? support f) + (fn [v i {:keys [support op values conflict]}] + ;; A conflicted correction is neither applied nor forgotten: it stays in + ;; the document, `conflicts` reports it, and a person decides. Applying it + ;; would misapply it; removing it would throw away hand work. + (if (or conflict (not (covers? support f))) v (let [x (read i values f)] (cond @@ -462,6 +522,17 @@ (first (problems (dissoc values :over))))))] (str "correction " (pr-str id) " " p))) + ;; A shape mismatch NOBODY HAS RECORDED is an authoring bug; one a + ;; regeneration recorded is a conflict awaiting a person, and `conflicts` + ;; reports those. The distinction is what keeps a topology change from + ;; making a document that will not load. + (and (map? ch) (vector? (:over ch))) + (into (for [l (:over ch) + :when (not (:conflict l)) + :let [why (conflict-with ch l)] + :when why] + (str "correction " (pr-str (:id l)) " " why))) + ;; A scale of zero divides every value in the block by zero, and a negative ;; one mirrors the geometry. Both are authored-data bugs that present as a ;; part drawn nowhere or inside out, not as an error. diff --git a/frontend/src/arthur/domain/clip.cljs b/frontend/src/arthur/domain/clip.cljs index 1c6f52b..da5d4a4 100644 --- a/frontend/src/arthur/domain/clip.cljs +++ b/frontend/src/arthur/domain/clip.cljs @@ -366,6 +366,23 @@ (map #(keyword (namespace wanted) (str (name wanted) "-" %)) (iterate inc 2)))))) +(defn conflicts + "Every hand correction in the document that its base has outgrown, as + `[{:symbol :node :channel :id :why}]`. + + SEPARATE FROM `problems` on purpose. A conflict is a document a person still + has to make a decision about — a regeneration changed the topology under a + correction that was right when it was made — and not a reason the document + will not load. Nothing is dropped and nothing is misapplied meanwhile: the + layer stays where it is, the picture is the base, and this is the list a view + offers to resolve." + [clip] + (vec (for [[sid sym] (:symbols clip) + [id n] (:nodes sym) + [prop c] (:channels n) + {:keys [why] :as x} (ch/conflicts c)] + (assoc (select-keys x [:id]) :symbol sid :node id :channel prop :why why)))) + (defn problems "Human-readable reasons this clip will not evaluate or save." [clip] diff --git a/frontend/src/arthur/flow/regenerate.cljs b/frontend/src/arthur/flow/regenerate.cljs index 7fe91da..03e58a8 100644 --- a/frontend/src/arthur/flow/regenerate.cljs +++ b/frontend/src/arthur/flow/regenerate.cljs @@ -1,7 +1,8 @@ (ns arthur.flow.regenerate "Recompute a changed feature from retained source tracks, then replace only channels owned by that feature. Upload remains project/save's ordinary job." - (:require [arthur.domain.feature :as feature] + (:require [arthur.domain.channel :as ch] + [arthur.domain.feature :as feature] [arthur.domain.params :as params] [arthur.flow.address :as address] [arthur.flow.freeze :as freeze] @@ -33,6 +34,35 @@ (select-keys (settings clip fid) (address/area-knobs (get-in clip [:features fid :area])))) +(defn- rebased + "`fresh` in place of `old`, carrying `old`'s corrections across. + + THIS IS WHAT A LAYER IS FOR. The base is regenerated and the hand work is not, + which is the whole reason a correction is stored over a channel rather than + written into it. A correction the new base no longer fits is MARKED rather than + dropped or misapplied — `channel/conflicts` is how a view finds it — and one + that fits again has its mark cleared, because a regeneration that restores the + topology has resolved it." + [old fresh] + (if-let [over (seq (:over old))] + (assoc fresh :over + (mapv (fn [l] + (if-let [why (ch/conflict-with fresh l)] + (assoc l :conflict why) + (dissoc l :conflict))) + over)) + fresh)) + +(defn- bases + "A node's channels without their corrections. + + For asking whether the authored channels still ARE the measurement: a + correction is not a hand PLACEMENT. The base under it has to go on following + re-measurement, or the first correction anyone makes would freeze the part it + was meant to adjust." + [channels] + (into {} (map (fn [[p c]] [p (dissoc c :over)])) channels)) + (defn- replace-feature [entry fragment fid] (let [paths (for [id (get-in entry [:clip :features fid :nodes]) [prop channel] (get-in fragment [:nodes id :channels]) @@ -42,9 +72,7 @@ (let [at [:clip :symbols (get-in entry [:clip :features fid :symbol]) :nodes id :channels prop] old (get-in entry at)] - (assoc-in entry at - (cond-> channel - (contains? old :over) (assoc :over (:over old)))))) + (assoc-in entry at (rebased old channel)))) entry paths) (update :store merge (:store fragment))))) @@ -109,8 +137,10 @@ (cond-> (-> entry (assoc-in (conj at :measured) measured) (update :store merge (:store baked))) - (= (:channels old) (:measured old)) - (assoc-in (conj at :channels) measured)))) + (= (bases (:channels old)) (bases (:measured old))) + (assoc-in (conj at :channels) + (into {} (map (fn [[p c]] [p (rebased (get-in old [:channels p]) c)])) + measured))))) (defn change "One scoped static edit. `source-inputs` holds dense landmarks and, when the diff --git a/frontend/test/arthur/domain/channel_test.cljs b/frontend/test/arthur/domain/channel_test.cljs index 6c95624..09a393e 100644 --- a/frontend/test/arthur/domain/channel_test.cljs +++ b/frontend/test/arthur/domain/channel_test.cljs @@ -364,3 +364,45 @@ with (assoc base :generated {:by :roto/lips-outer :params {:verts 8}})] (is (= (mapv #(ch/value-at base % store) (range 3)) (mapv #(ch/value-at with % store) (range 3)))))) + +(deftest a-correction-the-base-outgrew-is-a-conflict-and-not-a-problem + ;; What a topology change does: a re-freeze gives the mouth a different number + ;; of points, and a correction that was right when it was made can no longer + ;; be added component by component. That is not a broken document — it is a + ;; decision waiting for a person. + (let [pts (fn [n] {:animated? true :interp :hold :keys {0 (vec (repeat n 1))}}) + nudge (ch/layer :nudge [0 4] :offset (ch/framed [1 1 1 1])) + fits (assoc (pts 4) :over [nudge]) + outgrown (assoc (pts 6) :over [nudge])] + (is (nil? (ch/conflict-with (pts 4) nudge))) + (is (empty? (ch/conflicts fits))) + (is (empty? (ch/problems fits))) + (is (re-find #"different shape" (ch/conflict-with (pts 6) nudge))) + ;; Unrecorded, it is an authoring bug and says so. + (is (seq (ch/problems outgrown))) + (is (= [:nudge] (mapv :id (ch/conflicts outgrown)))) + ;; Recorded, the document is sound and the correction is simply not applied. + (let [marked (assoc (pts 6) :over [(assoc nudge :conflict "outgrown")])] + (is (empty? (ch/problems marked)) "a recorded conflict is not a reason not to load") + (is (= [:nudge] (mapv :id (ch/conflicts marked)))) + (is (= [1 1 1 1 1 1] (vec (ch/value-at marked 0 nil))) + "the base alone — neither misapplied nor silently dropped") + (is (= (ch/value-at marked 0 nil) (first (via-cursor marked [0]))) + "and the cursor skips it too")))) + +(deftest replace-never-conflicts-and-an-opaque-value-cannot-be-offset + (let [flag (ch/keyed {0 true} :hold)] + (is (nil? (ch/conflict-with flag (ch/layer :r [0 2] :replace (ch/framed false)))) + "replace states a whole value, so it has nothing to agree with") + (is (re-find #"not a number" (ch/conflict-with flag (ch/layer :o [0 2] :offset (ch/framed 1))))) + (is (nil? (ch/conflict-with (ch/keyed {} :hold) (ch/layer :o [0 2] :offset (ch/framed 1)))) + "an empty key map is not a disagreement"))) + +(deftest the-shape-of-a-channels-values-is-readable-without-sampling-it + (is (= :scalar (ch/value-shape (ch/framed 3)))) + (is (= 2 (ch/value-shape (ch/framed [1 2])))) + (is (= :scalar (ch/value-shape {:animated? true :dense {:stride 1}}))) + (is (= 40 (ch/value-shape {:animated? true :dense {:stride 40}}))) + (is (= 2 (ch/value-shape (ch/keyed {0 [1 2], 4 [3 4]} :linear)))) + (is (= :opaque (ch/value-shape (ch/keyed {0 :a} :hold)))) + (is (nil? (ch/value-shape (ch/keyed {} :hold))) "nothing to read it off")) diff --git a/frontend/test/arthur/flow/regenerate_test.cljs b/frontend/test/arthur/flow/regenerate_test.cljs index 8fa5bbb..e0b7a0b 100644 --- a/frontend/test/arthur/flow/regenerate_test.cljs +++ b/frontend/test/arthur/flow/regenerate_test.cljs @@ -2,6 +2,7 @@ (:require [cljs.test :refer [deftest is testing]] [clojure.walk :as walk] [arthur.demo.stage :as stage] + [arthur.domain.channel :as ch] [arthur.domain.clip :as clip] [arthur.domain.params :as params] [arthur.domain.project :as project] @@ -256,3 +257,98 @@ (doseq [[_ n] (filter (comp #{:audio} :kind val) nodes)] (is (contains? nodes (:linked-to n)) (str "the voice " (:id n) " still links to a node that is there"))))) + +;; ---- corrections survive the thing they are corrections to ---- + +(defn- corrected + "Put one offset correction on a node's channel, as a hand edit would." + [entry node path values] + (update-in entry [:clip :symbols :face-1 :nodes node :channels path :over] + (fnil conj []) (ch/layer :by-hand [2 6] :offset values))) + +(deftest regenerating-replaces-the-base-and-keeps-the-hand-correction + ;; The loop the whole layer design exists for: generate motion, correct it by + ;; hand, turn the generator's knob, keep the correction. + (let [width (count (:keys (channel @initial :iris-r [:xform :pos]))) + before (corrected @initial :iris-r [:xform :pos] (ch/framed [3 -3])) + after (regenerate/change before + {:scope :feature :id :face-1/eye-r :knob :gaze-gain :value 2}) + base (fn [entry] (dissoc (channel entry :iris-r [:xform :pos]) :over))] + (is (not= (base before) (base after)) "the base was regenerated") + (is (= (base (regenerate/change @initial + {:scope :feature :id :face-1/eye-r :knob :gaze-gain :value 2})) + (base after)) + "and regenerated to exactly what it would have been without the correction") + (is (= [(ch/layer :by-hand [2 6] :offset (ch/framed [3 -3]))] + (:over (channel after :iris-r [:xform :pos]))) + "while the correction came across untouched, and unconflicted") + (is (empty? (ch/conflicts (channel after :iris-r [:xform :pos])))) + (is (empty? (clip/problems (:clip after)))) + (is (= width (count (:keys (channel after :iris-r [:xform :pos])))) + "sanity: this channel is keyed, so the correction rides a keyed base"))) + +(deftest a-correction-does-not-stop-the-head-following-its-measurement + ;; `regenerate-head` leaves the authored channels alone once somebody has + ;; PLACED the head by hand — but a correction is not a placement. Comparing + ;; the bases is what keeps the first correction from freezing the part it was + ;; made to adjust. + (let [plain (regenerate/change @initial + {:scope :subject :id :face-1 :knob :anchor-avg :value 4}) + path [:clip :symbols :face-1 :nodes :head] + prop (first (keys (get-in @initial (conj path :measured)))) + shape (ch/value-shape (get-in @initial (conj path :channels prop))) + nudge (ch/layer :by-hand [2 6] :offset + (ch/framed (if (= :scalar shape) 1 (vec (repeat shape 0.5))))) + before (update-in @initial (conj path :channels prop :over) (fnil conj []) nudge) + after (regenerate/change before + {:scope :subject :id :face-1 :knob :anchor-avg :value 4}) + base (fn [entry] (dissoc (get-in entry (conj path :channels prop)) :over))] + (is (= (base plain) (base after)) + "the head's base followed the re-measurement, correction and all") + (is (= [nudge] (:over (get-in after (conj path :channels prop)))) + "and the correction is the one that was made, unmarked") + (is (empty? (ch/conflicts (get-in after (conj path :channels prop))))) + (is (empty? (clip/problems (:clip after)))))) + +(deftest a-regeneration-that-outgrows-a-correction-records-the-conflict + ;; `:verts` is the mouth's vertex count, so turning it IS the topology change + ;; the lane model names. A geometry correction is a row of components, and a + ;; base with a different number of them cannot take it. Marked, not dropped: + ;; the hand work stays in the document for a person to move, and the picture + ;; meanwhile is the base. + (let [path [:geom :pts] + fitted (fn [entry] + (ch/layer :by-hand [2 6] :offset + (ch/framed (vec (repeat (ch/value-shape (channel entry :mouth path)) + 0.5))))) + with (fn [layer] (update-in @initial + [:clip :symbols :face-1 :nodes :mouth :channels path :over] + (fnil conj []) layer)) + before (with (fitted @initial)) + after (regenerate/change before + {:scope :feature :id :face-1/mouth :knob :verts :value 10}) + layer (first (:over (channel after :mouth path)))] + (is (not= (ch/value-shape (channel @initial :mouth path)) + (ch/value-shape (channel after :mouth path))) + "the mouth really does have a different number of points now") + (is (= :by-hand (:id layer)) "the correction is still in the document") + (is (re-find #"different shape" (:conflict layer))) + (is (= [:by-hand] (mapv :id (ch/conflicts (channel after :mouth path))))) + (is (empty? (clip/problems (:clip after))) + "a recorded conflict does not make the document unloadable") + (is (= (dissoc (channel (regenerate/change @initial + {:scope :feature :id :face-1/mouth :knob :verts :value 10}) + :mouth path) + :over) + (dissoc (channel after :mouth path) :over)) + "and the base is what it would have been with no correction at all") + ;; And the document says so once, for a view to offer. + (is (= [{:id :by-hand :symbol :face-1 :node :mouth :channel [:geom :pts]}] + (mapv #(dissoc % :why) (clip/conflicts (:clip after))))) + ;; A mouth edit that does not change the vertex count leaves it applying. + (let [fine (regenerate/change before + {:scope :feature :id :face-1/mouth :knob :aperture-cut :value 0.2})] + (is (nil? (:conflict (first (:over (channel fine :mouth path)))))) + (is (empty? (ch/conflicts (channel fine :mouth path)))) + (is (empty? (clip/conflicts (:clip fine)))) + (is (empty? (clip/problems (:clip fine)))))))