My understanding is:

• Global undo step stores the state after the corresponding operation.
• Sculpt undo step stores the state before the corresponding operation. Once the step is undone, its state is swapped to become the state after the corresponding operation. When a step is redone, it is swapped back to before.

I don't find this clearly explained in the wiki or comments, but for me it's the most important thing to understand the logic here. That the contents of a "differential" undo step actually changes as you undo/redo it.

In D10475#264581, @Brecht Van Lommel (brecht) wrote:

• Sculpt undo step stores the state before the corresponding operation. Once the step is undone, its state is swapped to become the state after the corresponding operation. When a step is redone, it is swapped back to before.

But this is how sculpt undo works internally I think? This patch is mostly trying to formalize things from the external, higher general level of BKE_undo_system perspective, which is supposed to manage a mix of random undo types in a generic common way. From that point of view, I think sculpt undo behaves like a differential system, since you cannot load the step you want, but can only (un)apply the steps until you reach the target one? Which is why we need to process that target step n when redoing, while we only process up to step n + 1 in case of undo...

Trying to make an analogy, from BKE_undo_system point of view, sculpt undo works like a set of additions/subtractions:

• step 3 is result of + 1 + 2 + 3 (steps 1, 2, 3 are applied);
• to undo to step 1 you un-apply steps 3 and 2 (+ 1 + 2 + 3 - 3 - 2);
• to redo to step 2 you re-apply step 2 (+ 1 + 2 + 3 - 3 - 2 + 2);
• etc.

While memfile undo e.g. works like a set of states that you can load in sequence, you always get a state matching the last step you processed, regardless of being undo or redo:

• step 3 is result of 1 => 2 => 3 (step 3 is loaded);
• to undo to step 1 you load steps 3, 2, then 1 ( 1 => 2 => 3 => 3 => 2 => 1);
• to redo to step 2 you load step 2 (1 => 2 => 3 => 3 => 2 => 1 => 2);
• etc.

PS: yes, we definitively need to document also how each specific undo type works, but this is separated from the global common undo stack handling imho. Ideally each undo type behavior should be a black box from the perspective of BKE_undo_system.

For me personally, it's easier to think of all undo steps as changing the data from some state X to Y. Once that is known, we can derive how to chain together steps to go from the current state to the desired state.

If we think of one type of undo step as "loading states", and another as "applying steps", for me that's not a great abstraction because I don't know what those things mean precisely.

I'd also prefer to keep undo steps as states that are loaded. The undo systems can manage their own details about how the data is applied (where differential steps are more of an implementation detail).

We might still need a flag for undo-sytems for extra handling to properly implement differential undo systems though.

Another reason I'd rather not do this is currently, differential undo steps are only done by image and sculpt undo-systems, as long as these can be made to work properly, their internals can be refactored without impacting the basics of undo/redo.

• I've committed a fix for T82532 (which this patch also fixed), The sculpt assumed a mode-switch undo-step which doesn't always exist.

• Both with this patch and with current master, I found problems with the memfile undo getting out of sync with the brush strokes, see T86580: Sculpt undo bug, mixing sculpt brush and multi-res modifier changes..
  
  I'd like to understand what's going on here before applying patches that change sculpt undo behavior, otherwise we risk the new code not accounting for cases like this.

Are there remaining issues in master this patch fixes? The WITH_GLOBAL_UNDO_CORRECT_ORDER block doesn't seem directly related to UNDOTYPE_DIFFERENTIAL.