tildearrow / furnace

a multi-system chiptune tracker compatible with DefleMask modules
GNU General Public License v2.0
2.57k stars 206 forks source link

Instrument editor undo #2067

Closed alederer closed 1 month ago

alederer commented 1 month ago

Based roughly on the sample editor's undo feature --

Potential future issues/work

alederer commented 1 month ago

thanks for approving the CI workflow @freq-mod --

"fixed" the class-memaccess error on Linux by casting to unsigned char* (it's a reasonable warning but using memset on non-trivial class is intentional here to avoid garbage in the padding)

alederer commented 1 month ago

very low-hanging options:

other possibilities:

do we have an established minimum specification to run on? like hopefully we could profile and find a memcmp of 10k or whatever is trivial?

tildearrow commented 1 month ago

very low-hanging options:

  • recordUndoStepIfChanged can run a memcmp on the data to see if it can skip its byte-by-byte check (presuming a memcmp would be more optimized)
  • skip copying into cachedCurIns if we already know it hasn't changed

other possibilities:

  • we could only run the comparison on mouse-up and key-up, or something like that, but it makes me a bit nervous to assume that much about possible sources of modification (maybe since i already got bitten assuming all edits were from inside drawInsEdit!). if you feel comfortable with it i can implement it that way though.
  • i'd be hesitant to put it on a timer ("every 10 frames" or whatever), since it'd create uneven load (harder to profile, if it really was slow would cause jitters), and it could end up grouping together modifications that should have separate undo states

do we have an established minimum specification to run on? like hopefully we could profile and find a memcmp of 10k or whatever is trivial?

On mouse up and key up sound good. User input is the only way to change instrument parameters.