QinL / grafx2

Automatically exported from code.google.com/p/grafx2
0 stars 0 forks source link

Some changes can't be undone on animations > 32 frames #516

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Tools that alter all frames while keeping their dimensions may not be undoable 
on frame numbers above 32.
The relevant tools are :
- the "Pan" (scroll) tool, when you use right-click to offset all the frames at 
the same time.
- Palette modifications that cause a remap of the pixels: x-swap, x-inv, sort, 
reduce.

This bug relies on an unspecified behavior of the << operator, normally 
dependant on architecture. Strangely, on a Pentium-III it doesn't happen, on a 
Pentium-M it does...

Original issue reported on code.google.com by yrizoud on 15 Oct 2012 at 6:55

GoogleCodeExporter commented 8 years ago
Fixed in r2033, I think, but the change is bigger than I'm comfortable with. 
Will need a bit more extensive testing.
The same fault from Backup_layers(n) was also present in Create_new_frame(n) 
and Backup_the_spare(n). In theory this means that more functions could have 
been affected by the same bug:
- "copy to spare / palette and remap"
- a Lua script that draws in the spare, if the spare's current frame is > 32
The following could have crashed, due to creating incomplete backups while 
changing dimensions:
- "copy to spare / pixels only"
- "copy to spare / pixels and palette"
- Lua script which uses setsparepicturesize()

In practice, I have never been able to produce any of the above cases... Maybe 
we're lucky and it simply doesn't happen on x86 ?

In r2033, I removed some page-saving in the following cases that didn't need 
them : these operations don't change the pixel data, so the History step 
doesn't need a full backup.
- Copy to Spare / Pixels only (a duplicate history step was created)
- Copy to Spare / Pixels + palette (a duplicate history step was created)
- Modifying the transparent color index, from button or Layer menu.
- Palette screen : when changing palette data only (no x-swap, no x-inv, no 
sort, no reduce)

Original comment by yrizoud on 16 Oct 2012 at 12:35

GoogleCodeExporter commented 8 years ago
After a bit more search on <<, it seems that on x86 there would never be any 
problem. What I saw on my laptop (Pentium-M) may have been caused by 
compile-time optimizations : I typed on vi and shortened my test case.

Original comment by yrizoud on 16 Oct 2012 at 12:47

GoogleCodeExporter commented 8 years ago
And with more sleep I understand better why this bug doesn't have horrible 
consequences on x86: the only side effect is it will use more memory when you 
work on animations with for example 96 images : A change on frame 1 will cause 
a backup of images 1, 33, 65 instead of just 1. Similarly, a change on frame 34 
will backup frames 2, 34, 66 instead of just 34. The reference-count still 
works, so when you fill the Undo steps limit it will start cleaning up pages 
that no step references. (no memory leak)

Original comment by yrizoud on 16 Oct 2012 at 1:39

GoogleCodeExporter commented 8 years ago

Original comment by pulkoma...@gmail.com on 6 Jan 2013 at 12:51